NPM Resource Changed PBF bug Small PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar JackKellyUK
JackKellyUK
11 Feb 2023

Pull Request for Issue #39419, #32692.

Summary of Changes

Added a tiny delay to the js form validation blur event, to prevent execution before the click event is fired on other fields/buttons.

Testing Instructions

  1. Create a Users > Login Form menu item
  2. Go to the menu item
  3. Set the focus to the Username field (should be autofocused) and ensure the field is blank
  4. Tick the 'Remember me' checkbox

Actual result BEFORE applying this Pull Request

User would need tick the checkbox twice

Expected result AFTER applying this Pull Request

Tick box functions as expected

Link to documentations

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

avatar joomla-cms-bot joomla-cms-bot - change - 11 Feb 2023
Category JavaScript Repository NPM Change
avatar JackKellyUK JackKellyUK - open - 11 Feb 2023
avatar JackKellyUK JackKellyUK - change - 11 Feb 2023
Status New Pending
avatar richard67
richard67 - comment - 11 Feb 2023

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

+ npm run lint:js

> joomla@4.0.0 lint:js
> eslint --config build/.eslintrc --ignore-pattern '/media/' --ext .es6.js,.es6,.vue .

/********/src/build/media_source/system/js/fields/validate.es6.js
  306:77  error  Requires a space after '{'   block-spacing
  306:99  error  Requires a space before '}'  block-spacing
  306:99  error  Missing semicolon            semi

✖ 3 problems (3 errors, 0 warnings)
  3 errors and 0 warnings potentially fixable with the `--fix` option.
avatar JackKellyUK JackKellyUK - change - 11 Feb 2023
Labels Added: NPM Resource Changed ?
avatar dgrammatiko
dgrammatiko - comment - 12 Feb 2023

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

avatar JackKellyUK
JackKellyUK - comment - 12 Feb 2023

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

avatar dgrammatiko
dgrammatiko - comment - 12 Feb 2023

@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');
    }
  }
avatar dgrammatiko
dgrammatiko - comment - 12 Feb 2023

@JackKellyUK could you check if #39852 solves the problem?

avatar HLeithner
HLeithner - comment - 2 May 2023

This pull request has been automatically rebased to 4.3-dev.

avatar obuisard obuisard - test_item - 26 Aug 2023 - Tested successfully
avatar obuisard
obuisard - comment - 26 Aug 2023

I have tested this item ✅ successfully on e1a2007


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

avatar N6REJ N6REJ - test_item - 26 Aug 2023 - Tested unsuccessfully
avatar N6REJ
N6REJ - comment - 26 Aug 2023

I have tested this item ? unsuccessfully on e1a2007

I was unable to detect a difference after patch was applied trying all 3 major browsers


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

avatar Quy Quy - change - 3 Sep 2023
Labels Added: bug PR-4.3-dev ?
Removed: ?
avatar HLeithner
HLeithner - comment - 30 Sep 2023

This pull request has been automatically rebased to 4.4-dev.

avatar saschadube saschadube - test_item - 24 Feb 2024 - Tested unsuccessfully
avatar saschadube saschadube - test_item - 24 Feb 2024 - Tested successfully
avatar saschadube
saschadube - comment - 24 Feb 2024

I have tested this item ✅ successfully on dde0191

Works in Chrome, Firefox and Safari.


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

avatar sergejsteinz
sergejsteinz - comment - 24 Feb 2024

I have tested this item ✅ successfully. Hint: But you have to release the mouse button quickly, otherwise it jumps as before.


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

avatar richard67
richard67 - comment - 24 Feb 2024

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.

avatar richard67 richard67 - change - 24 Feb 2024
Labels Added: PBF Small PR-4.4-dev
Removed: PR-4.3-dev ?
avatar richard67 richard67 - alter_testresult - 24 Feb 2024 - 39838: Tested successfully
avatar richard67 richard67 - alter_testresult - 24 Feb 2024 - sergejsteinz: Tested successfully
avatar richard67
richard67 - comment - 24 Feb 2024

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.

avatar richard67 richard67 - change - 24 Feb 2024
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 24 Feb 2024

RTC


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

avatar Quy Quy - change - 14 Mar 2024
Status Ready to Commit Pending
avatar HLeithner HLeithner - change - 24 Apr 2024
Title
Added delay to form validation blur event
[4.4] Added delay to form validation blur event
avatar HLeithner HLeithner - edited - 24 Apr 2024

Add a Comment

Login with GitHub to post a comment