-
-
Notifications
You must be signed in to change notification settings - Fork 171
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
improvement: a11y: arrow-key navigation for messages #4294
Draft
WofWca
wants to merge
9
commits into
main
Choose a base branch
from
wofwca/roving-tabindex-for-messages
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WofWca
force-pushed
the
wofwca/roving-tabindex-for-messages
branch
from
October 30, 2024 16:18
21857cf
to
901f71c
Compare
WofWca
changed the title
WIP: improvement: a11y: arrow-key navigation for messages
improvement: a11y: arrow-key navigation for messages
Oct 30, 2024
WofWca
force-pushed
the
wofwca/roving-tabindex-for-messages
branch
3 times, most recently
from
October 31, 2024 10:50
6db255c
to
1d35935
Compare
6 tasks
WofWca
force-pushed
the
wofwca/roving-tabindex-for-messages
branch
from
November 3, 2024 09:33
1d35935
to
fe5b337
Compare
This was referenced Dec 19, 2024
WofWca
force-pushed
the
wofwca/roving-tabindex-for-messages
branch
from
December 19, 2024 13:50
fe5b337
to
69a0780
Compare
WofWca
force-pushed
the
wofwca/roving-tabindex-for-messages
branch
from
December 19, 2024 14:16
69a0780
to
0406c54
Compare
- Author name in MessageList - Author avatar in MessageList
WofWca
force-pushed
the
wofwca/roving-tabindex-for-messages
branch
from
December 20, 2024 14:26
0406c54
to
a3fcd33
Compare
- Join video chat - Info messages - "Show full message" (HTML)
Although I'm not quite sure if we want it to be a button when clicking on it does nothing.
Closes #2141. Basically what this commit comes down to: 1. Apply `useRovingTabindex` for message items 2. Set `tabindex="-1"` on all the interactive items inside every message that is currently not the active one, so that they do no have tab stops.
WofWca
force-pushed
the
wofwca/roving-tabindex-for-messages
branch
from
December 20, 2024 15:13
a3fcd33
to
b0f7112
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
roving-tabindex-messages.mp4
Based on #4291
Closes #2141
Basically what this commit comes down to:
useRovingTabindex
for message itemstabindex="-1"
on all the interactive itemsinside every message that is currently not the active one,
so that they do no have tab stops.
TODO:
because initially they're all active, so
tabbing to the messages list from the top selects
the first rendered one as the active one.
WIP: refactor:
useRovingTabindex
to utilize IDs #4292could help with this.
And remember that the messages list is dynamic (a new message can arrive), so simply setting the last one as active once won't cut it.
This is also not great for performance: changing
tabindex
on a bunch of messages makes them all re-render.
And otherwise, we probably want to update which one is
the active one as new messages arrive.
Perhaps we can simply implement the "End" shortcut as sort of a workaround for now.
jumpToMessage
calls probably ought to focus the message. For example, when jumping to a message through a click on a quote, or a webxdc info message. But not alljumpToMessage
should change focus. For example, receiving a new message shouldn't do it. This probably requires that we add afocus
parameter tojumpToMessage
. Perhaps even 2 parameters: whether to set the message as the active one in the messages list (without focusing it), and whether to focus it. But consider the fact that then the composer will be blurred and will have to be manually focused.This can be done in further MRs.
onClick
must be actual semantic<button>
s.See improvement: improve a11y #4210
for reference.
Namely, this goes for
Attachment
, and all theonClick
listeners inMessage.ts
.Update: here is an MR: improvement: a11y: turn more elements into <button>s #4429
tabindex
on<video>
and<audio>
onFocus
work? Does clicking something inside a message make that message the active one?onKeyDown
. Does pressing an arrow key when a child element is focused switch the active message?FYI This also applies to audios in gallery, see improvement: a11y: add arrow-key nav to gallery #4376