Navigation: Try removing :where for menu show/hiding #42964
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasmussen
added
[Type] Bug
|
Size Change: -24 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
|
Created an additional addon PR that re-enables padding inheritance from custom
At the moment the navigation block does not distinguish between top level and sublevel menu items, meaning we can't provide a default padding to either, unless we a) use |
|
Here is a second followup that allows us to keep the default padding rules, while still removing With custom
Without such rules, just showing defaults:
My preference would be for this last one, as it maintains ideal behavior while still addressing the functional aspect. |
jasmussen
requested review from
a team and
ellatrix
and removed request for
a team
|
@scruffian @adamziel it sounds like this is an use case where absorving the block's CSS into the styles algorithm would be benefitial, as done at #34180 for the button block. (not blocking this PR, just suggesting a future path forward to expand and test the approach tried at 34180) |
|
Not sure if it's the source of this issue for this block, but sharing for awareness: since #41934 the block CSS loads after global styles, hence potentially overriding theme & user styles. |
|
Since #39230 is closed I think we close this too. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Aims to address #39230.
Global styles outputs CSS rules from theme.json with delicate specificity, just targetting a specific element. In some places, the
:whereselector has been used to reduce specificity of default rules, so that those can exist but still be easily overridden by global styles.In the case of submenus showing/hiding, this has proven problematic in terms of affecting too fundamental a behavior, with insufficient browser support. See also discussion in #39621.
This PR removes the
:whereselector for submenus, ideally addressing that problem.Why?
Why was the
:whereselector added in the first place? It was added to allow us to provide a default padding when the menu had a background, so that menu items wouldn't be flush against the edge, like so:— while still allowing padding rules set in theme.json to override those default paddings. For example using the following in theme.json:
However, and perhaps due to changes in how theme.json outputs styles, custom theme.json provided paddings are broken in trunk at the moment. In the following GIF, the above theme.json rules are shown working in the editor, but not on the frontend:
The above GIF represents the behavior both fo trunk and of this branch. Given that's the case, presumably we can remove the
:whereselector for menu show/hiding, as the net result would be the same.I will follow up with a separate PR for a new approach to padding and theme.json.
Testing Instructions
This needs very thorough testing:
Across that matrix, please also test: