?
avatar andrepereiradasilva
andrepereiradasilva
9 Nov 2016

Steps to reproduce the issue

There are some changes in the permissions fields that IMHO should be reverted.
Now it allows to search (??) for the permission... and there is no ok/loading/error icon when chosen.
image

Expected result

Permissions select field like in 3.6.4

image

Actual result

Different.

System information (as much as possible)

Latest

Additional comments

None.

avatar andrepereiradasilva andrepereiradasilva - open - 9 Nov 2016
avatar joomla-cms-bot joomla-cms-bot - change - 9 Nov 2016
Labels Added: ?
avatar infograf768
infograf768 - comment - 9 Nov 2016

I don't have the search button here.

avatar dgt41
dgt41 - comment - 9 Nov 2016

@andrepereiradasilva I had the same problem in the calendar with chosen being applied on select tags. So just add element.setAttribute('data-chosen', true); // avoid Chosen, hack in the relevant element

avatar mbabker
mbabker - comment - 9 Nov 2016

If you need that hack it seems like there's an overreaching selector in use with the function loading Chosen.

avatar andrepereiradasilva
andrepereiradasilva - comment - 9 Nov 2016

i don't have time to check this one, that's why i created an issue. Leave this one for others.

But i essencially agree with @mbabker

avatar dgt41
dgt41 - comment - 9 Nov 2016

@mbabker the problem is that chosen in many views doesn't target specific classes instead is targeting all select elements...

avatar mbabker
mbabker - comment - 9 Nov 2016

And again, it's an overreaching selector because someone got the bad idea that every select list has to use Chosen so instead of it being an opt-in feature it's now something you have to opt-out of using hacks and other bad code practices.

avatar phproberto
phproberto - comment - 9 Nov 2016

I have a basic JS script to remove chosen for specific fields. Far from perfect but better than the current nightmare:

(function($) {
    'use strict';
    $.fn.chosenDestroy = function () {
        var $element = $(this);
        var timer;

        if (typeof $.fn.chosen === 'undefined') {
            return $(this);
        }

        timer = setTimeout(function(){
            if (typeof $element.data('chosen') === 'object') {
                $element.chosen('destroy').removeClass('chzn-done').show();

                clearTimeout(timer);
            }

        }, 100);

        return $(this);
    }
})(jQuery);

Example to ensure that no chosen is loaded for a field before using select2:

(function ($) {
    $(document).ready(function () {
        $('#my-field').chosenDestroy().select2();
    });
})(jQuery);
avatar dgt41
dgt41 - comment - 9 Nov 2016

