search

wp-shortcake / shortcake

Shortcake makes using WordPress shortcodes a piece of cake.
GNU General Public License v2.0
664 stars 142 forks source link

Ensure select's data value is an array before filtering #738

Closed jeremyfelt closed 7 years ago

jeremyfelt commented 7 years ago

During work in #707 to improve support for multi-select fields, handling of comma separated strings was removed as it may have been confusing if an option value had commas in it.

See 9894162ab8bf02941e9611766d5cc3d63d337e96

After this change, string values for single select elements are filtered as if they were arrays. This causes only the first character of a string to be compared to the select value. This in turn results in all select fields set to first option because nothing is seen as selected.

We can accept arrays, but also split() with no separator to turn strings into an array so that everything is parsed the same.

cowanr commented 7 years ago

This fix still leaves multi selects not being selected when editing a shortcode. The short term fix for me was to just ensure that I don't have comma's in my values and split on comma instead of an empty split.

<option value="{{ option.value }}" <# if ( ! _.isEmpty( _.filter( _.isArray( data.value ) ? data.value : data.value.split(","), function(val) { return val === option.value; } ) ) ) { print('selected'); } #>>{{ option.label }}</option>

mattheu commented 7 years ago

I tried to reproduce the issue @cowanr was having and I think can reproduce. I think they are correct that simply calling .split(',') will work instead.

Also I think we can simplify this code by using _.contains. I'm going to open a new PR with these additions.

goldenapples commented 7 years ago

Closing this PR in favor of #745.