Skip to content

Text zoom: fix visual bugs for buttons and number inputs#15973

Closed
kjellr wants to merge 8 commits into
masterfrom
fix/text-zoom-visual-bugs
Closed

Text zoom: fix visual bugs for buttons and number inputs#15973
kjellr wants to merge 8 commits into
masterfrom
fix/text-zoom-visual-bugs

Conversation

@kjellr

@kjellr kjellr commented Jun 3, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Addresses most of #15356. When text zoom is activated on the browser, the following issues were found:

  • Button text was frequently being pushed out of its container.
  • Select fields and number inputs were cropping the text, making it unreadable.

This PR changes the imposed height property to min-height to the buttons and number inputs. The select fields we use are taken directly from WP-Admin, with no added styles. So those will be adjusted as part of 47477-trac.

When implementing these button fixes, the only visual issues I noticed were:

  • In a couple areas, flexbox was able to stretch the buttons out vertically when they didn't have a set height. 38951df and ee7ea9f should fix those cases.
  • The toolbar preview/publish buttons break out of the toolbar a bit. I'd suggest this is better than the previous behavior.

Other than that, things should look exactly the same when 100% text zoom is active.

Before (at 200% Text Zoom):

Screen Shot 2019-06-03 at 13 39 56

Screen Shot 2019-06-03 at 13 40 37

Screen Shot 2019-06-03 at 13 40 51

After (at 200% Text Zoom)

Screen Shot 2019-08-28 at 3 39 20 PM

Screen Shot 2019-08-28 at 3 38 41 PM

Screen Shot 2019-08-28 at 3 39 33 PM

@kjellr kjellr added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. Needs Accessibility Feedback Need input from accessibility [a11y] Zooming labels Jun 3, 2019
@kjellr kjellr self-assigned this Jun 3, 2019
kjellr
kjellr commented Jun 3, 2019
View reviewed changes
Comment thread assets/stylesheets/_mixins.scss Outdated
@kjellr kjellr mentioned this pull request Jun 3, 2019
Open
kjellr
kjellr commented Jun 4, 2019
View reviewed changes
Comment thread packages/components/src/button/style.scss Outdated
&.is-button {
padding: 0 10px 1px;
line-height: 26px;
height: 28px;

@kjellr kjellr Jun 4, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These button styles are duplicated from core, so they should be updated there too.

@kjellr kjellr mentioned this pull request Jun 7, 2019
Closed
@afercia

afercia commented Jun 7, 2019

Copy link
Copy Markdown
Member

Discussed during today's accessibility bug-scrub. Thanks a lot for working on this. Needs some testing, hope feedback can be given as soon as possible. Any testers, welcome!

@afercia

afercia commented Jun 14, 2019

Copy link
Copy Markdown
Member

Discussed during today's accessibility bug scrub. This looks like a huge improvement!

Special thank to @kjellr for working on this.

Still, the general layout is “fixed width” and doesn’t allow the content area / sidebar to adapt their width, but at least text doesn’t overlap now! Fluid widths should be the standard in today's web development but I guess that's a bit out of the scope of this PR. Thanks!

@afercia afercia removed the Needs Accessibility Feedback Need input from accessibility label Jun 14, 2019
@anevins12 anevins12 mentioned this pull request Jun 14, 2019
Closed
@kjellr kjellr changed the title Text zoom: fix visual bugs for select, buttons, and number inputs Text zoom: fix visual bugs for buttons and number inputs Aug 28, 2019
@kjellr

kjellr commented Aug 28, 2019

Copy link
Copy Markdown
Contributor Author

Just noting that all the existing select field styles in Gutenberg are coming directly from WP-Admin now. That means that the select field fix I had in here before has been removed. Once we fix that in WP-Admin (as part of trac-47477), that fix will be inherited by Gutenberg as well. In the meantime, I've updated the title/description/images for this PR to only apply to the buttons and number inputs.

As such, this is now a generally small update. 🎉 Should be ready for a test if someone has a moment.

@paaljoachim

paaljoachim commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

Hey @kjellr
How can we move this PR onward?
Thanks!

@kjellr

kjellr commented Jul 21, 2020

Copy link
Copy Markdown
Contributor Author

This is probably pretty stale at this point, and unfortunately I don't have time to dedicate to it right now. The best bet is for someone else to take it over.

@paaljoachim paaljoachim added the Needs Dev Ready for, and needs developer efforts label Jul 21, 2020
@paaljoachim

paaljoachim commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

I went ahead and added the Needs Dev label to it.

@youknowriad

youknowriad commented Jul 22, 2020

Copy link
Copy Markdown
Contributor

I think most of these are already fixed now and if they're not it's better to start fresh with the G2 redesign. I'm closing the PR for now, please reopen if you disagree.

@youknowriad youknowriad deleted the fix/text-zoom-visual-bugs branch July 22, 2020 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduth aduth Awaiting requested review from aduth
@ajitbohra ajitbohra Awaiting requested review from ajitbohra
@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten
@gziolo gziolo Awaiting requested review from gziolo
@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta
@notnownikki notnownikki Awaiting requested review from notnownikki
@Soean Soean Awaiting requested review from Soean
@talldan talldan Awaiting requested review from talldan
@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees

@kjellr kjellr

Labels

[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Design Feedback Needs general design feedback. Needs Dev Ready for, and needs developer efforts

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kjellr @afercia @paaljoachim @youknowriad @priethor