fix(search): navigation clavier dans les résultats Cmd+K#259
Merged
Conversation
The Cmd+K palette had no keyboard navigation: users had to leave the keyboard and click to pick a result. Add Arrow Up/Down to move the selection and Enter to open it. The hovered row syncs with the keyboard cursor and the selected row scrolls into view as needed.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGlobalSearch suit l’index de sélection, borne l’index selon la taille des résultats, réinitialise l’index à 0 sur sélection/clear/Escape, permet navigation clavier (ArrowUp/ArrowDown/Enter), fait défiler l’option active en vue et remplace le rendu par des rôles ARIA listbox/option avec IDs stables et refs indexées. (≤50 mots) ChangesNavigation et Surlignage des Résultats
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
scope: frontend
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/global-search.tsx`:
- Around line 175-179: The input's keyboard selection isn't exposed to screen
readers; wire up ARIA so SRs announce the active item: give the combobox
container role="combobox" (or use aria-haspopup/listbox pattern), add
aria-expanded and aria-owns/aria-controls linking the Input to the suggestions
list, add aria-activedescendant on the Input pointing to the id of the currently
highlighted suggestion (use the existing activeIndex/activeSuggestionIndex
state), and ensure each suggestion element rendered in the list has a stable id
(e.g., getSuggestionId(index)), role="option", and aria-selected={index ===
activeIndex}; update these attributes in the handlers (handleQueryChange,
handleInputKeyDown) so activedescendant and aria-selected reflect the
highlighted item.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4aa53784-91be-4051-b987-f7b3dd7ff161
📒 Files selected for processing (1)
apps/web/src/components/global-search.tsx
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/global-search.tsx`:
- Around line 179-190: The combobox currently always sets aria-owns and
aria-controls to LISTBOX_ID even when the listbox isn't rendered (e.g.
query.length < 2 or no results), breaking the ARIA relationship; update the JSX
around the div with role="combobox" / the Input props (where LISTBOX_ID, isOpen,
query and activeDescendantId are used) to only include aria-owns and
aria-controls when the listbox will actually be rendered (e.g. isOpen &&
query.length >= 2 && resultsExist), otherwise omit those attributes (or set them
to undefined/null) so the ARIA relationship is only present when the listbox
exists. Ensure activeDescendantId handling remains consistent with the same
render condition.
- Around line 125-140: The ArrowUp/ArrowDown handlers in handleInputKeyDown move
relative to the raw selectedIndex value, which can be out-of-bounds if results
changed; change them to step from the clamp-safe activeIndex instead. Replace
setSelectedIndex((i) => Math.min(i + 1, results.length - 1)) with
setSelectedIndex(() => Math.min(activeIndex + 1, results.length - 1)) and
similarly replace setSelectedIndex((i) => Math.max(i - 1, 0)) with
setSelectedIndex(() => Math.max(activeIndex - 1, 0)), keeping
handleInputKeyDown, results, activeIndex and handleSelect references.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6e76a01-3ec5-4997-bdef-af3f17277714
📒 Files selected for processing (1)
apps/web/src/components/global-search.tsx
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/web/src/components/global-search.tsx (1)
180-180:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
aria-expandeddevrait refléter la visibilité réelle de la listbox.Actuellement
aria-expanded={isOpen}esttruemême quand la listbox n'est pas affichée (requête trop courte ou zéro résultat). Les lecteurs d'écran annoncent "développé" alors qu'il n'y a rien à sélectionner.🔧 Correctif proposé
- <div className="relative" role="combobox" aria-expanded={isOpen} aria-haspopup="listbox" aria-owns={shouldRenderListbox ? LISTBOX_ID : undefined}> + <div className="relative" role="combobox" aria-expanded={shouldRenderListbox} aria-haspopup="listbox" aria-owns={shouldRenderListbox ? LISTBOX_ID : undefined}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/global-search.tsx` at line 180, Le composant expose currently aria-expanded={isOpen} même quand la listbox n'est pas rendue, ce qui fait annoncer "développé" sans éléments; changez l'attribut aria-expanded sur l'élément role="combobox" pour refléter la visibilité réelle de la listbox en utilisant shouldRenderListbox (ou isOpen && shouldRenderListbox) au lieu de isOpen seul; repérez la div avec role="combobox" qui utilise isOpen, LISTBOX_ID et shouldRenderListbox et mettez à jour aria-expanded pour n'être true que quand la listbox est effectivement rendue.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@apps/web/src/components/global-search.tsx`:
- Line 180: Le composant expose currently aria-expanded={isOpen} même quand la
listbox n'est pas rendue, ce qui fait annoncer "développé" sans éléments;
changez l'attribut aria-expanded sur l'élément role="combobox" pour refléter la
visibilité réelle de la listbox en utilisant shouldRenderListbox (ou isOpen &&
shouldRenderListbox) au lieu de isOpen seul; repérez la div avec role="combobox"
qui utilise isOpen, LISTBOX_ID et shouldRenderListbox et mettez à jour
aria-expanded pour n'être true que quand la listbox est effectivement rendue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d4b10a5-8c76-450e-88d4-966b9e61b682
📒 Files selected for processing (1)
apps/web/src/components/global-search.tsx
Repository owner
unlocked this conversation
May 5, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
La palette Cmd+K affichait les résultats mais aucun moyen clavier de les sélectionner — il fallait lâcher le clavier et cliquer. Ce PR ajoute :
Implémentation React Compiler-friendly : pas de
setStatedansuseEffect, calculactiveIndexdérivé pour clamp si la data change pendant la nav.Test plan
Summary by CodeRabbit