For J4 the idea is to totally drop chosen (will still be available but won't be used by joomla).
There are only 3 areas that chosen was actually needed

  • create category on the fly
  • create module position
  • add new tag

and these will be using another (vanilla) script.
But this issue gave me one idea: forbid chosen to be applied on all select elements of the page

avatar Bakual
Bakual - comment - 9 Nov 2016

The original idea of chosen was fine.
The method to load chosen has a default value which would only trigger if the field has the class advancedSelect. So it was completely opt-in and not implementation specific.

The only thing which was done wrong was to call the method with the selector select which applies to all selects on the page. That's imho the only thing we need to change with J4.
But if the UI guys say they want to have native selects, I'm fine with that as well.

avatar dgt41
dgt41 - comment - 9 Nov 2016

@Bakual you can't have native select for the 3 cases I mentioned above. Also chosen is kinda outdated (see their repo) and requires jQuery so for J4 and those cases another native script will replace chosen. (behavior.chosen will still be around for B/C, but core will never use it again)

avatar Fedik
Fedik - comment - 9 Nov 2016

it already was removed once #6317 ?
who made it back? ?

avatar andrepereiradasilva
andrepereiradasilva - comment - 9 Nov 2016

the question of this issue is not the use of chosen (chosen is used in 3.6.4 too) ...

the question in this issue is:

  • Why the search field in the select chosen boxes now?
  • Where are the icons success/error/loading ajax calls?
avatar Bakual
Bakual - comment - 9 Nov 2016

you can't have native select for the 3 cases I mentioned above. Also chosen is kinda outdated

I'm well aware of that. And if we used the advancedSelect class we could just replace with select2 or any other shiny library without any issues.
Also chosen (or any other similar library) is very nice when it comes to big lists. The filtering feature comes in handy there.

requires jQuery

No issue for me at all.

nother native script will replace chosen

I hope it's not a solution we invent ourself. That would be a horrible decision.

Why the search field in the select chosen boxes now?

That's a bug that needs to be solved. I guess it relates to the chosen update we have done somewhere.
The search field used to appear only when we had more than 10 (?) items. Not it appears always. That's not only the case in permissions but in any select (eg also state, language, access, ...).

Where are the icons success/error/loading ajax calls?

It may be related to the fact that the permission tab now loads chosen as well, which it didn't do for 3.6.4. My guess is that is related to the chosen update as well.

avatar dgt41
dgt41 - comment - 9 Nov 2016

I hope it's not a solution we invent ourself. That would be a horrible decision.

There are plenty of such scripts out there, so writing another one won't be beneficial for nobody, so yes it's not a Joomla script and it WON'T be hacked in anyway to fit joomla's needs

avatar mbabker
mbabker - comment - 9 Nov 2016

it WON'T be hacked in anyway to fit joomla's needs

I give that statement a lifetime of until 4.1's release ?

avatar dgt41
dgt41 - comment - 9 Nov 2016

@mbabker actually with that vendor and grunt thing, someone will have to fork a script to do that (I hope will not happen)

avatar andrepereiradasilva
andrepereiradasilva - comment - 9 Nov 2016

vendor dir = do not touch! +1

avatar dgt41
dgt41 - comment - 9 Nov 2016

@andrepereiradasilva back to the issue @Fedik posted the PR above which got data-chosen="true" in the select element and is the solution you are looking

avatar Fedik
Fedik - comment - 10 Nov 2016

I will redo that PR, later these days, if no one will be faster ?

avatar Bakual
Bakual - comment - 10 Nov 2016

The data-chosen="true" is still there. It's the part in chosen which got removed I think.

avatar Fedik
Fedik - comment - 10 Nov 2016

hm, I see now, it the result of Chosen upgrade,
now they explicitly check for Chosen instance

...
chosen = $this.data('chosen');
...
if (!(chosen instanceof Chosen)) {
    $this.data('chosen', new Chosen(this, options));
}

hard ?

upd:
well, I hope no one will mind if I revert this couple lines of Chosen ?
it was good workaround for Joomla! while we use Chosen in wrong way JHtml::_('formbehavior.chosen', 'select')

avatar Bakual
Bakual - comment - 10 Nov 2016

The change with the search field now appearing is caused by those PRs (merged for 3.6.0):
#8623
#8635
#8634
#8632

They changed the call to chosen like this
-JHtml::('formbehavior.chosen', 'select');
+JHtml::
('formbehavior.chosen', 'select', null, array('disable_search_threshold' => 0 ));

This obviously isn't an ideal change. It means he enabled the search feature for all fields, but he only needs it for one single field (the category one).

avatar Bakual
Bakual - comment - 10 Nov 2016

I did a PR to remove the searchboxes again: #12855

I leave this issue open since it's only a partial solution.
Chosen probably still needs to be disabled (again) on the permission fieldset.

avatar brianteeman brianteeman - change - 10 Nov 2016
Status New Confirmed
avatar brianteeman brianteeman - change - 11 Nov 2016
Category com_fields
avatar andrepereiradasilva
andrepereiradasilva - comment - 12 Nov 2016

Closing this as we have two PR to solve this:
1. The searchbox in chosen #12855
2. The loading/ok/error/images #12874

Please test both

avatar andrepereiradasilva andrepereiradasilva - change - 12 Nov 2016
Status Confirmed Closed
Closed_Date 0000-00-00 00:00:00 2016-11-12 12:03:57
Closed_By andrepereiradasilva
avatar andrepereiradasilva andrepereiradasilva - close - 12 Nov 2016
avatar andrepereiradasilva andrepereiradasilva - close - 12 Nov 2016
avatar brianteeman
brianteeman - comment - 12 Nov 2016

Thank you

avatar Fedik
Fedik - comment - 13 Nov 2016

@andrepereiradasilva check this one #12877 it also fix the icons

Add a Comment

Login with GitHub to post a comment