[#29641] Very slow of JavaScript performance in Isis... (2)

[Fedik]

previous pull #1302
I made some updates based on @Bakual idea about apply chosen only for .advancedSelect
so here I removed JHtml::_('formbehavior.chosen', 'select'); from all tmpl/, and added options to the template for allow to enable/disable JHtml::_('formbehavior.chosen'); ... that will work only for <select> with class .advancedSelect

and I made new pull because previous have conflicts....

Links:

• original pull [#29641] Very slow of JavaScript performance in Isis...  #1302
• [#29641] Very slow of JavaScript performance in Isis in Opera
• chosen.js is the cause of the low backend performance (in google group)

[mbabker]

Please restore the JHtml::_('formbehavior.chosen', 'select'); calls in the front end component views. Those should be handled in a separate patch if you intend on changing Protostar's behavior.

[Fedik]

ah, yes, you are right ... I missed this

[Bakual]

This one will essentially remove chosen from the whole backend as currently, the class "advancedSelect" isn't used anywhere. If you're going to change the call from JHtml::_('formbehavior.chosen', 'select'); to JHtml::_('formbehavior.chosen'); (which I support!) you need to add the advancedSelect class to each select which should be supported by chosen (currently that's each select present). Otherwise you could as well just remove chosen alltogether.

[Fedik]

@Bakual that was the first part of the plan, delete all :)
add advancedSelect it other part of the plan

[Bakual]

@Fedik The second part is by far the harder part ;-)

What about having two parameters then, one for back- and one for front-end. This issue isn't necessarily exclusive to the back-end as far as I know.

@betweenbrain Since he did it as a template parameter, the frontend would have to be taken care by the frontend template. But here it would be a backward compatibility break for all existing templates which would have to be updated to include the chosen call if they wish to support it. Realistically speaking I don't think that is going to be accepted for the frontend.

[Fedik]

@infograf768 have a more details? cause here no any changes for /installation

[infograf768]

@Fedik
[... other errors...] [21-Dec-2013 09:27:58 UTC] PHP Warning: simplexml_load_file() [<a href='function.simplexml-load-file'>function.simplexml-load-file</a>]: /Applications/MAMP/htdocs/stagingcms/modules/mod_articles_news/mod_articles_news.xml:76: parser error : Attribute class redefined in /Applications/MAMP/htdocs/stagingcms/libraries/cms/installer/installer.php on line 1929 [21-Dec-2013 09:27:58 UTC] PHP Warning: simplexml_load_file() [<a href='function.simplexml-load-file'>function.simplexml-load-file</a>]: class=&quot;advancedSelect&quot;&gt; in /Applications/MAMP/htdocs/stagingcms/libraries/cms/installer/installer.php on line 1929 [21-Dec-2013 09:27:58 UTC] PHP Warning: simplexml_load_file() [<a href='function.simplexml-load-file'>function.simplexml-load-file</a>]: ^ in /Applications/MAMP/htdocs/stagingcms/libraries/cms/installer/installer.php on line 1929 [21-Dec-2013 09:27:58 UTC] PHP Warning: simplexml_load_file() [<a href='function.simplexml-load-file'>function.simplexml-load-file</a>]: /Applications/MAMP/htdocs/stagingcms/modules/mod_articles_news/mod_articles_news.xml:76: parser error : Attribute class redefined in /Applications/MAMP/htdocs/stagingcms/libraries/cms/installer/installer.php on line 2141 [21-Dec-2013 09:27:58 UTC] PHP Warning: simplexml_load_file() [<a href='function.simplexml-load-file'>function.simplexml-load-file</a>]: class=&quot;advancedSelect&quot;&gt; in /Applications/MAMP/htdocs/stagingcms/libraries/cms/installer/installer.php on line 2141 [21-Dec-2013 09:27:58 UTC] PHP Warning: simplexml_load_file() [<a href='function.simplexml-load-file'>function.simplexml-load-file</a>]: ^ in /Applications/MAMP/htdocs/stagingcms/libraries/cms/installer/installer.php on line 2141

[Bakual]

Sounds like an error in an XML file to me.

[Fedik]

yes, looks like the class duplication in mod_articles_news.xml .. I will check more

[infograf768]

we have the error in 2 modules:
breadcrumbs and article_news where we have twice a class declaration.
But anyway, after a discussion in the maintainers chat, it is judged that this can't be merged in a 3.2.x release for B/C reasons

[Fedik]

ok, duplication fixed ... think it happened during one of the sync

@infograf768 whether was offered some alternative?

[Bakual]

@Fedik A solution that was thrown into the room was using JLayouts instead to render the select boxes. And then use overrides for different uses.
That probably would work also for 3rd parties without issues. Since the last iteration of JLayouts, I think could even selectively override a layout for a single component or view. But I may be wrong here. @phproberto would know better and I think he wrote a good post somewhere how this works.

[wilsonge]

http://docs.joomla.org/Layout_Overrides_in_Joomla Roberto's JLayout article is also on the JDocs of course :)

[Bakual]

@wilsonge That's not the one about JLayouts 😄
I think http://docs.joomla.org/J3.2:Sharing_layouts_across_views_or_extensions_with_JLayout and http://docs.joomla.org/J3.2:JLayout_Improvements_for_Joomla! is what I meant.

[betweenbrain]

@Bakual can you explain how I could, for example, create a module and ship it so Chosen.js isn't applied to it?

[betweenbrain]

But anyway, after a discussion in the maintainers chat, it is judged that this can't be merged in a 3.2.x release for B/C reasons

Can we then merge it into a 3.3.0 branch then? There is supposed to be one created for exactly these types of situations.

[Bakual]

@betweenbrain I would have to look into JLayout again, but I think I read you can specify layouts "overrides" for specific extensions. Also I think Roberto has a PR open which would allow to specify an alternate layout for a field in the XML. This one would probably solve your issue with the module as well.

[betweenbrain]

@Bakual to be honest, I'm not sure if I agree with the idea of using JLayouts and overrides to handle this, but I am open to exploring it as I may not fully understand the implementation for solving the issue.

I do feel that the current approach of using a class like advancedSelect is a very valid way to solve this. Chosen.js should be opt-in, not opt-out.

[Bakual]

I do feel that the current approach of using a class like advancedSelect is a very valid way to solve this. Chosen.js should be opt-in, not opt-out.

@betweenbrain I agree that we MUST change the calls from JHtml::('bevahior.chosen', 'select') to JHtml::('bevahior.chosen'). It's currently just wrong and disables every attemp to customise anything, even when using JLayouts.
The question imho is only how to add the class. It could be done in the JLayout which would allow it to be overriden by templates and extensions.
So JLayout is not the solution itself, but it may help.

[betweenbrain]

Would it be possible to JLayout, within the template, to enable or disable
chosen as well as to set the class the extension needs to use for chosen to
target it? That way, template devs can change the call for chosen,
including globally targeting all select elements or changing the class, and
extensions can opt-in by just adding a class to their select elements?

Best,

Matt Thomas
@betweenbrain
http://matt-thomas.me/
http://betweenbrain.com/
https://github.com/betweenbrain

Sent from mobile. Please pardon any typos or brevity.
On Dec 21, 2013 9:56 AM, "Thomas Hunziker" [email protected] wrote:

I do feel that the current approach of using a class like advancedSelect
is a very valid way to solve this. Chosen.js should be opt-in, not opt-out.

@betweenbrain https://github.com/betweenbrain I agree that we MUST
change the calls from JHtml::('bevahior.chosen', 'select') to
JHtml::('bevahior.chosen'). It's currently just wrong and disables every
attemp to customise anything, even when using JLayouts.
The question imho is only how to add the class. It could be done in the
JLayout which would allow it to be overriden by templates and extensions.
So JLayout is not the solution itself, but it may help.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2018#issuecomment-31064812
.

[Bakual]

I don't know what exactly is possible with JLayouts. That is something Roberto would need to answer.

[mbabker]

Ask and ye shall receive. 3.3-dev branch open for business.

On Sat, Dec 21, 2013 at 7:07 AM, Matt Thomas [email protected]:

But anyway, after a discussion in the maintainers chat, it is judged that
this can't be merged in a 3.2.x release for B/C reasons

Can we then merge it into a 3.3.0 branch then? There is supposed to be one
created for exactly these types of situations.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2018#issuecomment-31062919
.

[Fedik]

as I understand, with JLayouts it should looks like:

• remove JHtml::('bevahior.chosen', 'select'), and put JHtml::('bevahior.chosen') in the template
• make the layout for each <select> field input type: list, category and so on
• overwrite these layouts in the template, add there class advancedSelect

[Bakual]

• remove JHtml::('bevahior.chosen', 'select'), and put JHtml::('bevahior.chosen') in the template
• make layout for each field input type: list, category and so on overwrite these layouts in the template, add there class advancedSelect If we make changes to the fields (like adding a JLayout), it will always be for frontend and backend. I think the advancedSelect class would have to be added on the default layout. And the template can override it if it wants. It could then add its own class and call JHtml with this custom class. One could even add a parameter to enable/disable this custom call. The JHtml call would have to be back at the components level, otherwise it will fail in frontend and for other backend templates, and thus not be full backward compatible. Just an idea. It sure needs more thinking.

[infograf768]

But anyway, after a discussion in the maintainers chat, it is judged that this can't be merged in a 3.2.x release for B/C reasons

Can we then merge it into a 3.3.0 branch then? There is supposed to be one created for exactly these types of situations.

Once there is an agreement on the way to proceed and we are sure we are B/C, sure.

[betweenbrain]

@infograf768 What is the B/C issue? I don't think that has ever been stated.

[betweenbrain]

@infograf768 can you tell us what the B/C is?

[phproberto]

I'm going to throw this on the frontenders working group to unblock it with their guidance

[brianteeman]

Closed as per the comment in the tracker

[betweenbrain]

@phproberto if you do tackle this with the frontenders group, please also see #3721