✨ Add generic type-safe autocomplete with radio variant (ENG-648)#16539
✨ Add generic type-safe autocomplete with radio variant (ENG-648)#16539Jacobjeevan wants to merge 1 commit into
Conversation
…r HealthcareServiceSelector - Add src/components/ui/generic-autocomplete.tsx: GenericAutocomplete<T> with a default generic matching the old Autocomplete contract (rename-only migration), custom getOptionValue/getOptionLabel/renderOption/renderSelected, and an enableRadio variant gated by RADIO_MAX_OPTIONS=5. - Mark Autocomplete as @deprecated pointing to the new component. - Refactor HealthcareServiceSelector to use GenericAutocomplete.
|
WalkthroughChangesAutocomplete migration
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new generic, type-safe GenericAutocomplete<T> UI component (with an optional radio-group rendering mode for small option sets), deprecates the legacy string-only Autocomplete, and refactors HealthcareServiceSelector to use the new component to reduce bespoke UI code.
Changes:
- Added
GenericAutocomplete<T>with configurable option extractors/renderers, mobile drawer + desktop popover behavior, and a radio variant gated byRADIO_MAX_OPTIONS. - Deprecated legacy
Autocompletevia JSDoc pointing consumers toGenericAutocomplete. - Refactored
HealthcareServiceSelectorto useGenericAutocompleteand custom rendering for service icon/details.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/pages/Facility/services/HealthcareServiceSelector.tsx | Replaced bespoke popover/drawer selector with GenericAutocomplete using custom renderers for service icon and metadata. |
| src/components/ui/generic-autocomplete.tsx | Added new generic autocomplete component, including optional radio-mode rendering for small option counts. |
| src/components/ui/autocomplete.tsx | Marked legacy Autocomplete as deprecated in favor of GenericAutocomplete. |
| return ( | ||
| <GenericAutocomplete<HealthcareServiceReadSpec> | ||
| options={services?.results ?? []} | ||
| value={selected?.id ?? ""} | ||
| onChange={(id) => | ||
| onSelect(services?.results.find((s) => s.id === id) ?? null) | ||
| } | ||
| onOpenChange={setOpen} | ||
| onSearch={setSearchValue} | ||
| isLoading={isFetching} | ||
| filter={false} | ||
| className="min-w-60 justify-start" |
| /** | ||
| * Minimum option count above which the radio variant falls back to the | ||
| * combobox. Radio buttons are only rendered when `enableRadio` is set AND the | ||
| * number of options is within this threshold. | ||
| */ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/components/ui/generic-autocomplete.tsx`:
- Around line 116-118: The default values for placeholder, inputPlaceholder, and
noOptionsMessage in the autocomplete component are hardcoded English. Remove
these literal defaults by making them undefined and translating them at each
callsite, or resolve them through the component’s i18n translation function
using appropriate keys.
- Line 314: Add a stable id to the CommandList or popup element in the generic
autocomplete, then set matching aria-controls attributes on both mobile and
desktop combobox triggers alongside their existing role and aria-expanded props.
In `@src/pages/Facility/services/HealthcareServiceSelector.tsx`:
- Around line 74-82: Update the options passed to GenericAutocomplete in
HealthcareServiceSelector so the preselected service is included alongside
services?.results, avoiding duplicates when it is already present. This ensures
the current value can render its label and icon immediately on mount.
- Around line 28-44: Validate styling_metadata.careIcon against the available
icon data before passing it to CareIcon in ServiceIcon; only call getIconName
for a recognized icon name, otherwise use the existing "d-health-worker"
fallback, preventing invalid keys from causing a lookup error.
🪄 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: 387fbba4-c3cb-49c5-ba22-9dd80ce08a76
📒 Files selected for processing (3)
src/components/ui/autocomplete.tsxsrc/components/ui/generic-autocomplete.tsxsrc/pages/Facility/services/HealthcareServiceSelector.tsx
| placeholder = "Select...", | ||
| inputPlaceholder = "Search option...", | ||
| noOptionsMessage = "No options found", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Default placeholder strings are hardcoded English.
placeholder, inputPlaceholder, and noOptionsMessage default to untranslated literals ("Select...", "Search option...", "No options found"). When a consumer omits these props, untranslated text is shown to users. Prefer defaulting to undefined and translating at the callsite, or resolving via t() inside the component with i18n keys.
As per coding guidelines: "All literal strings must use i18next for multi-language support in healthcare interfaces."
🤖 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/components/ui/generic-autocomplete.tsx` around lines 116 - 118, The
default values for placeholder, inputPlaceholder, and noOptionsMessage in the
autocomplete component are hardcoded English. Remove these literal defaults by
making them undefined and translating them at each callsite, or resolve them
through the component’s i18n translation function using appropriate keys.
Source: Coding guidelines
| } | ||
| variant="outline" | ||
| ref={ref} | ||
| role="combobox" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/components/ui/generic-autocomplete.tsx"
# Show an outline first, then the relevant line ranges around the reported triggers and popup/listbox.
python3 - <<'PY'
from pathlib import Path
path = Path("src/components/ui/generic-autocomplete.tsx")
lines = path.read_text().splitlines()
for start, end in [(250, 410)]:
print(f"\n--- {path}:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: ohcnetwork/care_fe
Length of output: 6579
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("src/components/ui/generic-autocomplete.tsx")
lines = path.read_text().splitlines()
# Print the top imports and the commandContent definition area.
for start, end in [(1, 170), (170, 250)]:
print(f"\n--- {path}:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: ohcnetwork/care_fe
Length of output: 8972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("src/components/ui/command.tsx")
lines = path.read_text().splitlines()
print(f"--- {path} ---")
for i, line in enumerate(lines, 1):
print(f"{i:4d}: {line}")
PYRepository: ohcnetwork/care_fe
Length of output: 6255
Add aria-controls to both combobox triggers. In src/components/ui/generic-autocomplete.tsx, the mobile and desktop buttons expose role="combobox" and aria-expanded but never reference the CommandList/popup, so the trigger isn’t associated with the option list. Give the list a stable id and point both triggers at it.
🧰 Tools
🪛 React Doctor (0.5.8)
[error] 314-314: Screen reader users can't tell the state of this combobox without its required ARIA props, so add aria-controls.
Add every required aria-* attribute so assistive tech can expose the role's state correctly.
(role-has-required-aria-props)
🤖 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/components/ui/generic-autocomplete.tsx` at line 314, Add a stable id to
the CommandList or popup element in the generic autocomplete, then set matching
aria-controls attributes on both mobile and desktop combobox triggers alongside
their existing role and aria-expanded props.
Sources: Coding guidelines, Linters/SAST tools
| const getIconName = (name: string): DuoToneIconName => | ||
| `d-${name}` as DuoToneIconName; | ||
|
|
||
| const ServiceIcon = ({ service }: { service: HealthcareServiceReadSpec }) => ( | ||
| <div className="relative size-6 rounded-sm flex items-center justify-center"> | ||
| <ColoredIndicator | ||
| id={service.id} | ||
| className="absolute inset-0 rounded-sm opacity-20" | ||
| /> | ||
| <CareIcon | ||
| icon={ | ||
| service.styling_metadata?.careIcon | ||
| ? getIconName(service.styling_metadata.careIcon) | ||
| : "d-health-worker" | ||
| } | ||
| className="size-4 relative z-1" | ||
| /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm careIcon value format (prefixed vs bare) and available duo-tone keys
fd -t f 'DuoTonePaths.json' -x sh -c 'echo "== {} =="; jq -r "keys[]" {} | grep -i "health-worker"'
rg -nP "careIcon" -g '*.ts' -g '*.tsx' -C2Repository: ohcnetwork/care_fe
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/CAREUI/icons/CareIcon.tsx
printf '\n---\n'
rg -n "careIcon" src -g '*.ts' -g '*.tsx'
printf '\n---\n'
rg -n "styling_metadata" src -g '*.ts' -g '*.tsx'Repository: ohcnetwork/care_fe
Length of output: 7535
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/pages/Facility/services/HealthcareServiceSelector.tsx
printf '\n---\n'
sed -n '1,260p' src/components/GenericAutocomplete/GenericAutocomplete.tsx
printf '\n---\n'
sed -n '1,220p' src/pages/Facility/settings/healthcareService/HealthcareServiceForm.tsxRepository: ohcnetwork/care_fe
Length of output: 3923
Guard careIcon before passing it to CareIcon
getIconName() casts arbitrary strings to DuoToneIconName, but CareIcon indexes icon data by that key. A bad styling_metadata.careIcon will throw here; validate the name and fall back to "d-health-worker".
🤖 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/Facility/services/HealthcareServiceSelector.tsx` around lines 28 -
44, Validate styling_metadata.careIcon against the available icon data before
passing it to CareIcon in ServiceIcon; only call getIconName for a recognized
icon name, otherwise use the existing "d-health-worker" fallback, preventing
invalid keys from causing a lookup error.
| options={services?.results ?? []} | ||
| value={selected?.id ?? ""} | ||
| onChange={(id) => | ||
| onSelect(services?.results.find((s) => s.id === id) ?? null) | ||
| } | ||
| onOpenChange={setOpen} | ||
| onSearch={setSearchValue} | ||
| isLoading={isFetching} | ||
| filter={false} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find callers that pass a non-null `selected` on mount
rg -nP '<HealthcareServiceSelector' -A6 --type=tsxRepository: ohcnetwork/care_fe
Length of output: 184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== HealthcareServiceSelector definition ==="
sed -n '1,220p' src/pages/Facility/services/HealthcareServiceSelector.tsx
echo
echo "=== Call sites ==="
rg -n --glob '*.tsx' 'HealthcareServiceSelector' src/pages src/components src | sed -n '1,120p'Repository: ohcnetwork/care_fe
Length of output: 5787
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/pages/Facility/services/HealthcareServiceSelector.tsx
printf '\n=== usage ===\n'
rg -n --glob '*.tsx' 'HealthcareServiceSelector' src | sed -n '1,120p'Repository: ohcnetwork/care_fe
Length of output: 4784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/components/Schedule/ResourceSelector.tsx ==="
sed -n '1,180p' src/components/Schedule/ResourceSelector.tsx
echo
echo "=== src/pages/Facility/settings/activityDefinition/ActivityDefinitionForm.tsx (around selector usage) ==="
sed -n '900,980p' src/pages/Facility/settings/activityDefinition/ActivityDefinitionForm.tsxRepository: ohcnetwork/care_fe
Length of output: 6520
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=$(git ls-files | rg 'generic-autocomplete\.(ts|tsx|js|jsx)$' | head -n 1)
echo "=== GenericAutocomplete file: $file ==="
sed -n '1,260p' "$file"Repository: ohcnetwork/care_fe
Length of output: 7780
Seed the selected service into options on mount. GenericAutocomplete only renders the label/icon when it can find the current value in options, so a prefilled service shows the placeholder until the popover loads.
🤖 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/Facility/services/HealthcareServiceSelector.tsx` around lines 74 -
82, Update the options passed to GenericAutocomplete in
HealthcareServiceSelector so the preselected service is included alongside
services?.results, avoiding duplicates when it is already present. This ensures
the current value can render its label and icon immediately on mount.
Greptile SummaryThis PR introduces
Confidence Score: 3/5The new component itself is well-structured, but the HealthcareServiceSelector refactor has a functional regression where users can no longer see which service is currently selected in the open dropdown. The missing isSelected check in HealthcareServiceSelector's custom renderOption is a real UI regression — the check mark that confirmed the active selection in the original code is gone. This affects every user who opens the healthcare service dropdown with a pre-selected value. src/pages/Facility/services/HealthcareServiceSelector.tsx — the renderOption callback needs to use the isSelected argument to restore the selection indicator. Important Files Changed
Reviews (1): Last reviewed commit: "✨ Add generic type-safe autocomplete; de..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| renderOption={(service) => ( | ||
| <div className="flex items-center gap-2 w-full"> | ||
| <ServiceIcon service={service} /> | ||
| <div className="flex flex-col min-w-0 flex-1"> | ||
| <span className="truncate text-sm font-medium" title={service.name}> | ||
| {service.name} | ||
| </span> | ||
| {service.extra_details && ( | ||
| <span className="text-xs text-gray-500 truncate"> | ||
| {service.extra_details} | ||
| </span> | ||
| )} | ||
| </div> | ||
| <span className="truncate">{selected.name}</span> | ||
| </div> | ||
| ) : ( | ||
| <span className="text-gray-400">{t("select_healthcare_service")}</span> | ||
| )} |
There was a problem hiding this comment.
The renderOption callback receives (option: T, isSelected: boolean) but this implementation only uses the first argument — isSelected is silently ignored. In the original code the selected service showed a CheckIcon when selected?.id === service.id. With the custom renderOption here the dropdown list has no visual indicator of the currently selected service, so users can't tell which item is active without looking at the trigger button.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // Radio variant — only when enabled and within the option threshold. | ||
| if (enableRadio && options.length <= RADIO_MAX_OPTIONS) { | ||
| return ( | ||
| <RadioInput | ||
| options={options.map((option) => ({ | ||
| label: getOptionLabel(option), | ||
| value: getOptionValue(option), | ||
| }))} | ||
| value={value} | ||
| onValueChange={onChange} | ||
| disabled={disabled} | ||
| aria-invalid={props["aria-invalid"]} | ||
| className={radioClassName} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The radio/combobox mode decision is based on options.length at render time. When an async caller passes isLoading={true} with an initially-empty options array, 0 <= RADIO_MAX_OPTIONS evaluates to true and an empty RadioInput is rendered while data is in flight. If the fetched data then comes back with more than RADIO_MAX_OPTIONS items the component silently switches to combobox mode mid-session. Consider gating the radio branch on !isLoading as well to avoid the empty flash and the abrupt layout change.
| placeholder = "Select...", | ||
| inputPlaceholder = "Search option...", | ||
| noOptionsMessage = "No options found", |
There was a problem hiding this comment.
The project uses react-i18next for all user-visible text, but these three defaults are hardcoded English strings. Any usage of GenericAutocomplete that relies on the defaults will be untranslated. The t instance is already available inside the function; moving the defaults to resolved translation keys would keep the component consistent with the rest of the codebase.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #10104 |
🥅 What are the changes?
Introduces a generic, type-safe replacement for the string-only
Autocomplete.GenericAutocomplete<T>(src/components/ui/generic-autocomplete.tsx)T = { label; value }) reproduces the existingAutocompleteprop contract exactly, so existing usages migrate by renaming the component only.getOptionValue/getOptionLabel, with optionalrenderOption(list item) andrenderSelected(trigger) for rich content.enableRadiovariant rendering a radio group (reusesRadioInput), gated by a module constRADIO_MAX_OPTIONS = 5— radio renders only when enabled andoptions.length <= 5, otherwise it falls back to the combobox.filter={false}),freeInput, clear button,shortcutId,onOpenChange.Autocompletevia a@deprecatedJSDoc pointing at the new component. The other ~20 usages migrate over time — untouched here.HealthcareServiceSelectorto useGenericAutocomplete(customHealthcareServiceReadSpec), dropping its hand-rolledCommand/Popover/Drawer(~167 fewer lines) while keeping the icon + colored indicator,extra_detailssubtitle, server search, clear, and mobile drawer.🧪 Testing
tsc --noEmit, ESLint (changed files), andnpm run buildall pass.import Autocomplete→import GenericAutocompleteand the JSX tag name; no prop changes for default string usage.Non-goals
Autocompleteusages (done incrementally).multi-select.tsx).Summary by CodeRabbit
New Features
Documentation