Skip to content
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

Reduce unnecessary selector specificity for Button block #23246

Merged
merged 1 commit into from
Jun 21, 2020

Conversation

oxyc
Copy link
Member

@oxyc oxyc commented Jun 17, 2020

Description

See #23222 (comment).

The selector used is unnecessarily specific (0,3,0) and it only needs to be (0,2,0) to override the block default margins since it's enqueued after and thus take location precedence.

I'd still leave the comment about it being specific since ideally it would be just .wp-block-button but it cant because of the block default margin selector (which in turn is there to override list margin-left properties.

There's some minor spacing difference between the buttons in editor and frontend but that's shown in master already and is unrelated to this.

One real world reason I'd argue this is needed is because it's much more common for themers use to .wp-block-buttons .wp-block-button than it would be for them to even know you can chain selectors like .wp-block-buttons .wp-block-button.wp-block-button. So to override it we'd probably see lots of .entry-content .wp-block-buttons .wp-block-button.

How has this been tested?

Tested adding buttons in twentynineteen and twentytwenty

Screenshots

Twentytwenty Editor:

Screen Shot 2020-06-17 at 12 58 10

TwentyTwenty Frontend:
Screen Shot 2020-06-17 at 12 57 54

TwentyNinteen Editor:

Screen Shot 2020-06-17 at 12 57 18

TwentyNineteen Frontend:

Screen Shot 2020-06-17 at 12 57 39

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@oxyc
Copy link
Member Author

oxyc commented Jun 17, 2020

ping @jasmussen if you can think of any cases I'm missing

@jasmussen
Copy link
Contributor

Wonderful! Thanks for the PR. I'm out for the day but will review tomorrow.

@ZebulanStanphill ZebulanStanphill added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. CSS Styling Related to editor and front end styles, CSS-specific issues. [Type] Code Quality Issues or PRs that relate to code quality [Package] Block library /packages/block-library labels Jun 17, 2020
@jasmussen jasmussen self-requested a review June 18, 2020 08:31
Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First off, thank you for keeping on this. It may seem like a trivial change, but it is one that potentially has serious quality of life improvements to themers. These details really matter.

I took this for a spin in the Twenty Nineteen theme. Before:

Screenshot 2020-06-18 at 10 29 37

Screenshot 2020-06-18 at 10 29 43

Editor and frontend above, both look the same, and correct.

After:

Screenshot 2020-06-18 at 10 31 35

Screenshot 2020-06-18 at 10 31 41

Above is also editor and frontend. As far as I can tell, things work as intended, even with the reduced specificity. Code also looks good, and the math seems right, for that reason, I'm approving, and thanks again.

I would love if @kjellr has time to sanity check this, but in absence of that, let's get this in.

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that seems fine on my end!

@oxyc oxyc merged commit 5a341a8 into WordPress:master Jun 21, 2020
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 21, 2020
This was referenced Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Package] Block library /packages/block-library [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants