Don't wipe existing settings in '#slim_select'.
Closed this issue · 6 comments
The oe_whitelabel integration of slim_select uses a '#process' callback on 'select' element type, to set $element['#slim_select'] = [].
This will destroy any custom setting that might already exist on the element.
E.g. a custom module or theme could use hook_field_widget_complete_options_select_form_alter() to either add specific settings in $field_widget_complete_form['widget']['#slim_select'], or it could set $field_widget_complete_form['widget']['#slim_select'] = NULL to completely block the slim_select integration for this specific element. But this would be overwritten by _oe_whitelabel_process_element_select().
Two possible solutions:
- In
_oe_whitelabel_process_element_select(), use$element += ['#slim_select' => []];instead of$element['#slim_select'] = [];, to not overwrite existing values. - In
oe_whitelabel_element_info_alter(), use$info['select']['#slim_select'] = [];. This will be merged inFormBuilder::doBuildForm()where the element defaults are applied.
One question is what to do with the 'multi-select' class that is being added in _oe_whitelabel_process_element_select(). This is a bit weird: Currently this class is only added if slim_select module is enabled. And indeed, this class does have an effect on the styling of the multi select elements. But if this is the goal, why not target the classes that are added by slim-select library? Or add this class on client side, when we are sure that slim-select is being applied.
If we no longer need to add the class in php, and use option 2 from above, we can drop the callback.
Btw the original goal was to remove the '_none' option if slim_select is active.
See https://www.drupal.org/project/slim_select/issues/3393773
Option 2 is not ok as we don't want to add ['#slim_select'] to all selects, only when it's a multivalue one.
The custom class is to apply the behaviour defined in BCL. Could be refactored out, but that would break BC.
Option 2 is not ok as we don't want to add
['#slim_select']to all selects, only when it's a multivalue one.
Good point. So we need to keep the '#process'.
The custom class is to apply the behaviour defined in BCL. Could be refactored out, but that would break BC.
We could still add it on client side with js, to be sure it is only added if slim_select is active.
But I think it is better to leave it as is until we experience real-world problems.
This leaves us with option one, which is a minor change to preserve existing config in the property.
Since I'm handling something related to slim select, I'll fix that too. Will provide link soon.
About removing the "None" option: For now we did that with custom code in the specific project.
But I think it would make sense to always do this for widget with slim_select active.