Block Editor: Update BEM syntax to CSS modifer guidelines#19738
Conversation
| const toggleState = container.querySelector( 'input[type="checkbox"]' ).checked; | ||
|
|
||
| const defaultControlGroup = container.querySelector( '.block-editor-responsive-block-control__group--default' ); | ||
| const defaultControlGroup = container.querySelector( '.block-editor-responsive-block-control__group:not(.is-responsive)' ); |
There was a problem hiding this comment.
It's a bummer that this requires the :not pseudo-selector. I suppose it's not particularly an issue here, but selectors that might potentially have several modifiers will need extra care; scoping a selector for a component's "default" state could require multiple :not checks that could get difficult to manage. Hopefully that's a relatively rare case though.
There was a problem hiding this comment.
(And I suppose it's more likely to be the case in tests like this; it's probably possible to architect without it being a concern in the CSS itself, and especially if some of the Emotion explorations move forward.)
There was a problem hiding this comment.
Yeah, I see this as mostly a use-case we'd want to isolate within tests, where we're explicitly wanting to distinguish between the default and the variant.
In real-world application, we should typically want to treat the variant as something of an additive layer upon the default, even to the extent of unsetting/overriding properties from those defaults, vs. ever feeling the need to use :not( .is-variant ) in CSS. If we found ourselves needing to use :not, it probably raises a bigger question of whether we can really consider there to be a default (vs. other named variants, or treating them as wholly different elements).
288cd55 to
731a6a3
Compare
731a6a3 to
b4a46ad
Compare
Previously: #17846 (comment), #16790 (comment)
This pull request seeks to update incorrect usage of BEM modifier syntax to use the
is--based modifier syntax as recommended in the CSS naming guidelines.This impacts two components:
LinkControlResponveBlockControlIn the latter case, the implementation has been revised as mentioned at #16790 (comment) to unify the application of a single
is-responsivemodifier class.Open Question: There was a
hiddenattribute included previously inResponsiveBlockControlwhich would never be applied given how the logic flowed. The changes here seek only to preserve the current behavior. If it needs to be applied, I can restore it as appropriate.Testing Instructions:
Repeat testing instructions from #17846 and #16790, verifying that there are no regressions.
Ensure tests pass: