Regression: styling of most form elements is uneven or off-center

[azaozz]

Introduced in #47477.

The change that caused this is the overriding of line-height and adding min-height to all "single-line" form elements:

input[type="text"],
input[type="password"],
input[type="date"],
input[type="datetime"],
input[type="datetime-local"],
input[type="email"],
input[type="month"],
input[type="number"],
input[type="search"],
input[type="tel"],
input[type="time"],
input[type="url"],
input[type="week"] {
	padding: 0 8px;
	/* inherits font size 14px */
	line-height: 2; /* 28px */
	/* Only necessary for IE11 */
	min-height: 30px;
}

This causes problems in many plugins and in some external packages like the UI in the classic editor/TinyMCE.

[azaozz]

Looking at [46356] overriding of the height of these form elements doesn't seem to solve any accessibility problems (it was done as an "accessibility fix"?). Seems all browsers handle form elements better without that code.

In addition the assumption that the inherited font size is 14px in all cases is wrong as this affects all forms everywhere.

Thinking this should be reverted for 5.3.1 and a better solution implemented if needed.

[SergeyBiryukov]

Related: #48420

[SergeyBiryukov]

Related: #48537

[afercia]

There's already #48420 to discuss and improve alignments of all the form controls.

I'd also like to remind that the last patch on #47477 solved most of these issues but there was some disagreement on committing it during Beta period so it was postponed.

I'd like to suggest to close this ticket and move the conversation to #48420.

overriding of the height of these form elements doesn't seem to solve any accessibility problems (it was done as an "accessibility fix"?)

Not sure I understand the objection. The main goal in #47477 was to remove any fixed height for all form controls to allow better scaling with text zoom. Keeping the top and bottom padding doesn't work very well when scaling with text only zoom, especially if padding is expressed in pixels. The best option is to not use top and bottom padding at all. Please let's continue conversation on #48420.

[azaozz]

Replying to afercia:

This is a specific regression for a specific "global" wp-admin CSS change that was released in WP 5.3. It should be handled as such. It's not better to "merge" it in another ticket as that would make it harder to trace the solution later on.

Keeping the top and bottom padding doesn't work very well when scaling with text only zoom, especially if padding is expressed in pixels.

Right. However it seems to work well when it is not in pixels. Removing the padding "broke" the old-style structural CSS in many places and should have been fixed during 5.3-beta.

[azaozz]

The above screenshots only show the drop-downs and buttons on the Media Library screen (latest Chromium). However all drop-downs and all buttons in all of wp-admin look similarly.

[afercia]

@azaozz please refer to the above screenshots. Those are the same UI parts illustrated by your screenshots the way they looked like in WordPress 5.2 on Chrome on Windows.

The text within selects, buttons, (and input fields) is already not vertically centered in WordPress 5.2 and that happens since the introduction of system fonts in WordPress 4.6.

Please look at the "Apply" button :) That's the most evident case. Please look at all the other input fields. Is that the "good" alignment you'd like to restore?

A "regression" implies there was a previous situation where things worked well. This is not the case, as the vertical alignment of text within form controls is far from ideal on Windows since WordPress 4.6.

I've already mentioned a few times on #47477 and #48420 that this problem on Windows can't be solved via CSS. The only way would be serving a different CSS for macOS and Windows but that would mean opening a can of worms as any operating system uses different system fonts and would need different CSS.

Specifically, the Windows 10 problem is caused by the default system font used there and its internal metrics: Segoe UI. This is a problem that's basically unsolved since a few years. More details on #48423 where I'm proposing to just remove Segoe UI from the "font stack".

Removing the padding "broke" the old-style structural CSS in many places

Having worked on these CSS changes and having extensively tested on all the operating systems and browsers I have access to, I think the best way to standardize all the form controls in WordPress is by avoiding to use top and bottom padding. I did try with padding. It just doesn't work well. Line-height and min-height gave the best results across OSes and browsers.

