-
Notifications
You must be signed in to change notification settings - Fork 537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: improved tablet view responsiveness #10379
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe changes update the encounter filtering interface in the EncounterList component. Dropdown filters have been replaced by tab triggers to filter encounters by status (e.g., "all", "planned", "in_progress", etc.) and by enhanced encounter class options. The modifications include updates to onClick event handling and query parameter management while adding flexbox layout properties to improve responsiveness. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant E as EncounterList
participant Q as QueryState
U->>E: Clicks filter tab (e.g., "planned")
E->>Q: Update query parameters with selected filter
Q-->>E: Return updated filter settings
E-->>U: Render filtered encounter list
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/pages/Encounters/EncounterList.tsx (3)
356-463
: Consider using a more semantic breakpoint for tablet view.The current implementation uses
md:hidden
which typically triggers at 768px. For better tablet support, consider using a custom breakpoint or thelg
breakpoint (1024px) as tablets commonly range from 768px to 1024px in width.-<div className="md:hidden"> +<div className="lg:hidden">
470-548
: Add aria-label to improve accessibility.The tab triggers lack proper accessibility attributes for screen readers.
<div className="flex flex-wrap"> <TabsTrigger value="all" + aria-label="Show all encounters" className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" onClick={() =>
557-661
: Consider lazy loading for improved performance.The desktop class filter is always rendered but hidden on mobile. Consider lazy loading this component to reduce the initial bundle size and improve performance on mobile devices.
+import { lazy, Suspense } from 'react'; + +const DesktopClassFilter = lazy(() => import('./DesktopClassFilter')); {/* Class Filter - Desktop */} <div className="hidden md:block p-4"> + <Suspense fallback={<div>Loading...</div>}> + <DesktopClassFilter + value={encounterClass} + onValueChange={(value) => + updateQuery({ + status, + priority, + encounter_class: value === "all" ? undefined : value, + }) + } + /> + </Suspense> - <Tabs value={encounterClass || "all"} className="w-full"> - ... - </Tabs> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/Encounters/EncounterList.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
src/pages/Encounters/EncounterList.tsx (2)
217-218
: LGTM! Improved layout structure with flexbox.The new flex container structure provides better organization and responsiveness.
467-551
: Verify tab overflow behavior on tablets.The desktop view's status filter tabs might overflow on tablet screens, especially with the "In Progress" and "Discharged" tabs. Consider testing the following scenarios:
- Text wrapping behavior
- Horizontal scrolling on overflow
- Tab visibility when all filters are displayed
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing i18n
@Tanuj1718 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit