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

The Button component fails to render a correct font-size #631

Open
afercia opened this issue Jun 29, 2018 · 3 comments
Open

The Button component fails to render a correct font-size #631

afercia opened this issue Jun 29, 2018 · 3 comments
Labels

Comments

@afercia
Copy link
Contributor

afercia commented Jun 29, 2018

Looking at #627 I'm building a page in the standalone yoast-component app to render all the buttons, as I think we're at a point where checking their behavior and visual appearance would be beneficial. While at it, I've noticed the Button component font-size is not applied correctly.

The only difference between BaseButton and Button should be that the latter has a smaller font-size (0.8rem). However it seems this fails, because of the order the CSS classes are applied to the button.

Right now, all buttons have the same font size:

screen shot 2018-06-29 at 11 24 03

The CSS class with font-size: 0.8rem; gets applied first and then overridden by the one with font-size: inherit;.

@afercia
Copy link
Contributor Author

afercia commented Jun 29, 2018

Same for the LinkButton, which is an a element styled like a button:

screen shot 2018-06-29 at 11 57 56

@afercia
Copy link
Contributor Author

afercia commented Jun 29, 2018

I have a quick PR ready for this but maybe we should discuss a bit what's really needed here. /Cc @IreneStr

Currently, the buttons always inherit the font-size from the parent and ancestors. This may or may not be desired, but seems to work in most of the cases. For example in WordPress this would inherit a 13 pixels font-size in most of the cases.

On the other hand, I'm not sure what the original intent of font-size: 0.8rem; is, as this value translates to a computed font-size of 12.8 pixels. At the very least, it should be 0.8125rem.

With a proper fix in place, the base buttons would keep a font-size of 16 pixels while all the other ones would have a font-size of 13 pixels.

screen shot 2018-06-29 at 16 47 25

However, I'm not sure this would be desired in all cases and maybe we should consider to pass the font-siz as a prop with a default value?

@IreneStr
Copy link
Contributor

IreneStr commented Jun 30, 2018

However, I'm not sure this would be desired in all cases and maybe we should consider to pass the font-siz as a prop with a default value?

@afercia Let's worry about this when it turns out to be relevant. For now a font-size: 0.8125rem; and a parent font size of 16px so that the button will have a 13px font seems fine. Because font-size: 0.8125rem; makes sense in WP as well, right?

This was referenced Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants