Enhancement: Facility search feature for User Dashboard#16248
Enhancement: Facility search feature for User Dashboard#16248pranavrutagi-egov wants to merge 3 commits intoohcnetwork:developfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
WalkthroughThis PR introduces facility search and recent facility tracking to the user dashboard. A new Jotai atom persists the last visited facility ID in localStorage. The router tracks facility navigation via URL path changes. The dashboard's Facilities tab is enhanced with a search control, recent facility sorting, and responsive UI (Drawer on mobile, Popover on desktop). Changes
🚥 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 unit tests (beta)
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. Comment |
🚀 Preview Deployment Ready!🔗 Preview URL: https://pr-16248.care-preview-a7w.pages.dev 📱 Mobile Access: This preview will be automatically updated when you push new commits to this PR. |
![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 the current code and only fix it if needed.
Inline comments:
In `@public/locale/en.json`:
- Line 3015: Remove the "last_visited" key from its current position (the entry
`"last_visited": "Last Visited",`) and append that exact key/value pair to the
very end of the en.json file so the file remains valid JSON (watch
commas/whitespace). Ensure you only move the existing key (do not duplicate),
preserve the exact label "Last Visited", and validate the file after change so
no trailing-comma or syntax errors are introduced.
In `@src/pages/UserDashboard.tsx`:
- Line 48: The import for useAtomValue from "jotai" is misplaced; move the line
importing useAtomValue so it sits with the other third-party imports at the top
of the file (near React and other external libs) to match the import grouping
used in AppRouter.tsx; update the import order but do not change the symbol name
(useAtomValue) or any references inside the UserDashboard component.
- Around line 333-335: The TabContent call is using the unsorted facilities list
(tabItems={facilities}) while the search dropdown uses sortedFacilities, causing
inconsistent ordering; update the TabContent invocation in UserDashboard (the
component rendering TabContent) to pass sortedFacilities (or the same sorted
array you compute for the search dropdown) as tabItems so the card grid and
search list use the same ordering, ensuring sortedFacilities is
defined/available in that scope and remove or replace any other usages of
facilities for display to avoid divergent orders.
In `@src/Routers/AppRouter.tsx`:
- Around line 33-34: Reorder the imports in AppRouter.tsx so third-party module
imports come first: move useEffect (from "react") and useSetAtom (from "jotai")
up into the top import group with other 3rd-party imports to follow the
project's convention ("3rd-party → library → CAREUI → UI → components → hooks →
utils → relative"); update the import block so useEffect and useSetAtom are
imported alongside other external packages and keep component/library imports
below them.
🪄 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: 18bc59e6-ce28-4e77-958d-fcf68bea046a
📒 Files selected for processing (4)
public/locale/en.jsonsrc/Routers/AppRouter.tsxsrc/atoms/recentFacilityAtom.tssrc/pages/UserDashboard.tsx
| "last_updated": "Last Updated", | ||
| "last_updated_by": "Last updated by", | ||
| "last_vaccinated_on": "Last Vaccinated on", | ||
| "last_visited": "Last Visited", |
There was a problem hiding this comment.
Move this new i18n key to the end of en.json to follow repo rule.
"last_visited" should be appended at the end of public/locale/en.json, not inserted mid-file (Line 3015).
Proposed change
- "last_vaccinated_on": "Last Vaccinated on",
- "last_visited": "Last Visited",
+ "last_vaccinated_on": "Last Vaccinated on",
...
- "zoom_out": "Zoom Out",
+ "zoom_out": "Zoom Out",
+ "last_visited": "Last Visited",
"℞": "℞"As per coding guidelines: "Append missing i18n keys directly to the end of the public/locale/en.json file without reading i18n files".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@public/locale/en.json` at line 3015, Remove the "last_visited" key from its
current position (the entry `"last_visited": "Last Visited",`) and append that
exact key/value pair to the very end of the en.json file so the file remains
valid JSON (watch commas/whitespace). Ensure you only move the existing key (do
not duplicate), preserve the exact label "Last Visited", and validate the file
after change so no trailing-comma or syntax errors are introduced.
| import { FacilityBareMinimum } from "@/types/facility/facility"; | ||
| import { Organization, getOrgLabel } from "@/types/organization/organization"; | ||
| import organizationApi from "@/types/organization/organizationApi"; | ||
| import { useAtomValue } from "jotai"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Move jotai import to align with 3rd-party imports at the top.
Similar to AppRouter.tsx, the jotai import should be grouped with other 3rd-party imports near the top of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/UserDashboard.tsx` at line 48, The import for useAtomValue from
"jotai" is misplaced; move the line importing useAtomValue so it sits with the
other third-party imports at the top of the file (near React and other external
libs) to match the import grouping used in AppRouter.tsx; update the import
order but do not change the symbol name (useAtomValue) or any references inside
the UserDashboard component.
| <TabContent | ||
| tabId="facilities-panel" | ||
| tabItems={facilities} |
There was a problem hiding this comment.
Inconsistent facility ordering between search list and card grid.
The search dropdown uses sortedFacilities (prioritizing recent facility first), but the TabContent card grid renders facilities in original order. This creates inconsistent UX where the "last visited" facility appears first in search but not in the grid view.
Consider using sortedFacilities for the TabContent as well for consistency:
🔧 Suggested fix
<TabContent
tabId="facilities-panel"
- tabItems={facilities}
+ tabItems={sortedFacilities}
description={t("dashboard_tab_facilities")}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TabContent | |
| tabId="facilities-panel" | |
| tabItems={facilities} | |
| <TabContent | |
| tabId="facilities-panel" | |
| tabItems={sortedFacilities} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/UserDashboard.tsx` around lines 333 - 335, The TabContent call is
using the unsorted facilities list (tabItems={facilities}) while the search
dropdown uses sortedFacilities, causing inconsistent ordering; update the
TabContent invocation in UserDashboard (the component rendering TabContent) to
pass sortedFacilities (or the same sorted array you compute for the search
dropdown) as tabItems so the card grid and search list use the same ordering,
ensuring sortedFacilities is defined/available in that scope and remove or
replace any other usages of facilities for display to avoid divergent orders.
| import { useSetAtom } from "jotai"; | ||
| import { useEffect } from "react"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Import ordering does not follow project conventions.
Per coding guidelines, 3rd-party imports (react, jotai) should appear at the top of the file, before library and component imports. Currently useSetAtom from jotai and useEffect from react are placed at the end.
♻️ Suggested import reordering
Move these imports to the top with other 3rd-party imports:
+import { useSetAtom } from "jotai";
+import { useEffect } from "react";
import careConfig from "@careConfig";
import { usePath, useRedirect, useRoutes } from "raviger";
...
-import { useSetAtom } from "jotai";
-import { useEffect } from "react";As per coding guidelines: "Order imports: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Routers/AppRouter.tsx` around lines 33 - 34, Reorder the imports in
AppRouter.tsx so third-party module imports come first: move useEffect (from
"react") and useSetAtom (from "jotai") up into the top import group with other
3rd-party imports to follow the project's convention ("3rd-party → library →
CAREUI → UI → components → hooks → utils → relative"); update the import block
so useEffect and useSetAtom are imported alongside other external packages and
keep component/library imports below them.
🎭 Playwright Test ResultsStatus: ❌ Failed
📊 Detailed results are available in the playwright-final-report artifact. Run: #7825 |
pranavrutagi-egov
changed the title
| const [facilitySearchOpen, setFacilitySearchOpen] = useState(false); | ||
| const recentFacilityId = useAtomValue(recentFacilityAtom); |
There was a problem hiding this comment.
This was not part of the scope 🤔
Why do we need it? What is the user story here?
There was a problem hiding this comment.
The atom persists the last visited facility, and updates when user navigates to some facility related route, thus marking it as last visited
| const sortedFacilities = useMemo(() => { | ||
| if (!recentFacilityId) return facilities; | ||
| return [ | ||
| ...facilities.filter((f) => f.id === recentFacilityId), | ||
| ...facilities.filter((f) => f.id !== recentFacilityId), | ||
| ]; | ||
| }, [facilities, recentFacilityId]); |
There was a problem hiding this comment.
Not a fan of the listing order chainging after each visit.
There was a problem hiding this comment.
The shared UI design had "last visited facility" on top, and I had also mentioned my approach in the issue. Please let me know if this is not needed, I will remove it.
| </Button> | ||
| ); | ||
|
|
||
| const facilitySearchCommand = ( |
There was a problem hiding this comment.
Do we even need a command search 🤔
The server is returning all facilites, why can't we just filter on that list 🤔
There was a problem hiding this comment.
Hi @bodhish, the data for facilities is not being fetched from server. The data is fetched only once from user.facilities || []; (line 67) and used in the search.
|
@nihal467 this looks fine |
nihal467
added
Work in Progress
discussion required
and removed
needs testing
needs review
labels
4 participants
Proposed Changes
Fixes #16220
recentFacilityAtomfor persisting last visited facilityAppRouterfor facility visitPopover(andDrawerfor mobile view) inUserDashboardfor search functionality for facilities based on existing similar implementation inRoleSelectcomponenten.jsonforlast_visitedTagging: @ohcnetwork/care-fe-code-reviewers
Relevent Screenshots:
Dashboard - Facilities Tab:
When focusing on search component:
Searching by facility name:
Merge Checklist
Summary by CodeRabbit