Match height of font range control with the size select.

[m-e-h]

Description

Match height of font range control with the font size select rather then the small reset button.
I believe this was intended to happen here #9802 (comment) but maybe got missed?

This PR also aligns the range up/down controls to the right side of the input. Currently they're crowding in on the number. (see screenshots)

How has this been tested?

visual tests in FF and Chrome

Screenshots

In Firefox:

Before

After

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[gziolo]

@m-e-h, it's been a few months since you opened this PR and it didn't get reviewed :( Would you mind refreshing this branch so we could test it again the latest version of the master branch of this repository? I add this PR to the current milestone to make sure it gets reviewed.

[m-e-h]

I just refreshed the branch. (I think I did it correctly).

Only question I have here is the padding adjustment I made.
Is this something that would be better to address separately here https://github.com/WordPress/gutenberg/blob/master/packages/components/src/range-control/style.scss#L125

I'm guessing all components-range-control__number inputs suffer from the browser styled controls moving away from the right edge and even overlaying the number in some cases.

Not familiar with the story that lead to the original padding but I'm thinking there may be an !important reason for it. 😃

[kjellr]

The height looks good on my end, but I'm not 100% sure of the padding update. This is a tough one since it looks like the OS-provided styles there can vary a bit. On a Mac at least, the arrow buttons are matched to the font size. This PR makes the arrows push just a little closer to the right edge than I'd anticipate they would:

Firefox:

Chrome:

Safari doesn't have that issue for some reason. 😄

[m-e-h]

Oh yeah, I guess it's more to do with the OS than the browser. Though Chrome and Firefox do render differently on my Ubuntu OS. In addition to my Firefox screenshot above, here's what I see in Chrome.

Most browser stylesheets appear to give inputs a 1px padding around. Maybe padding-right: 1px would be more consistent?

I can't think of where the 5px right padding would benefit a number input. Is there one?

I'm also wondering if we really need to overwrite any .components-range-control__number styles at all in this file. The default range control styles do fine for both the Columns and the Cover blocks. Could probably do something like this for the margin input + .components-range-control__number { margin-left: $grid-size; rather than overwriting it here.

[m-e-h]

Thinking about the padding a little more, it should probably be applied the same to both the right and left sides of the input to account for rtl layouts.

Another option may be to increase the width a tad. 55px wide should be enough to at least show the .1th place.

Can anyone even confirm what I'm seeing? Maybe my Linux distro or GTK theme is doing something weird. What's a "36.5" value look like on Windows?

[kjellr]

Another option may be to increase the width a tad. 55px wide should be enough to at least show the .1th place.

This seems like a relatively simple fix. It's currently 50px. Adding an extra 5px doesn't appear to break anything on my end. I'd probably round it up to 56px wide, so it's a clean multiple of our base size (8px).

[chrisvanpatten]

I feel like maybe there should be some more abstraction here for a generic number input, independent of the range control (also why are we using a range control class outside of the range control component…?) but this looks nice. Has always bugged me how small that input is. Maybe worth shipping and iterating later?

One thing to note, not a blocker, is that in my rare spare time I'm working on some new ideas for how the font size components looks/works in general, to make it easier to implement responsive font sizing. See #11671 for more on that effort, hoping to have an update soon :)

[kjellr]

@m-e-h — as of 8ea21ab, I'm now seeing the height of this field out of sync with the dropdown:

I think we should adjust one or the other to match in this case (probably the range control).

[m-e-h]

That's right @kjellr . Sorry I updated at the end of the day yesterday and didn't have time to comment.

@chrisvanpatten nailed it:

there should be some more abstraction here for a generic number input

Currently we have form.css giving us this as a base:

input[type="number"] {
    height: 28px;
    line-height: 1;
}

Then we have our blanket styles for all inputs (using high specificity selectors like .edit-post-sidebar input[type="number"]) giving us this padding as a base:

padding: 6px 8px;

Then for .components-range-control__number we override that padding with this:

padding: 3px 5px !important;

Some number inputs are fine with this base and stand 28px tall with 3pxx5px padding.

In the case of the font-picker (curently), we change the height to match either the is-large button at 30px height or the is-small button at 24px.

The new focal point component uses components-text-control__input for number inputs:

input[type="number"].components-text-control__input { // Needs specificity to override padding.
		max-width: 4em;
		padding: 6px 4px;
}

A few reusable component classes, like Chris mentioned would solve a lot of this. It would also lend a sort of core "style classes" for block creators to use.

For the Font size picker specifically, should I give it 30px height here or give the base .components-range-control__number height to 30px?

[m-e-h]

I'm thinking they should go on the base https://github.com/WordPress/gutenberg/blob/master/packages/components/src/range-control/style.scss#L120-L126
so that all number input components can be consistent.

.components-range-control__number {
	display: inline-block;
	height: 30px;
	font-weight: 500;
	width: 54px;
	padding: 3px 5px !important;
}

/* for number inputs that sit next to an unwrapped input like a range slider */
input + .components-range-control__number { 
        margin-left: $grid-size;
}

The focal point component could change from the components-text-control__input class to this one and use the CSS already in place.

[kjellr]

Thanks for the background!

I'm thinking they should go on the base so that all number input components can be consistent.

That seems like a good solution to me too. Thanks @m-e-h!

[m-e-h]

I just updated this PR again to focus on fixing the immediate issue. Ultimately all the input[type="number"] controls should be made uniform. #13863

But while that's being worked out, the font-size picker still looks pretty crappy.

I've also added a more sensible padding to the general input[type="number"] style. It was inheriting the general "input" padding but 8px right and left doesn't work on number inputs so !important was being used in the components to override that. I've cut that in half to 4px.

[kjellr]

Looks good! thanks for all the work on this, @m-e-h.