- Issue created by @catch
- First commit to issue fork.
- 🇳🇱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?
- 🇬🇧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.
- 🇳🇱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.
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?- 🇺🇸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 wantdisplay: 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.