Improve keyboard accessibility#371
Conversation
| activeOverlayRef.current = node; | ||
|
|
||
| // Auto focus the overlay that's currently selected | ||
| node?.focus(); |
There was a problem hiding this comment.
This part doesn't seem to be working and I can't really tell why?
There was a problem hiding this comment.
What is this meant to do that it isn't doing? Focus the entire active overlay (e.g. the entire ambassadors list)?
There was a problem hiding this comment.
Yeah, it's supposed to focus the active overlay that was just opened like how it does when setting the active ambassador
There was a problem hiding this comment.
src/pages/overlay/components/overlay/Ambassadors.tsx doesn't take a ref, and doesn't have a tabIndex, so that might be why?
(nor does src/pages/overlay/components/overlay/Settings.tsx, and src/components/Welcome.tsx has a ref but not tabIndex)
There was a problem hiding this comment.
The root element in Ambassadors.tsx doesn't take in a ref, but the parent element of the AmbassadorButton does and that's what it's trying to focus
Interestingly adding a setTimeout around the node?.focus() does cause the element to be focused properly, so I think it's something similar to https://stackoverflow.com/a/76033876 where it's just not triggering a re-render
|
Sorry for the delay on getting to this @flakey5, been pretty busy with some upcoming secret bits for Alveus. Any chance you could rebase this? |
MattIPv4
left a comment
MattIPv4
left a comment
There was a problem hiding this comment.
Sorry for taking so long to get to this
| activeOverlayRef.current = node; | ||
|
|
||
| // Auto focus the overlay that's currently selected | ||
| node?.focus(); |
There was a problem hiding this comment.
What is this meant to do that it isn't doing? Focus the entire active overlay (e.g. the entire ambassadors list)?
No worries at all! Addressed some of the review comments in latest commit, trying to see if there's a better way to detect when we should pause the sleeping timer though. Right now when we detect that we're focusing the app it also detects when we focus on child elements, so every time we tab to different overlays, ambassadors, etc. the event fires and calls caffeinate and the same happens with onBlur/uncaffeinate. I feel like ideally we can just detect when we app is focused/unfocused, but I'm not fully familiar with front end stuff to know if there's an actual good way to do that, wdyt? |
|
Yeh, I think my concern is that the focus event can be fired by things other than keyboard navigation -- clicking on a button or really anything with a tab index will generally cause it to become focused, I think? |
Closes alveusgg#139 Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
|
Apologizes for the delay on this - moving this back into the draft state for now. Current todos:
|
2 participants
Closes #139