Simplify js.module.css

Created on 21 March 2025, about 1 month ago

Problem/Motivation

js.module.css includes some classes for older browsers as of 2023 🐛 Table filter creates jank (layout shift) on page load Fixed .

https://caniuse.com/?search=scripting shows pretty broad support across multiple releases now.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component

CSS

Created by

🇬🇧United Kingdom catch

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

Merge Requests

Comments & Activities

  • Issue created by @catch
  • Merge request !11567Simplify js.module.css → (Closed) created by smustgrave
  • 🇺🇸United States smustgrave

    Seems pretty straight forward.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 474s
    #454570
  • First commit to issue fork.
    • nod_ committed f9be36f0 on 11.x
      Issue #3514748 by catch: Remove legacy browser support from js.module....
  • 🇫🇷France nod_ Lille

    Committed f9be36f and pushed to 11.x. Thanks!

  • Pipeline finished with Failed
    25 days ago
    Total: 386s
    #465299
  • 🇳🇱Netherlands Lendude Amsterdam

    This seems to have broken/stated showing some buttons that need to be hidden in the Views wizard:

    Should open a follow up or is this a revert and fix?

    • nod_ committed 99423203 on 11.x
      Revert "Issue #3514748 by catch: Remove legacy browser support from js....
  • 🇫🇷France nod_ Lille

    reverted for now, thanks for finding the issue!

  • Merge request !11855More specific selectors. → (Open) created by catch
  • 🇬🇧United Kingdom catch

    Cross-posted with nod_, it's a CSS specificity issue - Claro's .button display: inline-block; is overriding the .js-hide.

    Pushed a commit to https://git.drupalcode.org/project/drupal/-/merge_requests/11855#note_49... which seems to be enough.

  • 🇬🇧United Kingdom catch
  • Pipeline finished with Success
    13 days ago
    Total: 363s
    #475077
  • Pipeline finished with Success
    13 days ago
    Total: 342s
    #475084
  • 🇳🇱Netherlands Lendude Amsterdam

    Not confident enough in my css skills to mark RTBC, but checked, and it fixes the issue in the Views wizard at least

  • The views wizard issue is fixed, but the change in MR 11855 causes a regression from 🐛 Table filter creates jank (layout shift) on page load Fixed . The filter is popping in again on /admin/modules when I use Chrome inspector to disable cache and throttle to 3G. This doesn't happen on HEAD.

    Screen recording is attached.

  • 🇬🇧United Kingdom catch

    That's a good additional test case...

    The reason .js .js-hide doesn't work is because the .js class is added by... JavaScript so it completely undermines the point of the original MR. Obvious once you realise it, but wasn't when I tried it out to fix the views issue.

    I could not think of another good way to add additional specificity, so went for !important

    This works for both test cases, but let's be honest I haven't written much CSS either for money or for free since about 2009 so there might be a better way.

    Explicitly tagging for manual testing just in case I messed up the steps to reproduce too.

  • Pipeline finished with Failed
    12 days ago
    Total: 672s
    #475863
  • Manually tested both the views wizard and modules filter and they both look good now. I'm slightly concerned about the use of !important, but I'm not familiar enough with what's going on there to offer any alternatives w/r/t specificity. +1 for RTBC if others are good with the !important.

  • 🇺🇸United States bernardm28 Tennessee

    To overcome some experience builder past bugs, we would add an attribute to increase the specificity.
    Say.

    .js-show[class] {
        display: block;
      }

    https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors
    That could make it more specific than Claro and avoid using !important

  • 🇺🇸United States bernardm28 Tennessee

    Before this fix, without the !important

    After the fix with [class]

  • Looking back at comments from 🐛 Table filter creates jank (layout shift) on page load Fixed :

    Original patch in #2 had this:

    @media (scripting: enabled) {
      /* Extra specificity to override previous selector. */
      [class] .js-hide {
        display: none;
      }
    
      .js-show {
        display: block;
      }
    }
    

    Suggestion was made for this instead in #8:

    Why not .js-hide.js-hide like we do usually when we need to increase specificity?

  • Pipeline finished with Failed
    12 days ago
    Total: 438s
    #476002
  • 🇺🇸United States bernardm28 Tennessee

    Both are valid solutions, but DrupalCMS is leaning more on attributes [class].
    https://git.drupalcode.org/project/drupal_cms/-/merge_requests/374/diffs...
    So I would say preference goes towards class attributes.

  • From the IS, the goal is to remove the additional classes in the selectors since @media (scripting: enabled) is well supported. In which case, seems to me that the selectors within the media query should work well enough as is (and I've just tested to confirm), and the only thing needed is to remove the rules outside the media query.

    That reduces the diff size, and the use of .{CLASS}.{SAMECLASS} selectors is used often in core, so I think that part can be left as is.

    I have to say, this one in media-library.pcss.css is pretty amusing:
    .media-library-item .ajax-progress.ajax-progress.ajax-progress

  • 🇺🇸United States kwiseman

    The .js-hide[class] approach seems like the most concise, specific option without using !important, and I'm also in favor of it. However, that approach would select .js-hide with any other class, not just its duplicate, but I'm not sure how much of a problem that is if it's working for Drupal CMS.

  • 🇮🇪Ireland markconroy

    I think this is a good example of when we would use !important.

    In this instance, it's !important that when the class .js-hide is present that the item concerned is hidden.

    Perhaps we can use !important just for the hiding part and not for the showing part. For .js-show we may not want display: block all the time (sometimes we might want flex/grid/inline-block/etc) so !important should not be used for that.

    After that I think @godotislate "double class" idea is probably best. .js-hide.js-hide. If we use .js-hide[class] it might affect other classes if they are present.

Production build 0.71.5 2024