Unnamed/numeric attribute fix (revised) #198
Conversation
Make up to date
Updated master
Updated master
Since the code has changed a lot since wp-shortcake#180, I’m redoing the changes with the new file structure
|
|
||
| // Numeric attribute names | ||
| if ( ! isNaN( attr.get( 'attr' ) ) ) { | ||
| attrs.push( '"' + attr.get( 'value' ) + '"' ); |
There was a problem hiding this comment.
Do we need to wrap in quotes?
|
@mattheu How do you feel about this? |
|
@danielbachhuber removed the quotes |
Updated master
| attrs.push( attr.get( 'attr' ) + '="' + attr.get( 'value' ) + '"' ); | ||
|
|
||
| // Numeric attribute names | ||
| if ( ! isNaN( attr.get( 'attr' ) ) ) { |
There was a problem hiding this comment.
Lots of double negatives here... We could check $.isNumeric( attr.get( 'attr' ) ) instead?
|
Whilst I'd prefer to just not support numeric/positional attributes for simplicity - it is a feature of shortcodes. 😄 This works well - I think this its the best solution to the problem. Its a bit out of sync with master - but shouldn't be too bad to update. |
|
A couple of issues: I suppose the key is in the word 'numeric'... but should we also account for users who specify the shortcode attribute name as an Int? If not all numeric attributes are supplied it should probably default to |
|
@mattheu I believe handling of numeric attributes is the same. I did the change. Empty unnamed attributes now appear as |
Updated master
Updated master
Conflicts: js/build/shortcode-ui.js js/models/shortcode.js js/utils/shortcode-view-constructor.js
|
Synced the code to the current master |
|
@mattheu how do you feel about this? |
|
Hold on, things aren't working as they are supposed to |
|
This will need to be handled in a later release. |
This is a continuation of the fix for unnamed/numeric attributes. This PR replaces #180