[ENG-524] Optimized Encounter ID search by removing redundant React state#16488
[ENG-524] Optimized Encounter ID search by removing redundant React state#16488NikhilA8606 wants to merge 2 commits into
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughEncounterList’s patient search now derives its mode from ChangesSearch UI Refactor
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
NikhilA8606
changed the title
Greptile SummaryThis PR simplifies the encounter search control by removing three pieces of redundant local state (
Confidence Score: 5/5Safe to merge — the refactoring removes state duplication without changing any data-fetching logic or committed-search behaviour. The change is a focused state-reduction refactor on a single UI component. The query key and API call parameters remain driven by No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "Copilot suggestions" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Refactors the Encounter list “search by Encounter ID / patient name” UI to reduce duplicated React state and rely more on the Command component’s built-in selection/keyboard behavior, keeping search type derived from query params.
Changes:
- Removes duplicated state (
searchWith,activeSearchOptionIndex,searchInputRef) and simplifies syncingsearchTextfromqParams. - Replaces custom keyboard navigation/styling with
CommandInput+CommandItemselection behavior. - Restructures the search-type popover anchoring and open/close interactions.
| <Command | ||
| shouldFilter={false} | ||
| className="overflow-visible bg-transparent" | ||
| > | ||
| <PopoverTrigger asChild> | ||
| <div className="relative"> | ||
| <input | ||
| ref={searchInputRef} | ||
| <Popover | ||
| open={searchOptionsOpen && showSearchOptions} | ||
| onOpenChange={setSearchOptionsOpen} | ||
| > | ||
| <PopoverAnchor asChild> | ||
| <CommandInput | ||
| value={searchText} | ||
| onChange={(event) => { | ||
| const value = event.target.value; | ||
| onValueChange={(value) => { | ||
| setSearchText(value); | ||
| setSearchOptionsOpen(value.trim() !== ""); | ||
| setActiveSearchOptionIndex(0); | ||
| }} | ||
| onFocus={() => { | ||
| if (showSearchOptions) { | ||
| setSearchOptionsOpen(true); | ||
| onClick={() => { | ||
| if (showSearchOptions) setSearchOptionsOpen(true); | ||
| }} | ||
| onKeyDown={(event) => { | ||
| if (event.key === "Escape") { | ||
| setSearchOptionsOpen(false); | ||
| return; | ||
| } | ||
| if (event.key === "Enter" && !searchOptionsOpen) { | ||
| event.preventDefault(); | ||
| updateSearchQuery(searchWith, searchText); | ||
| } | ||
| }} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/pages/Encounters/EncounterList.tsx`:
- Around line 196-201: Keep the API search params derived only from committed
qParams, not from the draft searchText state. In EncounterList, update
searchQueryParams (and any callers such as the useEffect and the
data-fetching/query-key logic) so the request payload only uses
qParams.external_identifier/qParams.name and other committed filter values; do
not let unsubmitted searchText affect the query when filters or pagination
change. Use the existing searchWith, searchQueryParams, and qParams symbols to
keep the URL and fetched results in sync.
- Around line 587-607: The search field in EncounterList’s CommandInput relies
only on placeholder text, so it needs an explicit accessible name. Update the
CommandInput usage to add an aria-label that clearly identifies the search
control, while keeping the existing placeholder and handlers in place. Use the
CommandInput component in EncounterList as the target for this accessibility
fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8efbc7a6-1099-4c85-8837-7b471e101996
📒 Files selected for processing (1)
src/pages/Encounters/EncounterList.tsx
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #9855 |
Deploying care-preview with
|
| Latest commit: |
725852a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c202cdbb.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-524-audit-react-state-us.care-preview-a7w.pages.dev |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/Encounters/EncounterList.tsx (2)
615-641: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the active cmdk item synced to
searchWithsrc/pages/Encounters/EncounterList.tsx:615-641— thisCommandis uncontrolled, so cmdk keeps the first item active by default. When the popover is open, Enter can selectnameeven whileexternal_identifieris active. Pass the current mode asvalueso the highlighted item matches the active search mode.🤖 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 `@src/pages/Encounters/EncounterList.tsx` around lines 615 - 641, The Command menu in EncounterList is uncontrolled, so cmdk can keep the first item active even when a different search mode is selected. Update the Command setup around searchWithOptions, CommandList, and CommandItem so the current searchWith state is passed as the Command value, keeping the highlighted item synchronized with searchWith and ensuring Enter selects the active mode rather than the first option.
586-609: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep the input click from closing the open options list.
PopoverTrigger asChildforwards Radix’s toggle behavior toCommandInput, so clicking inside the already-open field to move the caret closes the list and focus alone won’t reopen it. Move the trigger off the input or ignore trigger clicks while the popover is open.🤖 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 `@src/pages/Encounters/EncounterList.tsx` around lines 586 - 609, Keep the search input from toggling the popover closed when it is already open. In EncounterList’s CommandInput/PopoverTrigger wiring, Radix’s asChild trigger behavior is causing clicks on the focused field to close the options list, so change the interaction so the input is not the active trigger or clicks are ignored while searchOptionsOpen is true. Make sure the fix preserves typing, caret movement, and the existing setSearchOptionsOpen/showSearchOptions behavior around CommandInput and PopoverTrigger.
🤖 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.
Outside diff comments:
In `@src/pages/Encounters/EncounterList.tsx`:
- Around line 615-641: The Command menu in EncounterList is uncontrolled, so
cmdk can keep the first item active even when a different search mode is
selected. Update the Command setup around searchWithOptions, CommandList, and
CommandItem so the current searchWith state is passed as the Command value,
keeping the highlighted item synchronized with searchWith and ensuring Enter
selects the active mode rather than the first option.
- Around line 586-609: Keep the search input from toggling the popover closed
when it is already open. In EncounterList’s CommandInput/PopoverTrigger wiring,
Radix’s asChild trigger behavior is causing clicks on the focused field to close
the options list, so change the interaction so the input is not the active
trigger or clicks are ignored while searchOptionsOpen is true. Make sure the fix
preserves typing, caret movement, and the existing
setSearchOptionsOpen/showSearchOptions behavior around CommandInput and
PopoverTrigger.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7727e180-9ace-4992-9c20-fec2ca98b3af
📒 Files selected for processing (1)
src/pages/Encounters/EncounterList.tsx
ohc-project-manager
Bot
added
Changes Required
and removed
needs review
needs testing
labels
Proposed Changes
Commandcomponent now handles the selected item styling automatically.Commandinbuilt keyboard navigationScreen.Recording.2026-07-01.at.1.10.58.AM.mov
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes