[ENG-749] feat: add search functionality for facilities and other in Dashboard and Facility Switcher - #16566
[ENG-749] feat: add search functionality for facilities and other in Dashboard and Facility Switcher#16566abhimanyurajeesh wants to merge 5 commits into
Conversation
…and Facility Switcher
WalkthroughChangesThe sidebar facility switcher and dashboard tabs now support client-side search. Matching results, localized empty states, search reset behavior, accessible labels, horizontal tab overflow, and Playwright coverage were added. Facility search
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds client-side facility search to both the dashboard Facilities tab and the sidebar facility switcher dropdown, along with Playwright tests for both surfaces. The filtering is computed from the already-loaded
Confidence Score: 5/5The change is purely additive client-side filtering with no network calls, schema changes, or auth impact — safe to merge. All filtering is done over the already-available No files require special attention; the two minor UX notes are in Important Files Changed
Reviews (3): Last reviewed commit: "Merge branch 'develop' into ENG-749" | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Adds facility-search UX to two key entry points (User Dashboard facilities list and the sidebar Facility Switcher dropdown) and introduces Playwright specs to validate filtering + empty-state behavior.
Changes:
- Add client-side facility filtering + “No facilities found” empty state on the User Dashboard facilities tab.
- Add facility filtering/search input inside the sidebar Facility Switcher dropdown.
- Add Playwright coverage for both dashboard facility search and facility switcher search.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/sidebar/sidebarFacilitySearch.spec.ts | New Playwright spec for facility filtering in the sidebar facility switcher dropdown. |
| tests/sidebar/dashboardFacilitySearch.spec.ts | New Playwright spec for facility filtering on the dashboard facilities view. |
| src/pages/UserDashboard.tsx | Adds dashboard facilities search input and empty state rendering. |
| src/components/ui/sidebar/facility/facility-switcher.tsx | Adds dropdown search input and filtering for facility switcher list. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/sidebar/facility/facility-switcher.tsx`:
- Around line 36-39: Reset searchQuery whenever the facility dropdown closes,
not only when a facility link is selected. Update the FacilitySwitcher menu
close handling around DropdownMenuContent and preserve the existing filtering
behavior while ensuring Escape, outside clicks, and “View Dashboard” leave an
empty query for the next open.
- Around line 36-39: Extract the duplicated case-insensitive facility-name
filtering from facility-switcher’s filteredFacilities and UserDashboard’s
filteredFacilities into a shared utility such as
filterFacilitiesByName(facilities, query). Replace both local implementations
with calls to that helper while preserving the existing matching behavior.
- Around line 92-96: Update the empty-results element in the facility switcher
to expose the “no_facilities_found” message as a screen-reader announcement by
adding an appropriate live-region or status role. Keep the existing styling and
conditional rendering based on filteredFacilities.length unchanged.
In `@src/pages/UserDashboard.tsx`:
- Around line 393-398: Update the empty-results branch in UserDashboard around
EmptyState so the “no facilities found” result change is announced to screen
readers by adding an aria-live="polite" region around the EmptyState or directly
to it, while preserving the existing title and styling.
- Around line 92-96: Extract the case-insensitive facility filtering used by
UserDashboard’s filteredFacilities and facility-switcher’s filteredFacilities
into a shared utility, then update both call sites to use it. Preserve the
current facility.name matching behavior while centralizing future filtering
changes.
🪄 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: 16d1829d-ca89-4ae8-bba2-f6b4d898c850
📒 Files selected for processing (4)
src/components/ui/sidebar/facility/facility-switcher.tsxsrc/pages/UserDashboard.tsxtests/sidebar/dashboardFacilitySearch.spec.tstests/sidebar/sidebarFacilitySearch.spec.ts
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #10543 |
Deploying care-preview with
|
| Latest commit: |
9bd46be
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://53ba18e1.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-749.care-preview-a7w.pages.dev |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
src/pages/UserDashboard.tsx:375
role="tabpanel"sections should be labelled by the corresponding tab button id (e.g.facilities-tab), butaria-labelledbycurrently points back to the panel id (facilities-panel). This breaks the ARIA tabs relationship for screen readers.
className="space-y-3 md:space-y-4"
id={tabId}
role="tabpanel"
aria-labelledby={tabId}
>
tests/facility/sidebar/dashboardFacilitySearch.spec.ts:25
- This assertion searches for the facility name across all links on the page, which can match unrelated navigation and make the test flaky. Scope the locator to the Facilities tab panel so it only checks the facility cards.
// Verify matching facility is still visible
await expect(
page.getByRole("link").filter({ hasText: facilityName }),
).toBeVisible();
tests/facility/sidebar/dashboardFacilitySearch.spec.ts:35
page.getByRole("link").all()counts every link on the dashboard (tabs, header, etc.) so this check doesn’t validate the facility list. Scope to the Facilities panel and assert against the facility-card links only.
// Ensure more than one facility link is shown after clearing search
await searchInput.clear();
const facilities = await page.getByRole("link").all();
expect(facilities.length).toBeGreaterThan(2);
tests/facility/sidebar/sidebarFacilitySearch.spec.ts:43
- Assertions and counts use
page.getByRole("menuitem"), which includes non-facility entries (e.g. “View dashboard”) and can make the checks inaccurate. Use the dropdown menu scope and count only facility links (/facility/...).
// Verify matching facility is still visible
await expect(
page.getByRole("menuitem").filter({ hasText: facilityName }),
).toBeVisible();
});
await test.step("Show empty state for non-matching search", async () => {
await searchInput.fill("zzz_nonexistent_facility_xyz");
await expect(page.getByText(/no facilities found/i)).toBeVisible();
// Ensure more than one facility menuitem is shown after clearing search
await searchInput.clear();
const facilities = await page.getByRole("menuitem").all();
expect(facilities.length).toBeGreaterThan(1);
amjithtitus09
left a comment
There was a problem hiding this comment.
Can think of reusable component if there are more usages in the future
| </Card> | ||
| ))} | ||
| </div> | ||
| ) : tabItems.length === 0 ? ( |
There was a problem hiding this comment.
Specific to facilities, but technically won't ever show up for other tabs. Hmm, I think we can lift this up.
| {/* Tabs Content */} | ||
| <div className="mt-4"> | ||
| {activeTab === DashboardTabs.TAB_FACILITIES && ( | ||
| <TabContent |
There was a problem hiding this comment.
We can just inline a new component here for just facilities with search, input, card etc, instead of changing the shape of TabContent.
If we ever add search to other tabs, we can adjust it then.
| description, | ||
| renderChild, | ||
| isLoading, | ||
| searchComponent, |
There was a problem hiding this comment.
lift these up/remove props
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (5)
src/pages/UserDashboard.tsx:377
aria-labelledbyon the tabpanel points to the panel id (tabId) instead of the corresponding tab button id (e.g.facilities-tab). This breaks the WAI-ARIA tab ↔ panel relationship for screen readers.
role="tabpanel"
aria-labelledby={tabId}
>
<p className="text-sm text-gray-800 font-normal px-1">{description}</p>
{searchComponent}
tests/facility/sidebar/dashboardFacilitySearch.spec.ts:25
- This assertion searches all links on the page, so it can pass even if the facility card/link is missing (e.g. profile/edit links also match role=link). Scope the locator to links within the Facilities panel.
await expect(
page.getByRole("link").filter({ hasText: facilityName }),
).toBeVisible();
tests/facility/sidebar/dashboardFacilitySearch.spec.ts:35
- This counts all links on the page after clearing the search, which doesn’t verify that multiple facility cards re-appeared (it can be satisfied by unrelated nav/profile links). Count only facility links within
#facilities-panel.
await searchInput.clear();
const facilities = await page.getByRole("link").all();
expect(facilities.length).toBeGreaterThan(2);
tests/facility/sidebar/sidebarFacilitySearch.spec.ts:33
getByRole("menuitem")includes non-facility entries (e.g. the "View dashboard" menu item), so this assertion can pass even if the facility list isn’t being filtered correctly. Scope to facility links (href starting with/facility/).
await expect(
page.getByRole("menuitem").filter({ hasText: facilityName }),
).toBeVisible();
tests/facility/sidebar/sidebarFacilitySearch.spec.ts:43
- This counts all menu items in the dropdown, including non-facility items like "View dashboard", so it doesn’t actually verify that multiple facilities are shown after clearing the search. Count only facility menu items/links.
// Ensure more than one facility menuitem is shown after clearing search
await searchInput.clear();
const facilities = await page.getByRole("menuitem").all();
expect(facilities.length).toBeGreaterThan(1);
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/UserDashboard.tsx (1)
31-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove these UI imports with the other UI imports.
Lines 31-32 place UI imports after utility imports. Place them in the UI import group before component, hook, and utility imports.
As per coding guidelines, imports must use the order: third-party, library, CAREUI, UI, components, hooks, utils, relative.
🤖 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/UserDashboard.tsx` around lines 31 - 32, Reorder the EmptyState and Input imports in UserDashboard.tsx so they appear in the UI import group before component, hook, and utility imports, while preserving the required third-party, library, CAREUI, UI, components, hooks, utils, and relative import order.Source: Coding guidelines
🤖 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 `@tests/facility/dashboard/dashboardSearch.spec.ts`:
- Around line 17-21: In tests/facility/dashboard/dashboardSearch.spec.ts at
lines 17-21 and 55-59, update both search-input setup blocks to assert
visibility with expect(searchInput).toBeVisible() instead of catching visibility
failures and calling test.skip(); keep the tests failing when the required
search control is absent.
- Around line 62-69: Update the search setup in dashboardSearch.spec.ts to
select a responsibility name that is visible in the target organization before
filling searchInput, rather than choosing an arbitrary role label. Remove the
toHaveCount(1) assertion and retain only the visibility check for the matching
organization link.
---
Outside diff comments:
In `@src/pages/UserDashboard.tsx`:
- Around line 31-32: Reorder the EmptyState and Input imports in
UserDashboard.tsx so they appear in the UI import group before component, hook,
and utility imports, while preserving the required third-party, library, CAREUI,
UI, components, hooks, utils, and relative import order.
🪄 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: a0489c80-f8a7-41aa-99d3-16b9739f721d
📒 Files selected for processing (2)
src/pages/UserDashboard.tsxtests/facility/dashboard/dashboardSearch.spec.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (6)
src/pages/UserDashboard.tsx:354
aria-labelledbyshould reference the controlling tab’s id (e.g. "facilities-tab"), not the panel’s own id. As written, the tabpanel labels itself, which breaks the tab/tabpanel accessible relationship.
<section
className="space-y-3 md:space-y-4"
id={tabId}
role="tabpanel"
aria-labelledby={tabId}
tests/facility/sidebar/sidebarFacilitySearch.spec.ts:43
page.getByRole("menuitem").all()includes non-facility items like “View dashboard”, so this assertion can pass even when only one facility is listed. Count only menuitems that contain a facility link.
// Ensure more than one facility menuitem is shown after clearing search
await searchInput.clear();
const facilities = await page.getByRole("menuitem").all();
expect(facilities.length).toBeGreaterThan(1);
src/components/ui/sidebar/facility/facility-switcher.tsx:5
useStateis imported after internal modules, breaking the repo’s import-order convention (3rd-party imports should be grouped first; see AGENTS.md:17). Move the React import up with the other 3rd-party imports.
import { CaretSortIcon, DashboardIcon } from "@radix-ui/react-icons";
import { Hospital, Search } from "lucide-react";
import { Link } from "raviger";
import { useTranslation } from "react-i18next";
src/components/ui/sidebar/facility/facility-switcher.tsx:39
- The facility search doesn’t trim whitespace, so queries like " foo" won’t match even when the facility exists. Trim + normalize the query (similar to the dashboard search) before filtering.
const [searchQuery, setSearchQuery] = useState("");
const filteredFacilities = facilities.filter((facility) =>
facility.name.toLowerCase().includes(searchQuery.toLowerCase()),
);
tests/facility/dashboard/dashboardSearch.spec.ts:2
fakeris only used to pick a random responsibility later in this spec. That randomness makes the test non-deterministic; once removed, this import becomes unused and should be dropped to avoid lint failures.
import { faker } from "@faker-js/faker";
import { expect, test } from "@playwright/test";
tests/facility/dashboard/dashboardSearch.spec.ts:70
- This test randomly searches for "Doctor" or "Staff", but the dashboard’s Responsibilities search filters by organization name (
item.name), so the assertion can be flaky or incorrect depending on fixture data. Use an existing rendered responsibility name from the panel to keep the test deterministic and aligned with the UI filter.
await test.step("Search for a random responsibility", async () => {
const responsibility = faker.helpers.arrayElement(["Doctor", "Staff"]);
await searchInput.fill(responsibility);
Proposed Changes
Closed PR #16248
Fixes #16220
Related to ENG-749
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit