Introduce jquery formvalidator for com_languages

[dgrammatiko]

Executive summary

This PR converts the form validation on com_languages to use plain jquery (no mootools call on every form).
Also NO MORE INLINE SCRIPTS!

Testing

1. Apply first PR Reduce unneeded calls for mootools #4888 (!important)
2. Apply this PR
3. In the admin area go to com_languages and try to submit any form.

If no javascript errors are logged in your browser and the functionality remains the same your test is a pass in any other case please report the errors here

Please also check these:
administrator/index.php?option=com_checkin should demonstrate multiselect without mt
administrator/index.php?option=com_users&view=mail should demonstrate form sent and validate without mt
administrator/index.php?option=com_modules should demonstrate multiselect and combobox without mt
http://localhost/administrator/index.php?option=com_admin&view=sysinfo should demonstrate highlighter.js without mt
Logout and log in to demonstrate the use of noframes without mt.

[smanzi]

@DGT41
Buttons (Save -> Cancel) inactive in &view=override&layout=edit
OK in &view=language&layout=edit

[dgrammatiko]

@smanzi It should be ok ok now!

[smanzi]

@DGT41 Travis is crying... should I test anyway?

[dgrammatiko]

@smanzi Travis hasn’t finished yet but he should be ok with this change i made! Go ahead!

[smanzi]

@test success

[anibalsanchez]

@test success

[roland-d]

Moving to RTC as we have 2 successful tests.

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5046.

[dgrammatiko]

@phproberto can you review this https://github.com/dgt41/joomla-cms/commit/82b6a29b13b717f2684b37c3839648e0bbf449ec

[anibalsanchez]

Creating an Override, it is OK. However, when the same page is accessed after saving, it loads the Override and generates this error:

SyntaxError: syntax error
var expired = ;
index.p...SERNAME (line 37, col 18)

The error is coming from here:

$expired = ($this->state->get("cache_expired") == 1 ) ? '1' : '';

var expired = ' . $expired . ';

[dgrammatiko]

@anibalsanchez stupid mistake (missing quotes), I pushed a correction, sorry about that 😞

[anibalsanchez]

@test success!

[Fedik]

here also, you wrapped the function to ready, but why? 😉
it unnecessary, there no reason to wait when DOM is ready, for define a function

[dgrammatiko]

@Fedik you are right is not needed, but waiting for the DOM to become ready ensures that scripts will not block the DOM, which is described as best practice (is there a chance to call the function before the DOM is completed?) and will be nice if devs follow it.
Also a good reference for javascript code styling is this post in CMS Group
Of course somebody has to update the coding styles

[Fedik]

oh oh 😃
I have a doubt that writing function definition in jQuery ready() method can be a good practice, ( by link that you suggested I did not found any about it)
And I know only couple reasons when it can be not bad idea, but the current code do not related to any of these reasons 😄
Also this part of the code use nothing from jQuery, and can work very good without it 😉

And how it can block DOM? it do nothing with DOM, it just a definition.
If you do like you do, then JS parser parse jQuery.ready and your code in it, and execute jQuery.ready() for add function definition to waiting the ready event. Then on ready event jQuery execute your function definition.
If you do as it was before, without jQuery, then JS parser parse your code and execute your function definition.

So, you still think that it will be better if all start make such thing? 😄

[dgrammatiko]

@Fedik This is indeed, as i said before, unneeded. The reason that I put it there was just to unify the script code. So when devs tries to bring their code to 3.4 (or whenever) they see a unique pattern on the core and just follow it.
What is very interesting for admin area is that if you take a 3.3.6 and apply all those PRs, for all pages (except the edit ones where you have a modal) the page loading time decreases by 20-30%. 🎉
Just to be clear here: if people think that this is a bad practice, I can revert this code to be plain js, no prob! ❔

[Fedik]

I think, the page loading/rendering time decreases because there no mootools anymore, and for the browser do not need to parse/execute mootools library 😉

[dgrammatiko]

@Fedik Of course you are right it is the absence of a big javascript library file that reduces the loading times. I didn’t implied that this was because of the .ready() function 😉

[phproberto]

I was merging your PRs but I totally agree with @Fedik here:

• Functions do not require to wait until DOM is ready
• We are adding here a jQuery dependency not needed

We should try to get as much code in plain JS as possible. I just checked it and we have some jQuery code inside core.js (and it makes jQuery required) but that's an error IMO. Probably we won't need to remove jQuery until v4.0 (and maybe then we keep it) but we should keep it clean and independent.

In fact we officially support Mootools and jQuery and using jQuery on core makes it mandatory for everybody.

[dgrammatiko]

@phproberto You want me to remove all the jQuery(document).ready(…);
from all the functions not mandatory to have it?
Can do that! gimme some time...

[phproberto]

The most important part is not to tie core to other libraries and do not transmit that to other developers as something recommended.

In our code we are loading jQuery and that's done in our own views so that wouldn't be a big problem. But yes, I'd like to see core not mixed with jQuery.

I've created an issue for the existing jQuery code in core.js #5254

[dgrammatiko]

Check #5039 #5041

[dgrammatiko]

Dammit for the closed ones the changes don’t come thru 😡

[phproberto]

Yes they are now merged so we need a new PR for that :(

[smanzi]

#5113 is front-end (will test)
#5047 (not in your list) cannot be applied:

The patch could not be applied because it conflicts with a previously applied patch: administrator/components/com_menus/views/item/tmpl/edit_modules.php

[dgrammatiko]

Thnks Sergio, #5047 is not touched so no need to test it again and the #5113 is front end (create article)

[smanzi]

@test success for this PR
Tested the set of: #5046 #5048 #5049 #5050 #5051 #5052 #5053 #5054 #5056 #5058 #5113 #5255

[smanzi]

@phproberto This is still open...

[infograf768]

Folks, I had to commit cd70e94

as editing or creating an override was broken

[smanzi]

@infograf768 You're right! Sorry, I didn't test that view; I tried installing a new language, but that is not com_languages but com_installer 😶

Is it normal that when I first try to install a new language I find the available languages list populated only by the following?

Albanian
Bahasa Indonesia
Bosnian
EnglishCA
Finnish
FrenchCA
Montenegrin
Portuguese Brazil
Serbian Cyrillic
Serbian Latin
Sinhala
Spanish
Swahili
Tamil
Thai
Turkish
Ukrainian
Uyghur
Vietnamese
Welsh

I find it confusing... I would expect either all available languages or none at all...

[infograf768]

Please click on "Find languages". It solves this issue. No idea why we get a limited list. I even get a pkg_weblinks.xml sometimes in that list.

[smanzi]

@infograf768 yep, I know I have to click "Find languages" but I find weird to have the languages list "somehow" populated...

[smanzi]

Should I open an issue for this?

[dgrammatiko]

@smanzi I am on it...

[smanzi]

OMG, you are on... EVERYTHING! 😄

[dgrammatiko]

@smanzi actually this is my bad...

[smanzi]

@DGT41 BTW, I have another that it is surely for you: get in touch on Skype when you have a second...

[dgrammatiko]

I am on Skype

[dgrammatiko]

@smanzi Actually my previous comment wasn’t supposed to be here. Of course I am not on this one, so go on and open an issue… 😄

[smanzi]

ah, OK!! 😄