WIP: revert lower specificity for alignments #39016
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
8 tasks
MaggieCabrera
requested review from
scruffian,
jffng,
jasmussen and
youknowriad
|
Size Change: -8 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
|
Looks like this one isn't enough, it's failing for me on Blockbase because of the load order of the CSS. |
|
Closing this in favor of #39014 |
|
Conceptually, this looks like a better approach to me than the alternative. In other words, if a block need to define a default for this, I'd prefer if they write Also, the approach in this PR is exactly the same thing we had for a long time and not sure why it started failing just recently (the move to data-align changed things but this is reverting things to how they were before, so there's something else going on here that we need to find) |
This doesn't work for the initial issue because of the load order of the CSS. There's two problems at hand: the lower specificity of the container descendants selector and the change of the load order of the CSS, combined with the specific edge case where the extra tag on the query block causes the container selector to be on the wrapper, not the actual block that needs the rule |
|
We're zeroing in on the cause in #39014 (comment). |
When did the loading order change, the styles here has been like that for months and the theme issues are recent right. |
It changed here WordPress/wordpress-develop@43a8631 but it wasn't a problem until we changed specificity in #38947 |
|
cc @jorgefilipecosta @oandregal Can we agree on the ideal loading order first, try to implement it and try to fix the specificity battles after (as otherwise I feel we're just creating issues by solving other issues). For me the load order should be:
|
|
To be clear, I'm not against the alternative PR but it only solves the problem for a specific block while it seems that something that can impact any block (including third-party ones). Could be a temporary fix. |
yeah, I'm not so sure myself but I wanted to get the ball rolling and start a conversation |
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.
Description
An alternative solution to #39014. In this case, I've reverted the changes on the specificity of the layout descendants selector and to keep the align left and right working as intended I've opted for overriding what we don't want to apply instead of using
:not, which was causing specificity battles on a number of edge cases.Testing Instructions
Same as #39014
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).