? Success

User tests: Successful: Unsuccessful:

avatar Achal-Aggarwal
Achal-Aggarwal
23 Oct 2013
avatar Achal-Aggarwal Achal-Aggarwal - open - 23 Oct 2013
avatar betweenbrain
betweenbrain - comment - 23 Oct 2013
avatar Achal-Aggarwal
Achal-Aggarwal - comment - 23 Oct 2013

Yeah, it solves that issue and if administrator feels performance issues then he can try different plugins for select list.

avatar Achal-Aggarwal
Achal-Aggarwal - comment - 23 Oct 2013

@betweenbrain Added that option to set custom selector. If you leave it blank than it will apply to all select and if you specify anything else it will be treated as a class and the selected plugins will be applied to anything having that class.
We can reverse its action to not to apply on the class mentioned in it.

avatar betweenbrain
betweenbrain - comment - 23 Oct 2013

@Achal-Aggarwal Nice approach. I like the flexibility this creates. I think that it will be important that the default behavior is to apply Chosen to existing select elements by using the advancedSelect class.

avatar betweenbrain
betweenbrain - comment - 23 Oct 2013

I did test this PR and it seems to function as intended, but existing <select> elements need a default class to target to maintain existing behavior.

avatar Bakual
Bakual - comment - 23 Oct 2013

This approach would solve the issue by allowing to disable chosen in core on a template level.

However it doesn't allow to mark specific selects where chosen should be applied or not as the fields still have no classes applied which could be used. Thus it currently has to be applied to all selects like today. I think this was an issue @betweenbrain raised which would still not be fixed with this.
Imho the proper approach is to use the default behavior of our JHtml::_(formbehavior.chosen) class which would apply chosen to the advancedSelect class. Then we could disable chosen for the places where it causes troubles and leave it enabled where it isn't an issue and provides a real value.

Some personal thoughts:

  • Personally I wouldn't add the selector as a parameter. I don't see where this could be useful. Especially since the selects currently have no classes applied.
  • I don't want to see select2 in core. Either chosen or select2, but not both. I think it will create a nightmare for extension developers who want to interact with a select. As an example I'm updating a select with a newly saved value without reloading the page. I already have to do a check if chosen is loaded and update it when necessary. If there is the possibility of select2 loaded, I have to do another check for that as well. It just makes things more complicate without adding a real value.
  • You need to add specific CSS rules to Isis (and maybe 3rd party extensions) for select2 as well. Given that the ones for chosen still have issues (RTL, input-append), I wouldn't add a second option.
avatar Fedik
Fedik - comment - 23 Oct 2013

apply chosen.js or select2.js to all <select> it is bad idea.
similar suggestion already closed :)
check #1302

avatar Achal-Aggarwal
Achal-Aggarwal - comment - 23 Oct 2013

okay, I agree that there will be conflicts if we apply to all select list. But I am not in favor of adding advancedSelect to all select list present in core extensions. The problem is chosen doesn't handle large list atleast not on opera so falling back to browser default and removing all the good looks of chosen is not a solution. Because of a single extension the whole backend will work with browser default select. Either we find out why chosen is not handling the work load and correct it or use some other plugin who does or give an option on extension level to disable it.

What if we go with PR #2018 and any 3rd party extension is using chosen also and populating select list extensively, then our purpose defeats of increasing performance. Than it would become the responsibility of the extension developer to choose another plugin or give an option to disable it.

I am closing this PR for obvious reasons and will focus on debugging of chosen.

avatar Achal-Aggarwal Achal-Aggarwal - close - 23 Oct 2013
avatar Bakual
Bakual - comment - 24 Oct 2013

I'd say first try updating chosen to v0.14.0 (the latest without namespace) which according to their release notes has performance improvements in it.
@Fedik did a PR already: #2228

If that solves the problem, it will be the easiest solution to the performance issue.
It then also allows to "Un-Chosen" the selects which may help with other issues as well.

Add a Comment

Login with GitHub to post a comment