Also, (this sounds like a conversation we already had) could you please explain to the broader audience reading this ticket what do you mean by "structural CSS"? I do know, I'm asking just for clarification and history.

and should have been fixed during 5.3-beta

I'd like to remind you on #47477 there was a patch ready that improved things a bit. I recommended to commit it even if WordPress was in Beta. That didn't happen.

Considering all this is already covered on other tickets (#48420 and #48423) and that this is not a regression but it's a change that made more evident pre-existing issues, can we agree to close this ticket and focus on the other tickets? I'd like to focus on solutions rather than struggling to coordinate tickets that are, basically, a duplicate of other open tickets.

[azaozz]

For some reason this looks quite better here in WP 5.2.

Considering all this is already covered on other tickets... can we agree to close this ticket

Indeed, perhaps this can be folded under an "umbrella" ticket however the main points here will remain unanswered:

• This code is a specific change that caused regressions. It was added in WP 5.3 and affects all screens in wp-admin. Seems it was meant as an improvement (perhaps to a specific place/element) but didn't improve anything.
• "Global" code changes that affect all of WordPress (regardless if they are CSS, JS, PHP) should not be made from partially related or unrelated tickets. This prevents proper reviews, testing, refinement, bug reports, etc.

Having worked on these CSS changes and having extensively tested on all the operating systems...

Sorry but it's not evident how this particular code was tested, reviewed, perhaps improved/enhanced during beta, and eventually committed and released.

[afercia]

Replying to azaozz:

Indeed, perhaps this can be folded under an "umbrella" ticket ...

Let's make this issue be the umbrella :)

I can only repeat the last patch on #47477 addressed most of the remaining glitches :) Although I recommended to commit that patch, it wasn't committed because (I assume) there was no consensus amongst the release leads given it was a bit late in the release cycle. I must note that other more important and more impactful changes were committed during Beta and RC. Not willing to argue but with the benefit of hindsight maybe committing that last patch would have been a better decision. I do know the development process isn't always linear and sometimes things just happen.

At the same time, the teams involved in these changes (specifically @audrasjb and I) were diligent and prepared what was needed for a revert before the release, maintaining a detailed list of all the committed changes. There was no objection to a revert, if really necessary. The revert didn't happen and I'd like to point out it was up to the release leads to make such a call.

About testing

Sorry but it's not evident how this particular code was tested ...

I'd like to remind everyone that two teams were involved in these changes and multiple contributors tested them. Moreover, @audrasjb made a huge effort to test the changes with more of 20 amongst the most known WordPress plugins to make sure there were no serious breakages. ​He published the testing results on the Make blog, including several screenshots.

That's not to say the testing catched all the issues but I'd like to point out that many of the reported glitches are actually pre-existing issues. The CSS changed in 5.3 just made them more evident. Also that's not to say these changes are perfect :) Perfection is enemy of good though and if WordPress doesn't start improving the admin CSS no progress will ever happen.

Admin CSS state

Any WordPress contributor that worked on the admin CSS knows very well that touching even a small thing can produce unexpected consequences. The admin CSS state is far from ideal, to be fair, and that's perfectly understandable in a so large project.

Lack of maintenance, standardization, established patterns, etc. led the admin CSS to its current state over the years. A CSS roadmap to sanity is waiting since 5 years ​https://make.wordpress.org/core/2014/12/19/core-css-roadmap/. As of today, no substantial progress was made.

Iterations

In my humble opinion, these changes go in the right direction because they try to standardize and simplify as much as possible. They remove several exceptions. They aren't perfect: they need iterations and adjustments. This was discussed by the teams involved and there was consensus for iterations in the following releases.

Given the current state of the admin CSS I'd strongly recommend improvements based on iterations also in minor releases. There's so much to do that I'm not sure everything can be addressed in a "big" change in a major release. I'd like to suggest to consider a different approach, based on smaller, more frequent, changes even if they may introduce temporary glitches if these glitches can be fixed after a few weeks in a minor release. I'd like to see this point discussed more in depth and consider to commit #48420 in WordPress 5.3.1 as that was the original plan.

As I see it, that's the only reasonable approach to start a CSS sanity roadmap. I realize such a process would need better tools.

There are now a number of very good proposal to improve the UI, see for example:

• ​Spacing system (grid)
• ​Type scale
• ​Color palette
• ​Reviewing components

Some of these proposals relate also to form controls. It's very likely that all the inputs and buttons will soon have a minimum default height of 32 pixels to adapt to a 8 pixels based grid. Honestly I don't see a good reason why such a change should wait for a major release. it would be great to have this kind of improvements released continuously and continuously refined also in minor releases instead of having to wait for months to fix, for example, a 2 pixels misalignment.

The Windows system fonts issue

Regarding the vertical alignments issue on Windows, I'd recommend to give #48423 a try. For a quick test, just remove "Segoe UI" from the font stack and see how the vertical alignments on Windows "magically" improve. That's the only reliable, future-proof, way to make things look better on Windows.

The "system fonts" change [37361] from April 2016 is basically an incomplete job. Even the commit message stated:

There will definitely be visual bugs, mainly around alignment and spacing; these should be documented and reported on the ticket and fixed more atomically so that our current and future selves have a better understanding of what happened and why.

For what is worth, on the related ticket I was the only one to point out there were vertical alignments problems on Windows, see ticket:36753#comment:84

The necessary adjustments and fixes never happened and today we're still in a situation where on Windows alignments are far from ideal.

Tools

Testing CSS changes in the admin is terribly hard. A real challenge. It's still, basically, a manual process.

On one hand, WordPress should do its best to standardize and remove all the exceptions introduced here and there to its UI components. I guess that before that, WordPress should maybe establish what the UI components are :)

