I don't think Chosen resources should be loaded on every page load. We should let developers decide when they want to use chosen.

I propose that Chosen is attached to select elements in 2 ways:

  1. Through the Field UI as a widget
  2. With the Form API (#type = 'chosen' etc)

If either of the above occurs, then a drupal library for chosen should be #attached to the element.
What do you think?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chrisarusso’s picture

Here's the patch. It affects 3 files:

git diff -w --stat
chosen.admin.inc | 10 ++++----
chosen.js | 6 +++-
chosen.module | 61 +++++++++++++++++++++++++++++++++++++++++++++--------

Commits with more thorough explanations are here: https://bitbucket.org/illmasterc/drupaljoe/changesets/tip/1476036

Xano’s picture

The rationale behind loading Chosen on every page and letting site admins configure a jQuery selector is that site admins rather than module developers can choose whether they want to use Chosen.

Module developers should not care about this (too much), since Chosen is just a glorified select element and it doesn't change how the data is processed.

fenda’s picture

But then site admins will need to find out the class of form elements for chosen to apply to, which is slightly 'developery' anyway. You're basically having site admins write code in the UI, which is usually not a preferred way to do things. This is a drupal module so we should do things the 'drupal way', right?

Why not Just stick a few lines of code in this modules documentation explaining how to, if required, add chosen to a specific form element.

e.g.

function hook_form_FORM_ID_alter(&$form, &$form_state, $form_id) {
  $form['my_select']['#pre_render'] = array('chosen_pre_render');
}
Xano’s picture

Not really, because the default settings will work for any select element on the website. Site admins will only have to edit the selector if they do not wish this behavior.

Also, requiring PHP for configuration heightens the bar. I agree that technically, from a DX point of view, configuring a jQuery selector in the UI is not the best solution, but it is certainly the easiest for most people.

fenda’s picture

Well the main issue here, the reason I opened this issue, was that we should not be adding JS via hook_init(). JS should only be added when it really is needed. For an empty drupal install, it wouldn't really matter to performance but when you've got a heavy traffic website with a large number of modules - every little bit of performance is crucial.

With the js selectors, Drupal cannot check if that element is on the page before including the JS.

Another issue is when you have a site that wants to apply chosen to a large number of custom form select elements. Let's say there is 50 in total. So they have to put 50 lines of js selectors in the UI. Not only is it hard to manage but a variable with that much text should usually be avoided where possible for performance reasons.

If a site admin can write js selectors, they can write 3 lines in a custom module and we should encourage that.

fenda’s picture

How about we offer options for the site admin to decide for themselves?

Automatically enable chosen on

  1. All select elements
  2. Specific select elements
  3. No elements (Form API usage only)

Setting to option 1 would do what the patch in #1 does. Option 2 is what Chosen does at the moment, Option 3 would do nothing, but offer the #pre_render = array('chosen_pre_render') form API usage for developers.

Xano’s picture

A patch to allow developers to always convert a specific element to a Chosen selector will most likely make it in. Whether they should be able to do that sing #pre_render or #type, or any other technical details, we should discuss in a separate issue.

Let's keep the jQuery selector in for now. If the PHP solution has been committed and found useful, we can decide whether to keep the selector functionality or not.

fenda’s picture

Isn't this issue specifically for the PHP solution?

Cyclodex’s picture

Hey

I see you were discussing here. But already some time ago, want to continue and finish this ? :)

fotuzlab’s picture

My 2 cents...
I think the issue is about striking a balance between flexibility and control.

Here is what I have done in Dropdown checkboxes:

1. Can be configured in form API using '#pre_render' element. Used '#pre_render' than '#dropdown_checkboxes' as the js works on the list after it has been build. Also provides a default fallback behavior.

2. Ids can be added in UI and optionally paths can be configured where the js should load (similar to block settings). If no paths are configured, js would load in hook_init() as currently handled by chosen.

minorOffense’s picture

Status: Active » Needs review
FileSize
2.88 KB

I've created a patch which allows you to turn the hook_init include of the library on/off as required. Mix that with the other patch I wrote to add proper support for drupal_add_library #1713584: Implement hook_library to allow other modules to use the library without using the UI. and you'd be able to use '#attached' to add Chosen to form api elements without having to include the library globally.

I set the default to work as it currently does, but you can turn off the include and do so as required. Giving you at least part of what #6 was discussing.

The patches are independent, which means we'd need to adjust one or the other to have them work together. I can create those if required.

shadcn’s picture