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

improvement: add arrow-key nav everywhere #4369

Merged
merged 10 commits into from
Dec 3, 2024
Merged

Conversation

WofWca
Copy link
Collaborator

@WofWca WofWca commented Nov 28, 2024

I have shallowly tested all the places where added it.

This completes arrow-key navigation for all the chat lists and contact lists, unless I missed something.

Follow up to #4224, #4291, #4361, #4362.

For easier review, hide white-space changes.
This MR includes one small refactor commit, otherwise it's just boilerplate.

And about boilerplate: please tell me whether in general this looks like a good way to write this, or if this feels wrong and unmaintainable.

@WofWca WofWca force-pushed the wofwca/arrow-keys-everywhere branch from 531ae69 to ac7df20 Compare November 29, 2024 14:36
@WofWca WofWca force-pushed the wofwca/arrow-keys-everywhere branch from ac7df20 to 0c29803 Compare November 29, 2024 14:58
This is needed so that we can use hooks inside the
`ReactionsDialogListItem`, namely the `useRovingTabindex` hook.
@WofWca WofWca changed the title WIP: improvement: add arrow-key nav everywhere improvement: add arrow-key nav everywhere Nov 29, 2024
@WofWca WofWca marked this pull request as ready for review November 29, 2024 16:17
Copy link
Member

@nicodh nicodh left a comment

Choose a reason for hiding this comment

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

I didn't test all places, but compared the changes with --ignore-space-change

LGTM!

@WofWca WofWca merged commit bef76c9 into main Dec 3, 2024
10 checks passed
@WofWca WofWca deleted the wofwca/arrow-keys-everywhere branch December 3, 2024 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants