ENG-405 Add infinite pagination for prescription list in encounter#16409
ENG-405 Add infinite pagination for prescription list in encounter#16409NikhilA8606 wants to merge 11 commits into
Conversation
WalkthroughPrescriptionListSelector now uses offset/limit infinite pagination, loads more prescriptions when a sentinel enters view, flattens paged results for lookup and rendering, and passes loading state plus the sentinel ref into PrescriptionList. ChangesInfinite Scroll Implementation
Suggested labels
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying care-preview with
|
| Latest commit: |
1c30026
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a3a25e96.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://eng-405-add-infinite-paginat.care-preview-a7w.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR adds infinite pagination to the encounter prescription selector so the prescriptions sidebar/drawer can load additional pages as the user scrolls, instead of fetching the full list in a single request.
Changes:
- Replaced the single
useQueryprescription fetch withuseInfiniteQueryusinglimit/offset. - Added an intersection-observer trigger to call
fetchNextPage()when the last item comes into view. - Appended a loading skeleton while fetching subsequent pages.
Greptile SummaryThis PR replaces the single
Confidence Score: 3/5The core infinite-pagination logic is correct, but two unresolved functional defects from prior review rounds are still present in PrescriptionListSelector.tsx and should be addressed before merging. The offset/count calculation in getNextPageParam is accurate, the facilityId query-key fix is in place, and the library upgrade is straightforward. However, the single loadMoreRef shared between the always-mounted desktop list and the drawer list means the intersection observer can silently detach after the drawer is closed on mobile, leaving desktop infinite scroll broken for the rest of that session. Additionally, a prescription selected from a page beyond the first batch cannot be resolved to an object from the already-loaded slice, so the mobile trigger button falls back to the select_prescription placeholder even though a valid selection exists. src/components/Medicine/PrescriptionListSelector.tsx — the ref-sharing and selected-item-resolution issues both live here Important Files Changed
Reviews (8): Last reviewed commit: "rename prop name" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Medicine/PrescriptionListSelector.tsx`:
- Around line 66-72: The queryKey for the prescriptions infinite query is
missing facilityId, causing stale cached results when facility changes; update
the queryKey array used where queryKey: ["infinite-prescriptions", patientId,
encounterId] (in PrescriptionListSelector) to include facilityId so it becomes
["infinite-prescriptions", patientId, encounterId, facilityId], ensuring the
queryFn (which calls prescriptionApi.list with facility: facilityId) refetches
when facilityId 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: c7813f98-2ea9-4848-8fd4-509331f5da92
📒 Files selected for processing (1)
src/components/Medicine/PrescriptionListSelector.tsx
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #9735 |
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 (1)
src/components/Medicine/PrescriptionListSelector.tsx (1)
122-130:⚠️ Potential issue | 🟠 Major | ⚡ Quick winShared intersection observer ref may cause infinite scroll to fail on one viewport.
The same
reffromuseInView()is passed to both the desktop and mobilePrescriptionListcomponents. SinceuseInViewtracks a single element, and both lists are rendered simultaneously (CSS-hidden, not conditionally rendered), the ref callback will be invoked for both sentinels. The last assignment wins, so the intersection observer may watch the hidden list's sentinel instead of the visible one.Proposed fix: Use separate refs for each viewport
const { ref, inView } = useInView(); + const { ref: desktopRef, inView: desktopInView } = useInView(); + const { ref: mobileRef, inView: mobileInView } = useInView(); // ... useEffect(() => { - if (inView && hasNextPage) { + if ((desktopInView || mobileInView) && hasNextPage) { fetchNextPage(); } - }, [inView, hasNextPage, fetchNextPage]); + }, [desktopInView, mobileInView, hasNextPage, fetchNextPage]); // Desktop list <PrescriptionList prescriptions={prescriptions} selectedPrescriptionId={selectedPrescriptionId} onSelectPrescription={onSelectPrescription} - ref={ref} + ref={desktopRef} isFetchingNextPage={isFetchingNextPage} /> // Mobile list inside Drawer <PrescriptionList - ref={ref} + ref={mobileRef} isFetchingNextPage={isFetchingNextPage} prescriptions={prescriptions} selectedPrescriptionId={selectedPrescriptionId} onSelectPrescription={handleSelectPrescription} />Also applies to: 188-194
🤖 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/Medicine/PrescriptionListSelector.tsx` around lines 122 - 130, The shared intersection-observer ref passed as ref={ref} to both PrescriptionList instances causes the observer to attach to whichever sentinel was assigned last; fix by creating separate refs/observers for desktop and mobile (e.g., useInView twice: const { ref: desktopRef } = useInView(...) and const { ref: mobileRef } = useInView(...)) and pass desktopRef to the desktop PrescriptionList and mobileRef to the mobile PrescriptionList (update any associated inView handlers/logic accordingly); apply the same change to the other occurrence around lines 188-194 so each viewport has its own observer/sentinel.
🤖 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/components/Medicine/PrescriptionListSelector.tsx`:
- Around line 122-130: The shared intersection-observer ref passed as ref={ref}
to both PrescriptionList instances causes the observer to attach to whichever
sentinel was assigned last; fix by creating separate refs/observers for desktop
and mobile (e.g., useInView twice: const { ref: desktopRef } = useInView(...)
and const { ref: mobileRef } = useInView(...)) and pass desktopRef to the
desktop PrescriptionList and mobileRef to the mobile PrescriptionList (update
any associated inView handlers/logic accordingly); apply the same change to the
other occurrence around lines 188-194 so each viewport has its own
observer/sentinel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c6235204-f567-4813-9a76-b72fc61fcf0f
📒 Files selected for processing (1)
src/components/Medicine/PrescriptionListSelector.tsx
We don't need to manage the inView state just to call fetchNextPage(), so useOnInView lets us react directly to visibility changes without introducing unnecessary state updates and re-renders that's why i used useOnInview here |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Medicine/PrescriptionListSelector.tsx (1)
113-115: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not resolve the selected prescription only from loaded pages.
With infinite pagination, a valid
selectedPrescriptionIdcan refer to a prescription beyond the loaded pages, causing the mobile trigger to fall through toselect_prescription. Ensure the selected prescription is fetched/passed independently, or include it in the initial loaded data.🤖 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/Medicine/PrescriptionListSelector.tsx` around lines 113 - 115, The current selectedPrescription lookup in PrescriptionListSelector only searches the loaded prescriptions array, so a valid selectedPrescriptionId from an unloaded page can be missed. Update the selection flow around selectedPrescriptionId/prescriptions so the chosen prescription is obtained independently of infinite pagination, either by fetching/passing that prescription separately or by ensuring it is included in the initial loaded data. Keep the mobile trigger logic using the resolved prescription from PrescriptionListSelector rather than falling back to select_prescription when the item exists but is not yet in loaded pages.
🤖 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/Medicine/PrescriptionListSelector.tsx`:
- Around line 90-91: The `useOnInView` callback in `PrescriptionListSelector` is
leaving a sentinel debug log on the prescription list scrolling path. Remove the
`console.log` from the `loadMoreRef`/`useOnInView` handler and keep the callback
focused on the load-more behavior only, so repeated in-view events no longer
emit noisy production logs.
---
Outside diff comments:
In `@src/components/Medicine/PrescriptionListSelector.tsx`:
- Around line 113-115: The current selectedPrescription lookup in
PrescriptionListSelector only searches the loaded prescriptions array, so a
valid selectedPrescriptionId from an unloaded page can be missed. Update the
selection flow around selectedPrescriptionId/prescriptions so the chosen
prescription is obtained independently of infinite pagination, either by
fetching/passing that prescription separately or by ensuring it is included in
the initial loaded data. Keep the mobile trigger logic using the resolved
prescription from PrescriptionListSelector rather than falling back to
select_prescription when the item exists but is not yet in loaded pages.
🪄 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: 2bbe875f-e013-49bd-9462-e0600e3288a6
📒 Files selected for processing (1)
src/components/Medicine/PrescriptionListSelector.tsx
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/components/Medicine/PrescriptionListSelector.tsx (2)
111-113: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve selected prescriptions that are not in the loaded page window.
After switching to infinite pagination,
selectedPrescriptiononly searches already fetched pages. IfselectedPrescriptionIdis initialized from route/state for an older prescription beyond the first page, the selector rendersselect_prescriptionand no item appears selected until that page is fetched.Fetch the selected prescription separately or ensure the page containing
selectedPrescriptionIdis loaded before deriving this value.🤖 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/Medicine/PrescriptionListSelector.tsx` around lines 111 - 113, The selected prescription lookup in PrescriptionListSelector only searches the currently loaded prescriptions array, so a route/state-selected prescription from an older page can disappear until that page is fetched. Update the selectedPrescription derivation to either load the prescription matching selectedPrescriptionId separately or ensure the page containing that id is fetched before computing the selected value, so the selector can preserve and render selections outside the current page window.
89-93: 🎯 Functional Correctness | 🟠 MajorSplit the load-more ref between the desktop and drawer lists.
useOnInViewtracks a single observed node, so reusing one ref for bothPrescriptionListinstances lets the last mounted sentinel take over and can break infinite scroll in the other view. Use separate refs for each list and keep the same load-more handler.🤖 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/Medicine/PrescriptionListSelector.tsx` around lines 89 - 93, The load-more sentinel is currently shared by both `PrescriptionList` instances, so `useOnInView` only tracks the last mounted node and can break infinite scroll in one view. Update `PrescriptionListSelector` to create separate in-view refs for the desktop and drawer lists while keeping the same `fetchNextPage` handler logic, and attach each ref to its own `PrescriptionList` load-more element.
🤖 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/components/Medicine/PrescriptionListSelector.tsx`:
- Around line 111-113: The selected prescription lookup in
PrescriptionListSelector only searches the currently loaded prescriptions array,
so a route/state-selected prescription from an older page can disappear until
that page is fetched. Update the selectedPrescription derivation to either load
the prescription matching selectedPrescriptionId separately or ensure the page
containing that id is fetched before computing the selected value, so the
selector can preserve and render selections outside the current page window.
- Around line 89-93: The load-more sentinel is currently shared by both
`PrescriptionList` instances, so `useOnInView` only tracks the last mounted node
and can break infinite scroll in one view. Update `PrescriptionListSelector` to
create separate in-view refs for the desktop and drawer lists while keeping the
same `fetchNextPage` handler logic, and attach each ref to its own
`PrescriptionList` load-more element.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f92b0669-c478-4c99-b147-19d8f3438974
📒 Files selected for processing (1)
src/components/Medicine/PrescriptionListSelector.tsx
Proposed Changes
Added infinite pagination for prescription list in encounter
Screen.Recording.2026-06-02.at.3.48.08.PM.mov
ENG-405
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit