-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove invalid buttons block CSS property #33793
Remove invalid buttons block CSS property #33793
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @mukeshpanchal27.
Would you be able to give the PR a proper title like "Fix invalid CSS". The PR titles appear in the changelog, so it's great if there's a readable description.
&:not( | ||
.is-content-justification-space-between, | ||
.is-content-justification-right, | ||
.is-content-justification-left, | ||
.is-content-justification-center | ||
) .wp-block[data-align="center"] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting one, because this is valid CSS and supported by browsers, but the spec this is part of is technically a working draft, which is why the validator has flagged it.
The 'note' part here mentions it:
https://drafts.csswg.org/selectors/#negation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way :not()
contributes to specificity is a little strange: the specificity is consistently the value of the most specific selector in the list.
For the existing code, this results in the :not()
selector contributing one class to the specificity. Bringing the specificity of the generated selector to 0, 0, 4, 0
.
In the proposed code, each instance of the :not()
selector contributes one class. Bringing the specificity of the generated selector to 0, 0, 7, 0
.
Given the code works in all browsers WP and Gutenberg supports, I'd suggest keeping the code as currently published. The reported validator issue isn't a practical issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I agree with leaving this one as is.
@mukeshpanchal27 Do you agree, and are you able to update this PR to address this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mukeshpanchal27 Checking if you're able to add this back to the PR branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@desrosj PR updated as requested.
This will fix for #32434