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

Button Padding not calculating #29675

Closed
pbking opened this issue Mar 9, 2021 · 5 comments
Closed

Button Padding not calculating #29675

pbking opened this issue Mar 9, 2021 · 5 comments
Labels
[Block] Buttons Affects the Buttons Block

Comments

@pbking
Copy link
Contributor

pbking commented Mar 9, 2021

Description

When evaluating button styling padding calculations appear to be breaking in some situations.

Step-by-step reproduction instructions

Add a button to a page. (Ensure that it is NOT styled as an 'outline button')
When running locally (using wp-env) the padding of the button is the size that is expected.
image
I see that the "padding" for this button is set to padding:calc(.667em + 2px) calc(1.333em + 2px); which I believe was done this way to ensure that 'regular' and 'outline' buttons are ultimately the same size.

Next I re-created the same site (using pressable) and note that the button padding is NOT rendering correctly. (This can also be seen in theamdemo.wordpress.com.)
image
I note that the padding value is set in this scenario to padding:calc(.667em+2px) calc(1.333em+2px); which is exactly the same as the other instance with the space removed from around the +, however the value is noted as an "invalid property value". Reintroducing the spaces manually addressed the padding errors.

Expected behaviour

The padding of the buttons should be expressed in such a way that will work in every scenario.

Actual behaviour

The padding styles of buttons are expressed in a way that breaks when content is optimized (spaces removed).

WordPress information

  • WordPress version: 5.6.2
  • Gutenberg version: 10.1.1
  • Are all plugins except Gutenberg deactivated? Yes
  • Are you using a default theme (e.g. Twenty Twenty-One)? No
@scruffian
Copy link
Contributor

This change was introduced in #29208

@scruffian
Copy link
Contributor

I don't think this is a Gutenberg issue, it's about spaces being removed from CSS, so I think it's more likely to be an environmental issue.

@ockham
Copy link
Contributor

ockham commented Mar 9, 2021

From a quick (!) glance, this could be similar to #28497.

(It's possible that we need to tweak the normalization done in #28501 in order to make sure that expressions like this are treated as invalid.) Edit: Nevermind, this part seems to be handled correctly by Gutenberg (and covered by a unit test):

it( 'leaves zero values in calc() expressions alone', () => {
const normalizedValue = getNormalizedStyleValue(
'calc(0em + 5px)'
);
expect( normalizedValue ).toBe( 'calc(0em + 5px)' );
} );

@ockham
Copy link
Contributor

ockham commented Mar 10, 2021

I agree that this seems to be WP.com (and Pressable) specific (rather than being a bug in Gutenberg itself) -- probably due to a CSS minifier violating the CSS spec by removing spaces around operands in a calc() expression.

I'll thus close this issue. To keep track of it for WP.com, please file an issue in the https://github.com/Automattic/wp-calypso repo instead. (Furthermore, from internal Slack discussions, it seems like WP.com teams are already aware and looking into this issue.)

@ockham ockham closed this as completed Mar 10, 2021
@ockham
Copy link
Contributor

ockham commented Mar 10, 2021

Seems I was mistaken: looks like this was also reported here, and fixed by #29554 (and minification was later removed altogether by #29624).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block
Projects
None yet
Development

No branches or pull requests

4 participants