On the other hand, I'd like to see better tools available on trunk to make the testing process easier. A few years ago @helen and @ryelle worked on ​the WordPress Admin Pattern Library plugin. Plugins are hard to maintain though and the plugin didn't become a widely used tool for testing.

As a start, I'd like to propose the introduction on trunk of a sort of style guide page where the rendering of various UI components can be tested. Starting simple, very basic, with form controls. I'm available to articulate in a more details proposal, as soon as I find some time.

[azaozz]

Replying to afercia:

Thanks for the lengthy response but it still doesn't address the issues this ticket tries to track and fix.

As I pointed out above this ticket is an attempt to rectify the situation from WP 5.3. It is not evident why this particular change was made, what it was supposed to fix, and how it was tested and reviewed.

This ticket should also be used to add or reference the patches that fix the regression in 5.3 (or the failure to do the originally intended fix), and how these patches are tested, reviewed and committed.

Let's make this issue be the umbrella :)

Don't think that's possible, or practical. "Umbrella" tickets can perhaps be used when there are several related tickets that require related changes. It is the same as adding Related #12345, #23456, #34567, $45678 to all corresponding tickets.

Indeed, it is possible to fix and close this ticket when committing a patch that was made for another ticket. In this case relevant tests and reviews should be linked here too.

[audrasjb]

Moving this ticket to milestone 5.4.
I will add links to any other previous related tickets that will are going to land in 5.3.1.

[audrasjb]

Hi @azaozz,

I may be wrong, but I think each detailed issue you raised in this ticket was fixed in previous patches. Can you see remaining issues related to this ticket?

[audrasjb]

Moving this tracking ticket to Future release as some patches landed (or are about to land) in 5.4.

[garrett-eclipse]

Hello, coming from #49576 this secondary issue would have a better home here;

On larger screens when the input is beside the button you'll find the button is slightly larger.

If this should have it's own ticket let me know and I'll move it along.

[afercia]

Thanks @garrett-eclipse. Will comment on #49576.

[davidbaumwald]

This was discussed during today's scrub. With 5.5 RC1 approaching, this is being moved to Future Release. If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, please update the milestone accordingly.