? Success

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
12 Oct 2016

Pull Request for Issue #NULL.

Summary of Changes

In almost all singular edit views (forms) joomla needs some javascript which is injected from php, e.g.:

JFactory::getDocument()->addScriptDeclaration('
    Joomla.submitbutton = function(task)
    {
        if (task == "profile.cancel" || document.formvalidator.isValid(document.getElementById("profile-form")))
        {
            Joomla.submitform(task, document.getElementById("profile-form"));
        }
    };
');

This PR is beefing up the already existing function Joomla.submitbutton in media/system/js/core.js to incorporate the most common cases.
How it works:
In any form that uses validation (JHtml::_('behavior.formvalidator');) an additional class named js-submit-button will reproduce the code above.

Extra functionality:

  • data attribute data-cancel="view.cancel" for forms that have a task different than view.cancel
  • data attribute data-skip-permissions="true" if you don't want to process permissions validation (accelerates the process)
  • data attribute data-permissions-selector="#permissions-sliders" specify the id of the permissions, if omitted and data-skip-permissions is set to true then #permissions-sliders is gonna be used

Then magic happens (not really...)

Testing Instructions

Try all the views that are changed by this PR. Check the files tab to see which ones are modified.

Documentation Changes Required

This behaviour needs to be documented (where?)

B/C

This SHOULD BE 100% B/C

Performance

Less is more...

@Fedik @andrepereiradasilva @mbabker

7332f3a 12 Oct 2016 avatar dgrammatiko init
avatar dgt41 dgt41 - open - 12 Oct 2016
avatar dgt41 dgt41 - change - 12 Oct 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2016
Labels Added: ?
avatar dgt41 dgt41 - edited - 12 Oct 2016
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2016
Category Administration Components Tags Front End JavaScript
avatar dgt41 dgt41 - change - 12 Oct 2016
The description was changed
avatar dgt41 dgt41 - edited - 12 Oct 2016
avatar mbabker
mbabker - comment - 12 Oct 2016

I get what you're trying to do but there's a part of me that feels like adding all these data attributes starts making the markup more complex and almost feels like we're building an Angular based app in PHP. Especially when we start cramming executable script snippets into those attributes.

avatar dgt41
dgt41 - comment - 12 Oct 2016

@mbabker it's all about separation of concerns, php shouldn't build javascript in the first place IMHO.

Especially when we start cramming executable script snippets into those attributes.

I guess you mean these ones: $this->form->getField('articletext')->save()
this is a callback to editor's onSave function, for none is null for tinyMCE is:

    /**
     * TinyMCE WYSIWYG Editor - copy editor content to form field
     *
     * @param   string  $editor  The name of the editor
     *
     * @return  string
     */
    public function onSave($editor)
    {
        return 'if (tinyMCE.get("' . $editor . '").isHidden()) {tinyMCE.get("' . $editor . '").show()};';
    }

I don't think this is some major or minor drawback, somehow we need to pass a (simple) callback.

avatar mbabker
mbabker - comment - 12 Oct 2016

Well if you really want to be a separationalist, PHP shouldn't be generating most of what we use it to generate today. But with a platform like this one, sometimes you have to trade off "pure" for ease of use.

I'm not saying it's a bad move necessarily, but to me it really feels like some of what you're trying to do is pushing the layout layer of our app stack more toward how JavaScript frontends are built. And honestly I find those confusing as all hell and excessively complex.

avatar dgt41
dgt41 - comment - 12 Oct 2016

@mbabker we just provide a workflow based on data attributes, but this IS NOT the only way. The old way still works as expected, check: https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_menus/views/item/tmpl/edit.php#L50-L76

So it's down to developer to use whatever they feel more familiar with. We are not dictating anything here, people are still free to choose

avatar mbabker
mbabker - comment - 12 Oct 2016

I get that. But honestly there are two issues.

  1. Our documentation of anything outside of pull requests sucks.
  2. People tend to copy what core is doing so we need to ensure it's all understandable, goto 1 to see why that's probably going to be an issue.
avatar dgt41
dgt41 - comment - 12 Oct 2016

This behaviour needs to be documented (where?)

I've also pointed the documentation part. The (where?) is also referring to what you've pointed out above:

There isn't any documentation on how to build a form:

screen shot 2016-10-12 at 18 40 14

avatar mbabker
mbabker - comment - 12 Oct 2016

Well, that's in part why we started just dumping docs in the Framework repos. At least there's a place to find the stuff for the repos we've started working on.

avatar Fedik
Fedik - comment - 12 Oct 2016

to be honest I doubt that we need it now,
this part of Joomla works fine ?
I would not touch this part for Joomla 3.x

avatar brianteeman
brianteeman - comment - 12 Oct 2016

If it's not broken don't fix it - perhaps

avatar andrepereiradasilva
andrepereiradasilva - comment - 12 Oct 2016

i think @dgt41 is trying to improve is to remove render blocking js
See https://developers.google.com/speed/docs/insights/BlockingJS

avatar dgt41
dgt41 - comment - 12 Oct 2016

I guess this piece of crap that we have in each and every page doesn't need some fixing:
screen shot 2016-10-12 at 19 15 56

Also this IS BROKEN according to https://www.w3.org/TR/CSP/

avatar Fedik
Fedik - comment - 12 Oct 2016

@dgt41 I am about the logic change for Joomla.submitbutton ?

It is fine to move an inline code in to admin-banners-edit.js - so we can have a place where we can put "layout related" JavaScript.
But I doubt we need something more here, for now.

avatar dgt41
dgt41 - comment - 12 Oct 2016

@Fedik I was about to do that, but then I realised that there is a common pattern for many (28 views) so I thought why don't I just enhance the Joomla.submitbutton to handle the most common cases. That's how this PR was created!
The benefits are:

  • less js files
  • less repeated code

We still NEED to get the rest of the inline scripts in some js files.
Applying this PR and #12371 and #11922 most of the inline scripts are already eliminated.

I really don't understand the opposition here, as this is optional the old way is still intact...

avatar mbabker
mbabker - comment - 12 Oct 2016

I'm not totally opposed, just voicing an opinion that there's a part of me thinking this is more complex and with our overall lack of documentation (you mostly have to reverse engineer pull requests or unit tests if any exist to figure out how to use parts of the API) it won't be clear what you can do and how.

I've done a lot of work in my own projects to shove as much JavaScript into separate files as possible, but there are still some places where I just can't get around it because the logic generating the scripts is too complex as is or is too coupled to data that's only available on the PHP side of the app.

avatar dgt41
dgt41 - comment - 12 Oct 2016

@mbabker you/we are already doing similar things since bootstrap integrated to Joomla.
As en example an alert box from bootstrap 2.3.2:

<div class="alert fade in">
            <button type="button" class="close" data-dismiss="alert">×</button>
            <strong>Holy guacamole!</strong> Best check yo self, you're not looking too good.
          </div>
  • Class dictates some js behaviour (fade in)
  • Data attribute dismiss dictates another js behaviour (close)

So, all and all, I am not introducing something totally new here.
Also, I guess, this is common (modern) practice all over the internet apps.

I could close this and introduce 56 js files (.js and .min.js) but I find that totally stupid ?

avatar mbabker
mbabker - comment - 12 Oct 2016

I know Bootstrap does it, but it's a lot less than what you're doing here. Maybe it's just seeing the script snippets in there that irks me. If you saw the app I've been working on this year you'd see I've got a boatload of data attributes and some very badly written JavaScript (I always said I should stick to PHP ? )

And yes, adding that many files is totally stupid.

avatar dgt41 dgt41 - edited - 12 Oct 2016
avatar dgt41
dgt41 - comment - 20 Oct 2016

I've removed the data attributes that contained javascript code. So by just having a class js-submit-button is all it's needed in most cases.
Extra functionality:

  • data attribute data-cancel="view.cancel" for forms that have a task different than view.cancel
  • data attribute data-skip-permissions="true" if you don't want to process permissions validation (accelerates the process)
  • data attribute data-permissions-selector="#permissions-sliders" specify the id of the permissions, if omitted and data-skip-permissions is set to true then #permissions-sliders is gonna be used
  • before and after save javascript code can be set by:
// Pass some PHP created script to javascipt
JFactory::getDocument()->addScriptOptions(
    'form',
    array(
        'beforeSave' => htmlentities($this->form->getField("description")->save(), ENT_COMPAT, 'UTF-8'),
        'afterSave' => htmlentities($afterSave, ENT_COMPAT, 'UTF-8'),

    )
);

@mbabker what do you think now?

avatar dgt41 dgt41 - change - 20 Oct 2016
The description was changed
avatar dgt41 dgt41 - edited - 20 Oct 2016
avatar dgt41 dgt41 - change - 20 Oct 2016
Title
[RFC] Reduce inline scripts
Reduce inline scripts
avatar dgt41 dgt41 - change - 20 Oct 2016
Title
[RFC] Reduce inline scripts
Reduce inline scripts
avatar dgt41 dgt41 - edited - 20 Oct 2016
avatar brianteeman brianteeman - change - 29 Oct 2016
Labels Added: ?
Removed: ?
avatar dgt41 dgt41 - change - 14 Nov 2016
The description was changed
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-11-14 11:42:34
Closed_By dgt41
avatar dgt41 dgt41 - close - 14 Nov 2016
avatar dgt41 dgt41 - close - 14 Nov 2016
avatar joomla-cms-bot joomla-cms-bot - change - 14 Nov 2016
Category Administration Components com_tags Front End JavaScript Administration com_admin com_banners com_categories com_config com_contact com_content com_finder com_languages com_messages com_newsfeeds com_plugins com_redirect com_tags com_templates com_users Front End JavaScript Components
avatar dgt41 dgt41 - change - 30 Dec 2016
The description was changed
Status Closed Pending
Closed_Date 2016-11-14 11:42:34
Closed_By dgt41
avatar dgt41 dgt41 - change - 30 Dec 2016
The description was changed
avatar dgt41 dgt41 - change - 30 Dec 2016
The description was changed
avatar dgt41 dgt41 - edited - 30 Dec 2016
avatar dgt41 dgt41 - edited - 30 Dec 2016
avatar dgt41 dgt41 - reopen - 30 Dec 2016
avatar dgt41 dgt41 - change - 30 Dec 2016
Title
Reduce inline scripts
Reduce inline scripts
avatar dgt41 dgt41 - reopen - 30 Dec 2016
avatar dgt41 dgt41 - change - 30 Dec 2016
The description was changed
avatar dgt41 dgt41 - edited - 30 Dec 2016
avatar dgt41
dgt41 - comment - 30 Dec 2016

I've simplified this a lot and removed the before/after scripts altogether.
I hope this can make it into core now

avatar ggppdk
ggppdk - comment - 31 Dec 2016

Yes indeed simpified,

but please change data-cancel attribute of the FORM tag
to be data-cancel-task

It is confusing and one needs to read documentation or the code to understand what it is,
and also it is may cause conflicts with custom scripts

avatar dgt41
dgt41 - comment - 31 Dec 2016

@ggppdk data attribute renamed to data-cancel-task

ba5bbe7 31 Dec 2016 avatar dgrammatiko oops
avatar Fedik
Fedik - comment - 31 Dec 2016

Some thought

data attribute data-cancel="view.cancel" for forms that have a task different than view.cancel

why we need it? ?
I think here need opposite thing data-task-validate="item.save,item.apply"

data attribute data-skip-permissions="true" if you don't want to process permissions validation

I think the permissions hacks should not be part of the core at all,
it can be placed in media/system/js/permissions.js

avatar dgt41
dgt41 - comment - 31 Dec 2016

I think here need opposite thing data-task-validate="item.save,item.apply"

The validation task is given by the button, but the cancel is needed for the com_config where the cancel task is application.congig.cancel

avatar ggppdk
ggppdk - comment - 31 Dec 2016

The validation task is given by the button, but the cancel is needed for the com_config where the cancel task is config.cancel.application

and by configuration of components, which have cancel task: config.cancel.component
and by configuration of modules, which have cancel task: config.cancel.modules
and it maybe need by 3rd party extensions to declare the name of their cancel task

just you missed to update file:
components/com_config/view/modules/tmpl/default.php

it is still data-cancel="..." instead of data-cancel-task="..."

avatar dgt41
dgt41 - comment - 4 Jan 2017

@ggppdk thanks for the review, I've corrected the components/com_config/view/modules/tmpl/default.php
Should be ok now!

avatar Bakual
Bakual - comment - 4 Jan 2017

I don't really see the point of this.
You want to get rid of a few simple inline JS lines which even I understand what they do?
On the other hand you add a CSS class which isn't used for styling but for triggering/enabling magic JS, plus some more undocumented data attributes. This will be a pain for developers to understand and reverse engineer what it does, and in the end we still need the inline scripts because we have some custom stuff to do.

avatar dgt41
dgt41 - comment - 4 Jan 2017

I don't really see the point of this. You want to get rid of a few simple inline JS lines which even I understand what they do?

Inline scripts BLOCK page rendering. That should be a good enough reason to eliminate wherever possible the inline scripts!

This will be a pain for developers to understand and reverse engineer what it does, and in the end we still need the inline scripts because we have some custom stuff to do.

So then we should ditch the Bootstrap scripts as these also use css class /data attributes...
The custom part that exist in most pages basically is an architecture defect, e.g.

			/** @deprecated 4.0  Editors need to pass their content to the textarea before save. **/
			' . $this->form->getField("description")->save() . '

And

			/** @deprecated 4.0  The following js is not needed since __DEPLOY_VERSION__. **/
  			if (task !== "category.apply")
  			{
  				window.parent.jQuery("#categoryEdit' . $this->item->id . 'Modal").modal("hide");
 			}

This CANNOT be solved easily in J3, for the first one check #12561 and the second one will be just removed in J4!

avatar Bakual
Bakual - comment - 4 Jan 2017

Inline scripts BLOCK page rendering. That should be a good enough reason to eliminate wherever possible the inline scripts!

Page rendering time isn't any concern at all when it comes to admin forms. And I doubt you can even measure the delay those few lines of JS will cause (if at all).

So then we should ditch the Bootstrap scripts as these also use css class /data attributes...

I never had to reverse engineer those because Bootstrap has excellent documentation. Which unfortunately can't be said about Joomla own JS things.

The stuff you deprecated is probably fine (if it works), but unrelated to the data attributes and CSS class I mentioned, right?

avatar dgt41
dgt41 - comment - 4 Jan 2017

@Bakual One more reason is that script files can be tested, inline script is way harder to do!

The stuff you deprecated is probably fine (if it works), but unrelated to the data attributes and CSS class I mentioned, right?

The second part was already deprecated I just added the editor part (editor needs to pass their data to the text area before form is submitted, and that SHOULD BE do by the EDITOR)
data-attributes in most cases isn't even needed, the css class is the only NEEDED change here. So we exchange some lines of inline script with a simple css class. Fair trade and also very easy to follow, I think!
This change WILL BE DOCUMENTED, if it gets accepted, but honestly Joomla needs to step up the javascript mess that is all over the place...

avatar Bakual
Bakual - comment - 5 Jan 2017

One more reason is that script files can be tested, inline script is way harder to do!

The script is a 3-liner (7 if you count the braces as well). I doubt we need a test for that.

Fair trade and also very easy to follow,

No, it's hard to follow because you will have no clue what that class is going to do.

This change WILL BE DOCUMENTED

Yeah, even if it will be documented somewhere, chances are low someone is going to find that documentation.

Joomla needs to step up the javascript mess that is all over the place...

I agree JavaScript can be improved in a lot of places. But honestly, that 3-liner really isn't a concern at all. It's a perfect example for a valid inline script. Easy to follow and understand. I really don't see why you want to complicate that part.

avatar dgt41
dgt41 - comment - 5 Jan 2017

I really don't see why you want to complicate that part.

That's your point of view. For me this is way easier to build a form, it's DRY, it's testable, it's B/C and is progress from the ancient technics used all over joomla. But my question to you is: since this affects ONLY part of the core component, why are you against it? I mean you can even still do something like:

<script>
    Joomla.submitbutton = function(task)
    {
        if (task == "profile.cancel" || document.formvalidator.isValid(document.getElementById("profile-form")))
        {
            Joomla.submitform(task, document.getElementById("profile-form"));
        }
    };
</script>

inside the BODY and it will work, even tho we are in 2017!!!

avatar andrepereiradasilva
andrepereiradasilva - comment - 5 Jan 2017

i'm with @dgt41 on this. ? to remove inline scripts.

avatar yvesh
yvesh - comment - 5 Jan 2017

@dgt41 +1 for me! No more inline scripts, no more alerts!

avatar Bakual
Bakual - comment - 5 Jan 2017

For me this is way easier to build a form,

Of course it is, because you wrote that PR. Otherwise it would not be easier because you first have to figure out what the heck goes on there. And it starts getting much more funny now if you ever need to do something custom and figure out how you will have to do that.

But my question to you is: since this affects ONLY part of the core component, why are you against it?

Because I think it is over-engineering stuff.

avatar yvesh
yvesh - comment - 5 Jan 2017

@Bakual @dgt41 The more direct alternative would be, to check if there is a submit button, form with form-validate and the form validator and then always add that default functionality instead of js-submit-button form class at all..

avatar Fedik
Fedik - comment - 14 Jan 2017

My suggestion to leave Joomla.submitbutton in rest until Joomla 4.x.
Current solution works fine for years, I would not touch it for J3.x.

@dgt41 I understood your purpose, but, I think, the solution is not really good, sorry.

In J4 we can do better.
In most cases Joomla.submitbutton used for run the form validation, but if look in to core.js at Joomla.submitform, you will see that it already support browser validation (not fully tested but it is another topic).
So in J4 we can drop Joomla.submitbutton in the layouts, and just change the submit button a bit:
instead of call:

Joomla.submitbutton('article.apply');
Joomla.submitbutton('article.save');
Joomla.submitbutton('article.cancel');

do:

Joomla.submitbutton('article.apply', true); // Joomla.submitbutton(task = 'article.apply', validate = true)
Joomla.submitbutton('article.save', true); // Joomla.submitbutton(task = 'article.save', validate = true)
Joomla.submitbutton('article.cancel');  // Joomla.submitbutton(task = 'article.cancel', validate = false)

And formvalidator from validation.js can be attached to form submit event:

// catch onsubmit 
$(document).on('submit', 'form', function(){
 if (!this.noValidate && !document.formvalidator.isValid(this)) {
   return false; // form not valid
 }
});

I suggest to add the label "Re-evaluate for v4.0", and we will back to this topic later ?

avatar dgt41
dgt41 - comment - 14 Jan 2017

My suggestion to leave Joomla.submitbutton in rest until Joomla 4.x.
Current solution works fine for years, I would not touch it for J3.x.

We are in the same side here on current solution works fine (I'm just abstracting this so we don't have to repeat the same code all over) but I disagree for Joomla 4.x. Let me explain:
So for J4 we can break backwards compatibility but this should be done carefully and only if there is no other way. In 3.x (and previous versions) this code was part of core.js and then a developer could override that functionality by re defining the same function name, cool. With your approach this would be a bit harder to do as someone would have to unregister the event listener for the form submit, this could bring a lot of blaming comments, as the simple override would not work as before...

This can easily be fixed by introducing a new function e.g.

Joomla.formButtonSubmit('task', 'validate', 'skipPermissions');

and retain the old Joomla.submitbutton().

So if that's the case then I guess my approach that keeps intact the old way but also provides a new way that requires less code (by adding a simple class and couple data attributes) is still a good approach as it keeps B/C, is opt in and is dead easy to use.

avatar Fedik
Fedik - comment - 15 Jan 2017

note: with "browser validation" I meant "HTML5 validation" ?

I think you over complicated here, a bit ?

Why do you need data attribute data-cancel="view.cancel"?
To check when NOT to run validation - your answer, right? ?
Well then, look at HTML5 form api, at the form attribute novalidate and property noValidate. And it already supported by Joomla.submitform(). So introduce data-cancel="view.cancel" it just wrong.

So for J4 we can break backwards compatibility but this should be done carefully

My suggestion can work without huge b.c. in J3.x, but as I said, I would not change it in J3.
You can try by yourself. change somwhere Joomla.submitbutton to:

Joomla.submitbutton = function(task) {
  if (task == "blabla.cancel") {
       Joomla.submitform(task, document.getElementById("form-id"), true);
   }
};

It will run HTML5 validation.

And changing the toolbar buttons from onclick="Joomla.submitbutton('article.apply');" to onclick="Joomla.submitbutton('article.apply', true);" (when need validation) introduce zero b.c..
In final it can be:

// Toolbar button:
// onclick="Joomla.submitbutton('article.apply', true);"

// core.js Joomla.submitbutton
Joomla.submitbutton = function(task, validate) {
       Joomla.submitform(task, document.getElementById("admin-form-id"), validate);
};

... someone would have to unregister the event listener for the form submit,

No need to unregistered. Because it will be already controlled by form.noValidate property.
It exactly how HTML5 validation works: each form validated by default on submit, and you need to add novalidate attribute, for disable the validation. Easy ?
Also formvalidator will be used only as fallback (example required HTML5 validation is not supported by Safary) and for provide custom validation handlers.

And in result we do not need new methods, just modify existing a bit.

About the permission, it in wrong place.
It should be part of the permission field. It nothing to do with the core form logic.

avatar dgt41
dgt41 - comment - 16 Jan 2017

@Fedik what will happen to IE8 that doesn't (?) support HTML5 validation?

avatar Fedik
Fedik - comment - 16 Jan 2017

what will happen to IE8 that doesn't (?) support HTML5 validation?

this:

Also formvalidator will be used only as fallback...

But, there a plan to support IE8 in Joomla 4? ?

avatar dgt41
dgt41 - comment - 16 Jan 2017

@Fedik no, but also we shouldn't break all the current forms/fields (I guess, I have to see how severe will be to move to HTML5 native validation). If you have some ideas I would like to hear them...

avatar Fedik
Fedik - comment - 16 Jan 2017

Maybe do you have some particular example?

I think in 90% the validation used only to check whether the required field has value, all else is email,phone etc.
For a very specific case can be used the input patern property, or custom handler by formvalidator.

Do not think that there will be big problems with it.
Only problem who will do it, but it usual problem ?

avatar dgt41
dgt41 - comment - 16 Jan 2017

@Fedik not really there but so far I have something like:

/**
 * @copyright	Copyright (C) 2005 - 2016 Open Source Matters, Inc. All rights reserved.
 * @license		GNU General Public License version 2 or later; see LICENSE.txt
 */

/**
 * Unobtrusive Form Validation library
 *
 * Inspired by: Chris Campbell <www.particletree.com>
 *
 * @since  1.5
 */
var JFormValidator = function() {
	"use strict";
	var handlers, inputEmail, custom,

	    forEach = function(arr, f) {
		    for (var i = 0, e = arr.length; i < e; ++i) f(arr[i], i);
	    },

	    setHandler = function(name, fn, en) {
		    en = (en === '') ? true : en;
		    handlers[name] = {
			    enabled : en,
			    exec : fn
		    };
	    },

	    handleResponse = function(state, el, empty) {
		    // Set the element and its label (if exists) invalid state
		    if (state === false) {
			    markInvalid(el, empty);

		    } else {
			    markValid(el);
		    }
	    },

	    markValid = function(el) {
		    // Get a label
		    var label = el.form.querySelector('label[for="' + el.id + '"]');

		    if (el.classList.contains('required') || el.getAttribute('required')) {
			    var message;
			    if (el.parentNode.tagName.toLowerCase() === 'span' && el.parentNode.classList.contains('input-group')) {
				    message = el.parentNode.parentNode.querySelector('span.invalid')
			    } else {
				    message = el.parentNode.querySelector('span.invalid')
			    }

			    el.classList.remove('invalid');
			    el.classList.add('valid');
			    el.setAttribute('aria-invalid', 'false');

			    // Remove message
			    if (message) {
				    el.parentNode.removeChild(message);
			    }

			    // Restore Label
			    if (label) {
				    label.classList.remove('invalid');
			    }
		    }
	    },

	    markInvalid = function(el, empty) {
		    // Get a label
		    var label = el.form.querySelector('label[for="' + el.id + '"]');

		    el.classList.remove('valid');
		    el.classList.add('invalid');
		    el.setAttribute('aria-invalid', 'true');

		    // Display custom message
		    var mesgCont, message = el.getAttribute('data-validation-text');

		    if (el.parentNode.tagName.toLowerCase() === 'span' && el.parentNode.classList.contains('input-group')) {
			    mesgCont = el.parentNode.parentNode.querySelector('span.invalid')
		    } else {
			    mesgCont = el.parentNode.querySelector('span.invalid')
		    }

		    if (!mesgCont) {
			    var elMsg = document.createElement('span');
			    elMsg.classList.add('invalid');
			    if (empty && empty === 'checkbox') {
				    elMsg.innerHTML = message ? message : Joomla.JText._('JLIB_FORM_FIELD_REQUIRED_CHECK');
			    } else if (empty && empty === 'value') {
				    elMsg.innerHTML = message ? message : Joomla.JText._('JLIB_FORM_FIELD_REQUIRED_VALUE');
			    } else {
				    elMsg.innerHTML = message ? message : Joomla.JText._('JLIB_FORM_FIELD_INVALID_VALUE');
			    }

			    if (el.parentNode.tagName.toLowerCase() === 'span' && el.parentNode.classList.contains('input-group')) {
				    el.parentNode.parentNode.appendChild(elMsg)
			    } else {
				    el.parentNode.appendChild(elMsg)
			    }
		    }

		    // Mark the Label as well
		    if (label) {
			    label.classList.add('invalid');
		    }
	    },

	    removeMarking = function(el) {
		    // Get a label
		    var message, label = el.form.querySelector('label[for="' + el.id + '"]');

		    if (el.parentNode.tagName.toLowerCase() === 'span' && el.parentNode.classList.contains('input-group')) {
			    message = el.parentNode.parentNode.querySelector('span.invalid')
		    } else {
			    message = el.parentNode.querySelector('span.invalid')
		    }

		    el.classList.remove('invalid');
		    el.classList.remove('valid');
		    el.removeAttribute('aria-invalid');

		    // Remove message
		    if (message) {
			    if (el.parentNode.tagName.toLowerCase() === 'span' && el.parentNode.classList.contains('input-group')) {
				    console.log(el.parentNode.parentNode)
				    el.parentNode.parentNode.removeChild(message);
			    } else {
				    el.parentNode.removeChild(message);
			    }
		    }

		    // Restore Label
		    if (label) {
			    label.classList.remove('invalid');
			    label.classList.remove('valid');
		    }
	    },

	    validate = function(el) {
		    debugger;
		    var tagName, handler;
		    // Ignore the element if its currently disabled, because are not submitted for the http-request. For those case return always true.
		    if (el.getAttribute('disabled') == 'disabled' || el.getAttribute('display') == 'none') {
			    handleResponse(true, el);
			    return true;
		    }
		    // If the field is required make sure it has a value
		    if (el.getAttribute('required') || el.classList.contains('required')) {
			    tagName = el.tagName.toLowerCase();
			    if (tagName === 'fieldset' && (el.classList.contains('radio') || el.classList.contains('checkboxes'))) {
				    if (!el.querySelector('input:checked').length){
					    handleResponse(false, el, 'checkbox');
					    return false;
				    }
			    } else if ((el.getAttribute('type') === 'checkbox' && !el.checked.length !== 0) || (tagName === 'select' && !el.value.length)) {
				    handleResponse(false, el, 'checkbox');
				    return false;
				    //If element has class placeholder that means it is empty.
			    } else if (!el.value || el.classList.contains('placeholder')) {
				    handleResponse(false, el, 'value');
				    return false;
			    }
		    }
		    // Only validate the field if the validate class is set
		    handler = (el.getAttribute('class') && el.getAttribute('class').match(/validate-([a-zA-Z0-9\_\-]+)/)) ? el.getAttribute('class').match(/validate-([a-zA-Z0-9\_\-]+)/)[1] : "";

		    if (el.getAttribute('pattern') && el.getAttribute('pattern') != '') {
			    if (el.value.length) {
				    isValid = new RegExp('^' + el.getAttribute('pattern') + '$').test(el.value);
				    handleResponse(isValid, el, 'empty');
				    return isValid;
			    } else {
				    handleResponse(false, el);
				    return false;
			    }
		    } else {
			    if (handler === '') {
				    handleResponse(true, el);
				    return true;
			    }
			    // Check the additional validation types
			    if ((handler) && (handler !== 'none') && (handlers[handler]) && el.value) {
				    // Execute the validation handler and return result
				    if (handlers[handler].exec(el.value, el) !== true) {
					    handleResponse(false, el, 'value');
					    return false;
				    }
			    }
			    // Return validation state
			    handleResponse(true, el);
			    return true;
		    }
	    },

	    isValid = function(form) {
		    var fields, valid = true, message, error, label, invalid = [], i, l;
		    // Validate form fields
		    fields = form.querySelectorAll('input, textarea, select, button');
		    for (i = 0, l = fields.length; i < l; i++) {
			    if (validate(fields[i]) === false) {
				    valid = false;
				    invalid.push(fields[i]);
			    }
		    }
		    // Run custom form validators if present
		    for (var prop in custom) {
			    if (custom.hasOwnProperty(prop)) {
				    if (custom[validator].exec() !== true) {
					    valid = false;
				    }
			    }
		    }

		    forEach(custom, function(key, validator) {
			    if (validator.exec() !== true) {
				    valid = false;
			    }
		    });

		    if (!valid && invalid.length > 0) {
		    	if (form.getAttribute('data-validation-text')) {
				    message = form.getAttribute('data-validation-text');
			    } else {
				    message = Joomla.JText._('JLIB_FORM_CONTAINS_INVALID_FIELDS');
			    }

			    error = {"error": [message]};
			    Joomla.renderMessages(error);
		    }
		    return valid;
	    },

	    attachToForm = function(form) {
		    var inputFields = [], elements;
		    // Iterate through the form object and attach the validate method to all input fields.
		    elements = form.querySelectorAll('input, textarea, select, button');
		    for (var i = 0, l = elements.length; i < l; i++) {
			    var el = elements[i], tagName = el.tagName.toLowerCase();
			    // Attach isValid method to submit button
			    if ((tagName === 'input' || tagName === 'button') && (el.getAttribute('type') === 'submit' || el.getAttribute('type') === 'image')) {
				    if (el.classList.contains('validate')) {
					    el.addEventListener('click', function() {
						    return isValid(form);
					    });
				    }
			    }
			    // Attach validate method only to fields
			    else if (tagName !== 'button' && !(tagName === 'input' && el.getAttribute('type') === 'button')) {
				    if (el.classList.contains('required')) {
					    el.setAttribute('aria-required', true);
					    //el.getAttribute('required') = 'required';
				    }
				    if (tagName !== 'fieldset') {
					    el.addEventListener('blur', function() {
						    return validate(this);
					    });
					    el.addEventListener('focus', function() {
						    return removeMarking(this);
					    });
					    if (el.classList.contains('validate-email') && inputEmail) {
						    el.setAttribute('type', 'email');
					    }
				    }
				    inputFields.push(el);
			    }
		    }
	    },

	    initialize = function() {
		    handlers = {};
		    custom = custom || {};

		    inputEmail = (function() {
			    var input = document.createElement("input");
			    input.setAttribute("type", "email");
			    return input.type !== "text";
		    })();
		    // Default handlers
		    setHandler('username', function(value, element) {
			    var regex = new RegExp("[\<|\>|\"|\'|\%|\;|\(|\)|\&]", "i");
			    return !regex.test(value);
		    });
		    setHandler('password', function(value, element) {
			    var regex = /^\S[\S ]{2,98}\S$/;
			    return regex.test(value);
		    });
		    setHandler('numeric', function(value, element) {
			    var regex = /^(\d|-)?(\d|,)*\.?\d*$/;
			    return regex.test(value);
		    });
		    setHandler('email', function(value, element) {
			    value = punycode.toASCII(value);
			    var regex = /^[a-zA-Z0-9.!#$%&’*+\/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$/;
			    return regex.test(value);
		    });
		    // Attach to forms with class 'form-validate'
		    var forms = document.querySelectorAll('form');
		    for (var i = 0, l = forms.length; i < l; i++) {
			    if (forms[i].classList.contains('form-validate'))
			    {
				    attachToForm(forms[i]);
			    }
		    }
	    };

	// Initialize handlers and attach validation to form
	initialize();

	return {
		isValid : isValid,
		validate : validate,
		setHandler : setHandler,
		attachToForm : attachToForm,
		custom: custom
	};
};

document.formvalidator = null;

document.addEventListener("DOMContentLoaded", function() {
	document.formvalidator = new JFormValidator();
});
avatar Fedik
Fedik - comment - 22 Jan 2017

not really there but so far I have something like

@dgt41 I see you tries to make validation.js without jQuery ?
To support HTML5 validation and fallback in validation.js will be need to modify method validate, to something like:

if (!formValidationSuported()){
  // fallback validation
}
// then call custom handlers

And modify error message displaying.
We can set custom message for native browser validation: https://developer.mozilla.org/en-US/docs/Learn/HTML/Forms/Data_form_validation#Customized_error_messages
so no need to search for label, as it currently done in validation.js

The Browser form validation are supported already not that bad http://caniuse.com/#search=validation
Only Safari can make a small problem with bug in the required attribute support.

Note.
Use .form-validate class to check whether form need to be validated can be dropped for HTML5 world ?
Validation enabled:
<form name="blabla">
Validation disabled:
<form name="blabla" novalidate>
See https://developer.mozilla.org/en/docs/Web/HTML/Element/form#attr-novalidate

avatar Fedik
Fedik - comment - 4 Feb 2017

@dgt41 I have made small example with Browser (HTML5) validation staging...Fedik:html5-validation for the Article form, similar can be done for all

avatar dgt41
dgt41 - comment - 4 Feb 2017

@Fedik we discussed in London sprint about HTML5 (native browser) validation and most people were against the idea of breaking B/C or introducing one more new way for this...

avatar Fedik
Fedik - comment - 4 Feb 2017

I am about j4 only ?

avatar dgt41 dgt41 - change - 13 Mar 2017
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2017-03-13 12:09:39
Closed_By dgt41
avatar dgt41 dgt41 - close - 13 Mar 2017

Add a Comment

Login with GitHub to post a comment