-
Notifications
You must be signed in to change notification settings - Fork 449
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
phone size responsive issue shifting details page #8831 Open #9256
phone size responsive issue shifting details page #8831 Open #9256
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
🧹 Outside diff range and nitpick comments (2)
src/components/Shifting/ShiftDetails.tsx (2)
483-493
: Improved mobile responsiveness for action buttonsThe flex container with wrap and gap properties provides better button layout on mobile devices. However, consider adding
flex-col
for mobile to stack buttons vertically for better touch targets.- <div className="flex flex-wrap gap-2 mt-4"> + <div className="flex flex-col sm:flex-row flex-wrap gap-2 mt-4">🧰 Tools
🪛 eslint
[error] 492-492: Replace
data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"
with⏎··················data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"⏎················
(prettier/prettier)
[error] 493-493: Replace
·navigate(
/shifting/${data?.external_id}/update)
with⏎··················navigate(
/shifting/${data?.external_id}/update)⏎················
(prettier/prettier)
485-485
: Consider adding loading state to update buttonThe button's disabled state is correctly handled, but since it navigates to another route, consider adding a loading state to prevent double-clicks.
+ const [isNavigating, setIsNavigating] = useState(false); // ... <ButtonV2 className="w-full sm:w-auto" tooltip={ ["COMPLETED", "CANCELLED"].includes(data?.status || "") ? `A shifting request, once ${data?.status.toLowerCase()} cannot be updated` : "" } tooltipClassName="tooltip-top -translate-x-28 -translate-y-1 text-xs" - disabled={data?.status === "COMPLETED" || data?.status === "CANCELLED"} + disabled={data?.status === "COMPLETED" || data?.status === "CANCELLED" || isNavigating} - onClick={() => navigate(`/shifting/${data?.external_id}/update`)} + onClick={() => { + setIsNavigating(true); + navigate(`/shifting/${data?.external_id}/update`); + }} > - {t("update_status_details")} + {isNavigating ? t("navigating") : t("update_status_details")} </ButtonV2>Also applies to: 492-492, 493-493
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Shifting/ShiftDetails.tsx
(1 hunks)
🧰 Additional context used
🪛 eslint
src/components/Shifting/ShiftDetails.tsx
[error] 492-492: Replace data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"
with ⏎··················data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"⏎················
(prettier/prettier)
[error] 493-493: Replace ·navigate(
/shifting/${data?.external_id}/update)
with ⏎··················navigate(
/shifting/${data?.external_id}/update)⏎················
(prettier/prettier)
[error] 498-498: Replace ·className="w-full·sm:w-auto"·onClick={()·=>·setIsPrintMode(true)}
with ⏎················className="w-full·sm:w-auto"⏎················onClick={()·=>·setIsPrintMode(true)}⏎··············
(prettier/prettier)
🔇 Additional comments (2)
src/components/Shifting/ShiftDetails.tsx (2)
498-498
: LGTM: Print button responsive width
The responsive width classes for the print button are correctly implemented, ensuring proper button sizing across different screen sizes.
🧰 Tools
🪛 eslint
[error] 498-498: Replace ·className="w-full·sm:w-auto"·onClick={()·=>·setIsPrintMode(true)}
with ⏎················className="w-full·sm:w-auto"⏎················onClick={()·=>·setIsPrintMode(true)}⏎··············
(prettier/prettier)
483-498
: Changes align with PR objectives
The responsive design changes effectively address the phone size responsive issues mentioned in issue #8831. The implementation follows mobile-first design principles and maintains functionality across all screen sizes.
🧰 Tools
🪛 eslint
[error] 492-492: Replace data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"
with ⏎··················data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"⏎················
(prettier/prettier)
[error] 493-493: Replace ·navigate(
/shifting/${data?.external_id}/update)
with ⏎··················navigate(
/shifting/${data?.external_id}/update)⏎················
(prettier/prettier)
[error] 498-498: Replace ·className="w-full·sm:w-auto"·onClick={()·=>·setIsPrintMode(true)}
with ⏎················className="w-full·sm:w-auto"⏎················onClick={()·=>·setIsPrintMode(true)}⏎··············
(prettier/prettier)
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
🧹 Outside diff range and nitpick comments (2)
src/components/Shifting/ShiftDetails.tsx (2)
488-496
: Fix formatting while maintaining responsive implementationThe responsive width implementation looks good, but there are some formatting issues to address.
Apply this formatting fix:
<ButtonV2 className="w-full sm:w-auto" tooltip={ ["COMPLETED", "CANCELLED"].includes(data?.status || "") ? `A shifting request, once ${data?.status.toLowerCase()} cannot be updated` : "" } tooltipClassName="tooltip-top -translate-x-28 -translate-y-1 text-xs" - disabled={data?.status === "COMPLETED" || data?.status === "CANCELLED"} - onClick={() => navigate(`/shifting/${data?.external_id}/update`)} + disabled={ + data?.status === "COMPLETED" || data?.status === "CANCELLED" + } + onClick={() => + navigate(`/shifting/${data?.external_id}/update`) + } >🧰 Tools
🪛 eslint
[error] 495-495: Replace
data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"
with⏎··················data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"⏎················
(prettier/prettier)
[error] 496-496: Replace
·navigate(
/shifting/${data?.external_id}/update)
with⏎··················navigate(
/shifting/${data?.external_id}/update)⏎················
(prettier/prettier)
501-501
: Fix formatting while maintaining consistent responsive designThe responsive width implementation matches the update button, maintaining consistency.
Apply this formatting fix:
- <ButtonV2 className="w-full sm:w-auto" onClick={() => setIsPrintMode(true)}> + <ButtonV2 + className="w-full sm:w-auto" + onClick={() => setIsPrintMode(true)} + >🧰 Tools
🪛 eslint
[error] 501-501: Replace
·className="w-full·sm:w-auto"·onClick={()·=>·setIsPrintMode(true)}
with⏎················className="w-full·sm:w-auto"⏎················onClick={()·=>·setIsPrintMode(true)}⏎··············
(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json
(1 hunks)src/components/Shifting/ShiftDetails.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
🪛 eslint
src/components/Shifting/ShiftDetails.tsx
[error] 495-495: Replace data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"
with ⏎··················data?.status·===·"COMPLETED"·||·data?.status·===·"CANCELLED"⏎················
(prettier/prettier)
[error] 496-496: Replace ·navigate(
/shifting/${data?.external_id}/update)
with ⏎··················navigate(
/shifting/${data?.external_id}/update)⏎················
(prettier/prettier)
[error] 501-501: Replace ·className="w-full·sm:w-auto"·onClick={()·=>·setIsPrintMode(true)}
with ⏎················className="w-full·sm:w-auto"⏎················onClick={()·=>·setIsPrintMode(true)}⏎··············
(prettier/prettier)
🔇 Additional comments (1)
src/components/Shifting/ShiftDetails.tsx (1)
486-486
: LGTM! Improved button layout responsiveness
The flex container with wrap and gap utilities ensures proper button spacing and stacking on mobile devices.
@ayushpatil2122 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
Added a Tailwind css classes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores
husky
package version for routine maintenance.