-
Notifications
You must be signed in to change notification settings - Fork 522
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
FIXED: The facility and appointment pages lack mobile responsiveness #9712
FIXED: The facility and appointment pages lack mobile responsiveness #9712
Conversation
WalkthroughThe pull request focuses on enhancing mobile responsiveness for the Facility Home and Appointments pages. Changes include adjustments to layout, styling, and element sizing to ensure a better user experience across different screen sizes. The modifications primarily involve responsive design techniques such as using Tailwind CSS breakpoint classes, adjusting flex layouts, and modifying element sizes and spacing to create a more adaptive interface. Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Finishing Touches
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 (1)
src/components/Facility/FacilityHome.tsx (1)
240-240
: Consider unifying breakpoints for consistent usability.
Currently,mt-4
is applied for all smaller screens and reset atmd:mt-0
. If you prefer uniform spacing patterns, you could also usesm:mt-0
or consistently usemd:flex-row
in the parent container if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Facility/FacilityHome.tsx
(2 hunks)src/components/Schedule/Appointments/AppointmentsPage.tsx
(2 hunks)
🔇 Additional comments (4)
src/components/Facility/FacilityHome.tsx (1)
226-228
: Responsive layout enhancement looks good.
Switching to a column layout at smaller breakpoints and a row atsm
aligns well with the PR's objective. Good job!src/components/Schedule/Appointments/AppointmentsPage.tsx (3)
252-254
: Good responsive sizing for the Input component.
Usingw-full
on smaller screens andmd:w-[300px]
for larger devices is a clear and effective approach to ensure readability across screen sizes.
261-261
: Full-width Filter button on smaller screens is sensible.
This ensures the button remains accessible on devices with limited horizontal space. Nicely done.
316-325
: ScrollArea usage for the list view is well-implemented.
Wrapping theAppointmentRow
in a scrollable container avoids content overflow. This is consistent with the board view’s horizontal scroll approach.
Check the other pages as well, see if any of the other pages needs responsive fixes and make appropriate changes @modamaan |
👋 Hi, @modamaan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
…modamaan/care_feAmaan into issues/9674/facility-responsive
…modamaan/care_feAmaan into issues/9674/facility-responsive
appointmentsCare.mp4 |
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: 1
🧹 Nitpick comments (2)
src/components/Schedule/Appointments/AppointmentTokenCard.tsx (2)
34-34
: Consider adjusting line height for better mobile readabilityWhile the font size adaptation is good, consider adding line height adjustment for better readability on mobile devices.
- <h3 className="text-base sm:text-lg font-bold tracking-tight"> + <h3 className="text-base leading-snug sm:text-lg font-bold tracking-tight">
66-66
: Consider additional spacing adjustments for token numberThe font size adaptation is good, but consider adding margin adjustments for better visual balance on mobile.
- <p className="text-4xl sm:text-5xl font-bold leading-none"> + <p className="text-4xl sm:text-5xl font-bold leading-none mt-1 sm:mt-0">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Schedule/Appointments/AppointmentTokenCard.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: CodeQL-Build
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/components/Schedule/Appointments/AppointmentTokenCard.tsx (3)
27-27
: Well-implemented responsive card width!The card's width adaptation (full width on mobile, 30rem on larger screens) provides an optimal layout across different devices. The hover effects and transitions also add a nice touch to the user experience.
67-69
: Backend integration needed for token numberThe TODO comment indicates that the token number is currently using a temporary solution. This should be properly integrated with the backend.
Would you like me to help create a GitHub issue to track the backend integration task?
Line range hint
27-69
: Overall excellent mobile-first implementation!The changes effectively improve mobile responsiveness through:
- Consistent use of Tailwind's mobile-first approach
- Well-balanced responsive typography
- Appropriate use of breakpoints
- Maintained visual hierarchy across screen sizes
While I've suggested some minor improvements, the overall implementation successfully addresses the mobile responsiveness objectives.
i pad responsive added loom-video.6.mp4 |
This doesn't look nice on mobile |
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 (1)
src/components/Schedule/Appointments/AppointmentDetailsPage.tsx (1)
176-176
: LGTM! Better mobile margins.Removing horizontal margins on mobile screens (
mx-0 sm:mx-6
) prevents content from being squeezed on small devices.Consider using a consistent spacing scale across breakpoints. You could extract these values into a reusable Tailwind class:
- <div className="mx-0 sm:mx-6 mt-10"> + <div className="mx-0 sm:mx-6 md:mx-8 lg:mx-10 mt-10">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Schedule/Appointments/AppointmentDetailsPage.tsx
(2 hunks)src/components/Schedule/Appointments/AppointmentTokenCard.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Schedule/Appointments/AppointmentTokenCard.tsx
🔇 Additional comments (1)
src/components/Schedule/Appointments/AppointmentDetailsPage.tsx (1)
138-138
: LGTM! Improved tablet layout.The change from
md:flex-row
tolg:flex-row
improves the layout on tablet devices by maintaining a vertical stack until larger screen sizes.To ensure consistent responsive behavior across the application, check other appointment-related pages:
✅ Verification successful
Verified: Different breakpoint is appropriate for details view
The
lg
breakpoint in AppointmentDetailsPage is justified as it handles more complex content compared to other appointment pages that usemd
breakpoint for simpler layouts (filters and lists).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar flex patterns in appointment components rg -l "flex.*flex-(row|col)" src/components/Schedule/Appointments/Length of output: 253
Script:
#!/bin/bash # Search for flex patterns with context in appointment components rg "flex.*flex-(row|col)" src/components/Schedule/Appointments/ -B 2 -A 2Length of output: 3777
what changes should i make ? |
loom-video.7.mp4 |
…modamaan/care_feAmaan into issues/9674/facility-responsive
👋 Hi, @modamaan, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Closing the PR as its inactive. |
@Jacobjeevan @rithviknishad @bodhish can you please reopen the PR, i can complete today itself |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style
Refactor