-
Notifications
You must be signed in to change notification settings - Fork 42
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
SideNav
- Disable keyboard navigation/focus of hidden elements
#1909
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
SideNav
- fixed navigation elements remaining interactive when minimizedSideNav
- fix navigation elements remaining interactive when minimized
SideNav
- fix navigation elements remaining interactive when minimizedSideNav
- Disable keyboard navigation/focus of hidden elements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a praise and... a question (sorry). 😀
@@ -9,10 +9,10 @@ | |||
<div class="hds-side-nav__wrapper-header"> | |||
{{yield to="header"}} | |||
</div> | |||
<div class="hds-side-nav__wrapper-body"> | |||
<div class="hds-side-nav__wrapper-body" inert={{@isMinimized}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice and neat! 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tidy approach, nice thinking!
8782fd0
to
b67f6f4
Compare
This reverts commit e4b21a2.
b67f6f4
to
590e287
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a couple of comments, but I am happy with the solution
let me know when the comments are resolved that I'll approve
.changeset/strong-balloons-repeat.md
Outdated
"@hashicorp/design-system-components": patch | ||
--- | ||
|
||
`SideNav` - fixed navigation elements remaining interactive when minimized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`SideNav` - fixed navigation elements remaining interactive when minimized | |
`SideNav` - fixed issue with navigation elements remaining interactive when minimized |
let { onToggleMinimizedStatus } = this.args; | ||
|
||
if (typeof onToggleMinimizedStatus === 'function') { | ||
onToggleMinimizedStatus(this.isMinimized); | ||
} | ||
} | ||
|
||
@action | ||
didInsert(element) { | ||
this.containersToHide = element.querySelectorAll( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌 🙌 🙌 🙌
590e287
to
910a296
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job! 👏
📌 Summary
Currently, interactive elements within the SideNav are focusable even when the SideNav is minimized/collapsed. This makes them visible and interactive to screen-reader users even though they are not visible.
The usual and easiest way to fix this issue is to set
display: none
on such elements. However, because of the desired fade effect, we can't use this method and we rely on changing thevisibility
of these containers. To fix this issue while preserving the fade effect, we make inert the containers potentially holding such interactive elements.To facilitate the review of this feature I resurrected the demo app previously used for other SideNav improvements.
🔗 External links
Jira ticket: HDS-2936
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.