Contrib.social

Convert remaining jQuery val replacement not found by eslint

Open on Drupal.org →
Created on 19 January 2022, almost 3 years ago
Updated 26 July 2024, 4 months ago

Problem/Motivation

The eslint rules used in #3239134: Refactor (if feasible) uses of the jQuery val function to use VanillaJS → is not catching all usage of .val() it is still used in the following files: 

core/misc/states.es6.js
        return this.val() === '';
          return this.filter(':checked').val() || false;
        return this.val();
          return this.filter(':checked').val() || false;
        return this.val();

core/modules/ckeditor/js/models/Model.es6.js
        this.get('$textarea').val(

core/modules/color/preview.es6.js
          form.find('.color-palette input[name="palette[base]"]').val(),
          form.find('.color-palette input[name="palette[text]"]').val(),
          form.find('.color-palette input[name="palette[link]"]').val(),
            .val(),
            .val(),

core/modules/filter/filter.filter_html.admin.es6.js
        this.$allowedHTMLFormItem.val(
        this.$allowedHTMLFormItem.val(this._generateSetting(this.userTags));

// This one is a legitimate use
core/modules/quickedit/js/views/EditorView.es6.js
            .val(value);

core/themes/claro/js/vertical-tabs.es6.js
        .val(this.details.attr('id'));

We should also open an issue in the upstream project.

Steps to reproduce

Proposed resolution

Update states.js
Do not change jquery.form.js because it is excluded from eslint

Remaining tasks

Open an issue in the upstream project.

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Needs work

Version

11.0 🔥

Component
Javascript  →

Last updated about 7 hours ago

Created by

🇫🇷France nod_ Lille

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 8 months ago →
    🇲🇩Moldova thebumik
  • Merge request !73903259381: Follow up jquery val() replace with VanillaJS → (Open) created by thebumik
  • Status changed to Needs review 8 months ago
  • Comment 8 months ago →
    🇲🇩Moldova thebumik

    Replaced jQuery val() methods with VanillaJS.
    Please review!

  • Pipeline finished with Failed
    8 months ago
    Total: 189s
    #141741
  • Pipeline finished with Failed
    8 months ago
    Total: 199s
    #141763
  • Pipeline finished with Failed
    8 months ago
    Total: 178s
    #141776
  • Status changed to Needs work 8 months ago
  • Comment 8 months ago →
    needs-review-queue-bot

    The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Pipeline finished with Failed
    8 months ago
    Total: 177s
    #141792
  • Comment 8 months ago →
    🇫🇷France nod_ Lille

    few comments on the MR, thanks for picking it up!

  • Pipeline finished with Failed
    8 months ago
    Total: 178s
    #141824
  • Comment 8 months ago →
    🇫🇷France nod_ Lille

    I think you reverted a bit too much code in jquery.form.js, we still need to remove the calls to .val() maybe I wasn't clear, sorry.

  • Pipeline finished with Failed
    8 months ago
    Total: 183s
    #141996
  • Pipeline finished with Failed
    8 months ago
    Total: 1016s
    #143301
  • Comment 8 months ago →
    🇫🇷France nod_ Lille

    Had a closer look. Couple of things:

    1. Let's drop the changes to filter.filter_html.admin.js: The whole thing is deprecated and will be removed in D11. 📌 Deprecate core/modules/editor/js/editor.admin.js JS APIs in Drupal 10, for removal in Drupal 11 Fixed
    2. In the same spirit, since jquery.form.js is excluded from eslint, let's remove changes to this one as well. It's a temporary fork until we get rid of it and this one require jQuery to work at all so no sense in removing the dependency on it.
    3. The states test failure is legit, we do not support select with multiple values, jQuery return an array with all the selected options, .value only return the first value. We need to work around that somehow.

    Sorry for making you add/remove changes to specific files, I changed my mind after spending a bit more time with the code.

  • Pipeline finished with Canceled
    7 months ago
    Total: 173s
    #156432
  • Pipeline finished with Canceled
    7 months ago
    Total: 586s
    #156439
  • Pipeline finished with Canceled
    7 months ago
    Total: 221s
    #156446
  • Pipeline finished with Failed
    7 months ago
    Total: 175s
    #156452
  • Pipeline finished with Canceled
    7 months ago
    #156462
  • Pipeline finished with Failed
    7 months ago
    Total: 176s
    #156468
  • Pipeline finished with Failed
    7 months ago
    Total: 480s
    #156479
  • Pipeline finished with Failed
    7 months ago
    #156488
  • Pipeline finished with Failed
    7 months ago
    Total: 175s
    #156492
  • Pipeline finished with Failed
    7 months ago
    Total: 167s
    #156499
  • Pipeline finished with Failed
    7 months ago
    Total: 556s
    #156507
  • Comment 6 months ago →
    🇮🇳India yash.rode pune

    yash.rode → made their first commit to this issue’s fork.

  • Pipeline finished with Failed
    6 months ago
    Total: 511s
    #184970
  • First commit to issue fork.
  • Pipeline finished with Success
    6 months ago
    Total: 719s
    #185646
  • Issue was unassigned.
  • Status changed to Needs review 6 months ago
  • Comment 6 months ago →
    utkarsh_33

    The test are fixed now.In response to

    Let's drop the changes to filter.filter_html.admin.js: The whole thing is deprecated and will be removed in D11. #2567801: Deprecate core/modules/editor/js/editor.admin.js JS APIs in Drupal 10, for removal in Drupal 11

    06c9630b

    commit removes the whole file but as far as i understood from the comment is that we just wanted to revert the changes from that file.It would be helpful if @nod_ can verify the change.
    Marking it needs review as rest everything is addressed apart from the changes in 06c9630b commit.

  • Comment 5 months ago →
    🇺🇸United States smustgrave

    smustgrave → changed the visibility of the branch 3259381-follow-up-for-jquery to hidden.

  • Status changed to RTBC 5 months ago
  • Comment 5 months ago →
    🇺🇸United States smustgrave

    Still not a huge fan of [0]. personally but it was used in the previous commit so would assume fine here.

  • Comment 5 months ago →
    🇳🇿New Zealand quietone

    I read the IS, the comments and the MR. I have updated the title to reflect what is being fixed here and why.

    This still needs a followup per the issue summary.

  • Status changed to Needs work 4 months ago
  • Comment 4 months ago →
    🇫🇷France nod_ Lille

    I'd like to have a function to avoid duplicating the code in two places here. The function can be local to this file, no need to add something to the global Drupal object

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024