wp-shortcake / shortcake

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

Selectbox with 'Multiple' set does not render multiselect box as expected #757

Closed forkbombe closed 7 years ago

forkbombe commented 7 years ago

When setting param multiple to true when employing a select box it does not render the multiple select box as expected.

array(
  'label'     => 'Sites',
  'attr'      => 'sites',
  'type'      => 'select',
  'options'   => $site_opts,
  'description'   => 'Select which sites you want to display job postings from', 

  // has no effect
  'multiple' => true 
),

I have tried adding the addition 'multiple' attribute to the data.meta being passed to the template from views/edit-attribute-field.js but the template converts the array into comma separated text. Setting multiple values results in no value being set at all.

... (around line 41)

// If multiple is set
if(data.multiple===true) {
  _meta.push('multiple');
}

data.meta = _meta.join( ' ' );
...
christianwach commented 7 years ago

It appears that the docs are wrong on this. Try this instead:

'meta' => array( 'multiple' => true ),

Works for me :-)

davisshaver commented 7 years ago

Hi @christianwach @bambattajb - thanks for raising this issue.

I looked at the dev.php plugin for reference. Currently we do use 'multiple' => true' in a few examples, but for types post_select, term_select, and user_select the example shortcode seems to be working as expected.

We don't support that attribute on select type but you can use meta to pass attributes directly to the select element. This is a little different than what we are doing with the other fields. I've made a PR to show this in the docs. https://github.com/wp-shortcake/shortcake/pull/759

@christianwach can you point me to where you see the error in docs?

christianwach commented 7 years ago

@davisshaver Perhaps "error" was too strong a word. "Omission" might have been a better description since there seems to be no documentation of the fact that multi-selects are possible - even though they work as expected when using meta to define them. Which is great :)

davisshaver commented 7 years ago

@christianwach Just wanted to make sure. Could you take a look at #759?

christianwach commented 7 years ago

@davisshaver Looks good to me. Thanks for updating.

forkbombe commented 7 years ago

Thanks. Definitely works with meta => array('multiple'=>true) and updated docs are very useful