-
Notifications
You must be signed in to change notification settings - Fork 50
feat: add keyboard navigation for court search results #2106
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
base: dev
Are you sure you want to change the base?
feat: add keyboard navigation for court search results #2106
Conversation
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
👷 Deploy request for kleros-v2-testnet-devtools pending review.Visit the deploys page to approve it
|
|
Name | Link |
---|---|
🔨 Latest commit | 05b97b3 |
👷 Deploy request for kleros-v2-university pending review.Visit the deploys page to approve it
|
WalkthroughAdds keyboard navigation to TopSearch: tracks a selectedIndex, handles ArrowUp/ArrowDown/Enter, updates StyledCard to accept a keyboardSelected prop, and refactors input/row handlers to manage navigation, selection highlighting, and navigation to a court. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant TS as TopSearch
participant R as Router
U->>TS: Type in search input
TS->>TS: handleSearchChange()\nupdate query, reset selectedIndex, filter results
U->>TS: Press ArrowDown / ArrowUp
TS->>TS: handleKeyDown()\nupdate selectedIndex -> mark keyboardSelected
U->>TS: Press Enter
TS->>TS: handleKeyDown(Enter)\nresolve selected court
TS->>R: Navigate to court
TS->>TS: Clear search & reset selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (1)
171-178
: Add ARIA combobox semantics (combobox + listbox/option + activedescendant).The feature is “accessible” only if SRs can perceive the active option. Add proper roles/attrs to the input, list, and options, and wire aria-activedescendant to the active option id.
Apply:
<StyledSearchbar dir="auto" type="text" placeholder="Search" value={search} - onChange={handleSearchChange} - onKeyDown={handleKeyDown} + onChange={handleSearchChange} + onKeyDown={handleKeyDown} + role="combobox" + aria-autocomplete="list" + aria-expanded={Boolean(search && filteredCourts.length)} + aria-controls={listboxId} + aria-activedescendant={activeOptionId ?? undefined} /> - {search && filteredCourts.length > 0 && ( - <SearchResultsContainer> + {search && filteredCourts.length > 0 && ( + <SearchResultsContainer id={listboxId} role="listbox"> {filteredCourts.map((court, index) => ( <StyledCard key={court.id} selected={selectedIndex === -1 && court.id === currentCourtId} - keyboardSelected={index === selectedIndex} + $keyboardSelected={index === selectedIndex} + role="option" + aria-selected={index === selectedIndex} + id={`court-option-${court.id}`} onClick={() => handleCourtClick(court.id)} >Add these helper vars near the top of the component (before return):
const listboxId = "court-search-results"; const activeOptionId = selectedIndex >= 0 ? `court-option-${filteredCourts[selectedIndex].id}` : undefined;This keeps focus on the input while exposing the active descendant to AT.
Also applies to: 179-181, 181-187
🧹 Nitpick comments (4)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (4)
75-86
: Stop prop leakage: make keyboardSelected transient ($keyboardSelected).Styled-components will forward unknown props down the tree unless marked transient. Keep using selected (the base Card might rely on it), but switch keyboardSelected to a transient prop to avoid non-standard DOM attributes like keyboardselected="true".
Apply:
-const StyledCard = styled(Card)<{ selected: boolean; keyboardSelected: boolean }>` +const StyledCard = styled(Card)<{ selected: boolean; $keyboardSelected: boolean }>` ${hoverShortTransitionTiming} height: auto; width: 100%; - padding: ${({ selected, keyboardSelected }) => (selected || keyboardSelected ? "16px 13px" : "16px")}; + padding: ${({ selected, $keyboardSelected }) => (selected || $keyboardSelected ? "16px 13px" : "16px")}; cursor: pointer; border: none; - border-left: ${({ selected, keyboardSelected, theme }) => - selected || keyboardSelected ? `3px solid ${theme.primaryBlue}` : "none"}; - background-color: ${({ selected, keyboardSelected, theme }) => - selected || keyboardSelected ? theme.mediumBlue : "transparent"}; + border-left: ${({ selected, $keyboardSelected, theme }) => + selected || $keyboardSelected ? `3px solid ${theme.primaryBlue}` : "none"}; + background-color: ${({ selected, $keyboardSelected, theme }) => + selected || $keyboardSelected ? theme.mediumBlue : "transparent"};And update usage:
- keyboardSelected={index === selectedIndex} + $keyboardSelected={index === selectedIndex}Also applies to: 185-186
119-127
: Ensure ID comparison is type-safe to keep “smart selection” working.Route params are strings; court IDs may be numeric or string. Coerce to string on both sides to avoid silently breaking the “current court” highlighting.
- const selectedCourt = courts.find((c) => c.id === currentCourtId); + const selectedCourt = courts.find((c) => String(c.id) === String(currentCourtId));And in render:
- selected={selectedIndex === -1 && court.id === currentCourtId} + selected={selectedIndex === -1 && String(court.id) === String(currentCourtId)}Optional micro-optimizations:
- Precompute the flattened courts once per data change to avoid re-flattening on every keystroke:
const flattenedCourts = useMemo( () => (data?.court ? flattenCourts(data.court) : []), [data?.court] ); const filteredCourts = useMemo(() => { const q = search.toLowerCase(); const courts = flattenedCourts.filter((c) => c.name.toLowerCase().includes(q)); ... }, [flattenedCourts, search, currentCourtId]);
Also applies to: 184-184
128-149
: Handle Enter/Escape consistently; prevent default on Enter.Prevent the default Enter behavior and add Escape to clear/close results. This avoids stray “submit/beep” behavior and lets users exit navigation quickly.
switch (e.key) { case "ArrowDown": e.preventDefault(); setSelectedIndex((prev) => Math.min(prev + 1, filteredCourts.length - 1)); break; case "ArrowUp": e.preventDefault(); setSelectedIndex((prev) => Math.max(prev - 1, -1)); break; case "Enter": - if (selectedIndex >= 0) { + e.preventDefault(); + if (selectedIndex >= 0) { navigate(`/courts/${filteredCourts[selectedIndex].id}`); setSearch(""); setSelectedIndex(-1); - } - break; + } + break; + case "Escape": + if (search) { + e.preventDefault(); + setSearch(""); + setSelectedIndex(-1); + } + break; }Nice-to-have: support Home/End to jump to first/last result.
128-149
: Keep the highlighted row in view while arrowing.As the user navigates, the active row can scroll out of view. Consider scrolling it into view when selectedIndex changes.
Example:
// inside component const listRef = React.useRef<HTMLDivElement | null>(null); // attach to <SearchResultsContainer ref={listRef} ... /> React.useEffect(() => { if (selectedIndex < 0) return; const el = document.getElementById(`court-option-${filteredCourts[selectedIndex]?.id}`); el?.scrollIntoView({ block: "nearest" }); }, [selectedIndex, filteredCourts]);Also applies to: 181-187
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
web/src/pages/Courts/CourtDetails/TopSearch.tsx
(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (1)
web/src/styles/commonStyles.ts (1)
hoverShortTransitionTiming
(3-5)
🔇 Additional comments (1)
web/src/pages/Courts/CourtDetails/TopSearch.tsx (1)
150-154
: Reset selection on search change and on click — LGTM.State resets are correct and avoid stale/highlighted rows after actions.
Also applies to: 155-159
@tractorss I have tried to fix the issue can you please review it. The problem was in the ArrowUp key handler where the modulo arithmetic could result in selectedIndex becoming -1, causing the navigation bug where users needed to press keys twice to move properly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 🚀
<StyledCard | ||
key={court.id} | ||
selected={court.id === currentCourtId} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution @ManiBAJPAI22 !
I would like to keep the original logic to highlight the selected court, and make it so such that the keyboardSelected
style doesn't clash with it (i.e. no primary-blue left-border). Maybe some kind of outline? also, we have a new version of the ui-components library that will eventually substitute all these components, and this new library has functionality already built-in, although we don't have a date for the migration yet.
Background
Adds accessible keyboard navigation support for the court search results in the Courts page search bar.
New Features
Technical Changes
selectedIndex
state to track keyboard navigationhandleKeyDown
for ArrowUp/ArrowDown/Enter key handlingStyledCard
component to supportkeyboardSelected
propTesting
Files Changed
web/src/pages/Courts/CourtDetails/TopSearch.tsx
- Added keyboard navigation logic and styling♿ Accessibility Impact
This change significantly improves keyboard accessibility for users who rely on keyboard navigation, making the court search functionality more inclusive and user-friendly.
🎯 What does this PR do?
Adds accessible keyboard navigation support for the court search results in the Courts page search bar.
✨ New Features
🔧 Technical Changes
selectedIndex
state to track keyboard navigationhandleKeyDown
for ArrowUp/ArrowDown/Enter key handlingStyledCard
component to supportkeyboardSelected
prop🧪 Testing
📁 Files Changed
web/src/pages/Courts/CourtDetails/TopSearch.tsx
- Added keyboard navigation logic and styling♿ Accessibility Impact
This change significantly improves keyboard accessibility for users who rely on keyboard navigation, making the court search functionality more inclusive and user-friendly.
PR-Codex overview
This PR enhances the
TopSearch
component by adding keyboard navigation support, allowing users to navigate search results using arrow keys. It also modifies the styling ofStyledCard
to accommodate a newkeyboardSelected
prop for visual feedback.Detailed summary
StyledCard
to accept a new propkeyboardSelected
.keyboardSelected
.selectedIndex
state to track the currently selected search result.handleKeyDown
for keyboard navigation (ArrowDown, ArrowUp, Enter).handleSearchChange
to update search input and reset selection.handleCourtClick
to navigate and reset search on court selection.filteredCourts
to applykeyboardSelected
logic.Summary by CodeRabbit
New Features
Style