User tests: Successful: Unsuccessful:
Pull Request for Issue #39419, #32692.
Added a tiny delay to the js form validation blur event, to prevent execution before the click event is fired on other fields/buttons.
User would need tick the checkbox twice
Tick box functions as expected
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
Category | ⇒ | JavaScript Repository NPM Change |
Status | New | ⇒ | Pending |
Labels |
Added:
NPM Resource Changed
?
|
@JackKellyUK would be nice to profile the script and figure out why the delay is needed. Adding a constant amount of delay might work on some machines but might fail on others depending on their CPU, etc, thus it would be better to figure out where the race condition occurs and maybe it would be possible to rewrite the script in such way (promises/async-await) to eliminate it regardless of cpu power of host.
@JackKellyUK would be nice to profile the script and figure out why the delay is needed. Adding a constant amount of delay might work on some machines but might fail on others depending on their CPU, etc, thus it would be better to figure out where the race condition occurs and maybe it would be possible to rewrite the script in such way (promises/async-await) to eliminate it regardless of cpu power of host.
I've outlined the issue here with solutions I can think of. I can't think of an efficient way of using a promise in combination with the blur event.
@JackKellyUK so according to your findings the problem comes from the insertion of the span for the invalid message. I would suggest to add the delay inside the validation method, something like:
markInvalid(element, empty) {
// Get a label
const label = element.form.querySelector(`label[for="${element.id}"]`);
element.classList.remove('form-control-success');
element.classList.remove('valid');
element.classList.add('form-control-danger');
element.classList.add('invalid');
element.parentNode.classList.remove('has-success');
element.parentNode.classList.add('has-danger');
element.setAttribute('aria-invalid', 'true');
setTimeout(() => {
// Display custom message
let mesgCont;
const message = element.getAttribute('data-validation-text');
if (label) {
mesgCont = label.querySelector('span.form-control-feedback');
}
if (!mesgCont) {
const elMsg = document.createElement('span');
elMsg.classList.add('form-control-feedback');
if (empty && empty === 'checkbox') {
elMsg.innerHTML = message !== null ? Joomla.sanitizeHtml(message) : Joomla.sanitizeHtml(Joomla.Text._('JLIB_FORM_FIELD_REQUIRED_CHECK'));
} else if (empty && empty === 'value') {
elMsg.innerHTML = message !== null ? Joomla.sanitizeHtml(message) : Joomla.sanitizeHtml(Joomla.Text._('JLIB_FORM_FIELD_REQUIRED_VALUE'));
} else {
elMsg.innerHTML = message !== null ? Joomla.sanitizeHtml(message) : Joomla.sanitizeHtml(Joomla.Text._('JLIB_FORM_FIELD_INVALID_VALUE'));
}
if (label) {
label.appendChild(elMsg);
}
}
}, 100);
// Mark the Label as well
if (label) {
label.classList.add('invalid');
}
}
@JackKellyUK could you check if #39852 solves the problem?
This pull request has been automatically rebased to 4.3-dev.
I have tested this item ✅ successfully on e1a2007
I have tested this item ? unsuccessfully on e1a2007
I was unable to detect a difference after patch was applied trying all 3 major browsers
Labels |
Added:
bug
PR-4.3-dev
?
Removed: ? |
This pull request has been automatically rebased to 4.4-dev.
I have tested this item ✅ successfully on dde0191
Works in Chrome, Firefox and Safari.
I have tested this item ✅ successfully. Hint: But you have to release the mouse button quickly, otherwise it jumps as before.
I have tested this item ✅ successfully. Hint: But you have to release the mouse button quickly, otherwise it jumps as before.
@sergejsteinz Please use the blue "Test this" button in the issue tracker to submit your test result so the test is properly counted. Leaving just a comment is not sufficient. Thanks in advance.
Labels |
Added:
PBF
Small
PR-4.4-dev
Removed: PR-4.3-dev ? |
I've set the test result for @sergejsteinz because it was not counted before as he hasn't used the "Test this" button to submit the result.
Status | Pending | ⇒ | Ready to Commit |
RTC
Status | Ready to Commit | ⇒ | Pending |
Title |
|
This pull request has been automatically rebased to 5.2-dev.
Title |
|
@JackKellyUK Could you fix the javascript cose style errors reported here https://ci.joomla.org/joomla/joomla-cms/61744/1/26 for the "javascript-cs" step?