Skip to content

Introduce jquery formvalidator for com_languages #5046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

Introduce jquery formvalidator for com_languages #5046

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8d3f308
formvalidator & jquery
dgrammatiko Nov 8, 2014
7af4564
implode error
dgrammatiko Nov 8, 2014
4c70552
Quotes
dgrammatiko Nov 10, 2014
29c8df4
fix
dgrammatiko Nov 11, 2014
63160f4
small fix
dgrammatiko Nov 11, 2014
d6828dd
respect the new lines
dgrammatiko Nov 12, 2014
2a14685
JS CS
dgrammatiko Nov 21, 2014
8103e49
Merge branch 'staging' of https://github.com/joomla/joomla-cms into _…
dgrammatiko Nov 29, 2014
10b81c5
.ready()
dgrammatiko Nov 29, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions administrator/components/com_languages/views/language/tmpl/edit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,20 @@

JHtml::addIncludePath(JPATH_COMPONENT.'/helpers/html');

JHtml::_('behavior.formvalidation');
JHtml::_('behavior.formvalidator');
JHtml::_('formbehavior.chosen', 'select');
?>
<script type="text/javascript">

JFactory::getDocument()->addScriptDeclaration('
jQuery(document).ready(function() {
Joomla.submitbutton = function(task)
Copy link
Member

@Fedik Fedik Nov 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@dgrammatiko dgrammatiko Nov 21, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Member

@Fedik Fedik Nov 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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? 😄

Copy link
Contributor Author

@dgrammatiko dgrammatiko Nov 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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! ❔

Copy link
Member

@Fedik Fedik Nov 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😉

Copy link
Contributor Author

@dgrammatiko dgrammatiko Nov 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 😉

{
if (task == 'language.cancel' || document.formvalidator.isValid(document.id('language-form')))
if (task == "language.cancel" || document.formvalidator.isValid(document.getElementById("language-form")))
{
Joomla.submitform(task, document.getElementById('language-form'));
Joomla.submitform(task, document.getElementById("language-form"));
}
}
</script>
});');
?>

<form action="<?php echo JRoute::_('index.php?option=com_languages&layout=edit&lang_id='.(int) $this->item->lang_id); ?>" method="post" name="adminForm" id="language-form" class="form-validate form-horizontal">

Expand Down
14 changes: 8 additions & 6 deletions administrator/components/com_languages/views/languages/tmpl/default.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,26 @@
}

$sortFields = $this->getSortFields();
?>
<script type="text/javascript">

JFactory::getDocument()->addScriptDeclaration('
jQuery(document).ready(function() {
Joomla.orderTable = function()
{
table = document.getElementById("sortTable");
direction = document.getElementById("directionTable");
order = table.options[table.selectedIndex].value;
if (order != '<?php echo $listOrder; ?>')
if (order != "' . $listOrder . '")
{
dirn = 'asc';
dirn = "asc";
}
else
{
dirn = direction.options[direction.selectedIndex].value;
}
Joomla.tableOrdering(order, dirn, '');
Joomla.tableOrdering(order, dirn, "");
}
</script>
});');
?>

<form action="<?php echo JRoute::_('index.php?option=com_languages&view=languages'); ?>" method="post" name="adminForm" id="adminForm">
<?php if (!empty( $this->sidebar)) : ?>
Expand Down
38 changes: 20 additions & 18 deletions administrator/components/com_languages/views/override/tmpl/edit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,37 @@

defined('_JEXEC') or die;

JHtml::addIncludePath(JPATH_COMPONENT.'/helpers/html');
JHtml::addIncludePath(JPATH_COMPONENT . '/helpers/html');

JHtml::_('behavior.formvalidation');
JHtml::_('behavior.keepalive');
JHtml::_('formbehavior.chosen', 'select');
?>
<script type="text/javascript">
window.addEvent('domready', function()
{
document.id('jform_searchstring').addEvent('focus', function()

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

JFactory::getDocument()->addScriptDeclaration('
jQuery(document).ready(function() {
document.getElementById("jform_searchstring").addEvent("focus", function() {
if (!Joomla.overrider.states.refreshed)
{
if (!Joomla.overrider.states.refreshed)
var expired = "' . $expired . '";
if (expired)
{
<?php if ($this->state->get('cache_expired')) : ?>
Joomla.overrider.refreshCache();
Joomla.overrider.states.refreshed = true;
<?php endif; ?>
}
this.removeClass('invalid');
});
}
this.removeClass("invalid");
});
Joomla.submitbutton = function(task)
{
if (task == 'override.cancel' || document.formvalidator.isValid(document.id('override-form')))
{
Joomla.submitform(task, document.getElementById('override-form'));
Joomla.submitbutton = function(task) {
if (task == "override.cancel" || document.formvalidator.isValid(document.getElementById("override-form")))
{
Joomla.submitform(task, document.getElementById("override-form"));
}
}
}
</script>
});
');
?>

<form action="<?php echo JRoute::_('index.php?option=com_languages&id='.$this->item->key); ?>" method="post" name="adminForm" id="override-form" class="form-validate form-horizontal">
<div class="row-fluid">
Expand Down
15 changes: 7 additions & 8 deletions administrator/components/com_menus/views/item/tmpl/edit_modules.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@

JHtml::_('behavior.core');

$script = array();
$script[] = " jQuery(document).ready(function() {";
$script[] = " jQuery('#showmods').on('click', function(e) {";
$script[] = " jQuery('.table tr.no').toggle();";
$script[] = " });";
$script[] = " })";
JFactory::getDocument()->addScriptDeclaration("
jQuery(document).ready(function() {
jQuery('#showmods').on('click', function(e) {
jQuery('.table tr.no').toggle();
});
})
");

// Add the script to the document head.
JFactory::getDocument()->addScriptDeclaration(implode("\n", $script));
?>

<div class="control-group">
Expand Down