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.

Sign up for GitHub

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

Jump to bottom

[CollapsibleNav] Make CollapsibleNav Items Accessible on Small Screens and In Browsers with High Zoom #155817

Merged
merged 12 commits into from
May 1, 2023
Merged

[CollapsibleNav] Make CollapsibleNav Items Accessible on Small Screens and In Browsers with High Zoom #155817

merged 12 commits into from
May 1, 2023

Conversation

breehall
Copy link
Contributor

@breehall breehall commented Apr 26, 2023 •
edited
Loading

Fixes elastic/eui#6702

Pinging @elastic/eui-team for visibility

Summary

✨ What's the problem?

A user submitted an issue where they were unable to use the main collapsible nav when their browser was zoomed in at 400% for accessibility. The nav items weren't visible and could not be interacted with at this level.

image

✨ What changed?

When the container for the collapsible nav has a max-height of 15em, overflow and additional flex properties are used to allow the nav groups to be traversed.

Screen.Recording.2023-04-25.at.9.42.51.PM.mov

✨ QA

View the Kibana homepage and set the Zoom on your browser to 400%. This will need to be done in a relatively small window (like the one on your computer itself, not an external monitor). Open the main navigation menu on the left. You should be able to navigate all links in menu with both keyboard and by scrolling the sections.

Checklist

Delete any items that are not applicable to this PR.

  • Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)

For maintainers

@breehall
[CollapsibleNav] Add a styling condition to ensure that Collapsible N…
27c3d68 
…av Group items can be seen and interacted with when the screen is very small or the browser is using a high zoom level
@breehall breehall marked this pull request as ready for review April 26, 2023 13:15
@breehall breehall requested a review from a team as a code owner April 26, 2023 13:15
@breehall breehall marked this pull request as draft April 26, 2023 13:26
@breehall breehall added EUI release_note:skip Skip the PR/issue when compiling release notes labels Apr 26, 2023
@breehall
Copy link
Contributor Author

@elasticmachine merge upstream

@breehall breehall marked this pull request as ready for review April 26, 2023 14:36
@breehall breehall changed the title [CollapsibleNav] Add a styling condition to ensure that Collapsible N… [CollapsibleNav] Make CollapsibleNav Items Accessible on Small Screens and In Browsers with High Zoom Apr 26, 2023
@breehall breehall mentioned this pull request Apr 26, 2023
Closed
@breehall breehall requested a review from a team as a code owner April 27, 2023 13:43
@breehall breehall requested a review from cee-chen April 27, 2023 13:45
@breehall
Copy link
Contributor Author

Putting this in draft to investigate test failure

@breehall breehall marked this pull request as draft April 27, 2023 14:58
cee-chen
cee-chen reviewed Apr 27, 2023
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

One super minor comment left and I think this is good to go!

packages/core/chrome/core-chrome-browser-internal/src/ui/header/collapsible_nav.scss Outdated Show resolved Hide resolved
@cee-chen
Copy link
Contributor

@elasticmachine merge upstream

cee-chen
cee-chen approved these changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Looks great to me - thanks Bree!

Re-merged upstream in hopes that it fixes the FTR tests 🤞

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 360.9KB 362.5KB +1.5KB
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 17 19 +2
securitySolution 399 402 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 18 20 +2
securitySolution 479 482 +3
total +5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@breehall breehall marked this pull request as ready for review April 27, 2023 20:12
ryankeairns
ryankeairns approved these changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Tested, works as described and as seen in video. Nice enhancement!

Dosant
Dosant approved these changes May 1, 2023
View reviewed changes
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

code changes lgtm, thanks!

@breehall breehall merged commit 8cdef94 into elastic:main May 1, 2023
@kibanamachine kibanamachine added v8.9.0 backport:skip This commit does not require backporting labels May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Dosant Dosant Dosant approved these changes

@cee-chen cee-chen cee-chen approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting EUI release_note:skip Skip the PR/issue when compiling release notes v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[EuiCollapsibleNav] Not scrollable at smaller screens (and should also potentially expand to full-width)
6 participants
@breehall @cee-chen @kibana-ci @ryankeairns @Dosant @kibanamachine