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

Correct text zoom issue in the Content Structure popover #15984

Merged
merged 1 commit into from
Jun 24, 2019

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Jun 4, 2019

Fixes #15357.

As noted in the issue, text in the Content Structure popover currently overlaps when using the browser's text zoom feature. This PR switches the fixed percentage width to use the somewhat more flexibleflex-basis instead. It also adds a tiny bit of spacing in between each item.


Before

Viewed at 100%
Screen Shot 2019-06-04 at 2 51 11 PM

Viewed at 200%
Screen Shot 2019-06-04 at 2 50 57 PM


After

Viewed at 100% (No changes)

Screen Shot 2019-06-04 at 2 49 43 PM

Viewed at 200%

Screen Shot 2019-06-04 at 2 50 20 PM

@kjellr kjellr added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Feature] Document Outline An option that outlines content based on a title and headings used in the post/page Needs Accessibility Feedback Need input from accessibility [a11y] Zooming labels Jun 4, 2019
@kjellr kjellr self-assigned this Jun 4, 2019
@kjellr kjellr requested a review from afercia June 4, 2019 18:57
Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

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

@kjellr thanks! Screenshot including some the first heading in the content structure popover, from Firefox with zoom > text only at very high level:

Screenshot 2019-06-13 at 17 17 25

Looks good to me: the text doesn't overlap.

Ideally, the popover width should be able to adjust a bit depending on the available space. Something like a "range" between a min and max width maybe. Not sure if that would impact the calculation of the popover position which is done via JavaScript. If possible, I'd encourage some exploration in that direction 🙂

The build fails, not sure why.

@afercia afercia removed the Needs Accessibility Feedback Need input from accessibility label Jun 13, 2019
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

The build is passing, and the change looks good to me 👍 I guess we can merge this solution but I agree it would be good to explore a dynamic popup width in other PR/issue.

@kjellr
Copy link
Contributor Author

kjellr commented Jun 24, 2019

Thank you both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Document Outline An option that outlines content based on a title and headings used in the post/page [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Content overlaps at 200% text enlarge
4 participants