Improve keyboard navigation for profile selector#368
Improve keyboard navigation for profile selector#368Shyam-123pandey wants to merge 1 commit intovalhalla:masterfrom
Conversation
|
Please Take a review of this PR. |
There was a problem hiding this comment.
Pull request overview
This PR enhances the ProfilePicker transport-profile selector to better support keyboard users by enabling ArrowLeft/ArrowRight-driven profile switching (addressing accessibility gaps described in #353).
Changes:
- Added an
onKeyDownhandler to profile toggle buttons to react to ArrowLeft/ArrowRight. - Implemented wrap-around cycling through the available profiles.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const handleKeyDown = (event: React.KeyboardEvent, index: number) => { | ||
| if (event.key === 'ArrowRight') { | ||
| event.preventDefault(); | ||
| const next = (index + 1) % profiles.length; | ||
| const nextProfile = profiles[next]; | ||
|
|
||
| if (nextProfile) { | ||
| handleUpdateProfile(nextProfile.value as Profile); | ||
| } | ||
| } | ||
|
|
||
| if (event.key === 'ArrowLeft') { | ||
| event.preventDefault(); | ||
| const prev = (index - 1 + profiles.length) % profiles.length; | ||
| const prevProfile = profiles[prev]; | ||
|
|
||
| if (prevProfile) { | ||
| handleUpdateProfile(prevProfile.value as Profile); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
handleKeyDown changes the selected profile but does not move focus to the newly selected ToggleGroupItem. Because the handler’s index is bound to the originally focused button, subsequent ArrowLeft/ArrowRight presses can keep computing the next/prev from the old index and repeatedly re-select the same profile. Consider implementing roving-focus behavior by programmatically focusing the next/prev item (e.g., keep refs to items and call .focus() on the target), or compute next/prev from the currently selected activeProfile and also guard against no-op updates.
| data-state={profile.value === activeProfile ? 'on' : 'off'} | ||
| onKeyDown={(e) => handleKeyDown(e, i)} | ||
| > |
There was a problem hiding this comment.
The new ArrowLeft/ArrowRight behavior introduced via onKeyDown isn’t covered by tests. Since this component already has a fairly comprehensive profile-picker.spec.tsx, add a test that focuses a profile button and verifies that pressing ArrowRight/ArrowLeft calls resetSettings/onProfileChange with the expected next/previous profile (including wrap-around).
|
Please Take a review of this. Thanks |
|
What's the 1. rule of our GSoC guidelines? Leave us alone! You can see what's going on here, we have > 10 open PRs, all waiting to get reviewed. You won't skip the line by reminding us twice within one hour. What that really does is it's getting your PR closed and scores negative points for your person. The PR description is clearly AI generated, which is wonderfully exemplified by your 1 non-AI super cocky and really unnecessarily passive-aggressive comment. Without those things you would have been a top contender, after all you're the first person to (probably) not use AI to generate the code. But as it stands, your attitude here is so far off my mentality that I don't see any chance for us collaborating. |
Sorry sir don't be angry , of course i used AI for desc and finding files, where can be this files like that. But Sir working continiouly i did two msg for review , don't be angry Please. Like sir i am working whole day for contribution, so due to confuison i did two msg on same PR. Please give one more chance to me move ahead. |
|
if you're feeling mistreated you can try contacting the gsoc admins and complain, that's your right. personally I've said all I can say. |
Currently only the active profile button can be focused using Tab navigation.
Closes: #353
Telling very hosnestly, u can see above 23 line of code can do such improvement and i saw previous PRs change 1000 line of code just for few improvment (that showing all thing using AI) But In my case All of You can see How much effort i am providing not to use AI Blindly
This PR improves accessibility by allowing users to switch between transport
profiles using ArrowLeft and ArrowRight keys.
Changes:
Vedio:
keyboard.controll.mp4