-
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: Organization Users page needs responsiveness #10413
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request updates the styling and layout in four components:
Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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 (4)
src/pages/Facility/settings/locations/LocationView.tsx (4)
98-98
: Remove unnecessary flex-wrap property.The
flex-wrap
property has no effect on aflex-col
container since columns don't wrap. Consider simplifying to:- <div className="my-4 flex flex-col flex-wrap"> + <div className="my-4 flex flex-col">
99-101
: Simplify nested flex containers.While the flex-wrap properties improve responsiveness, the nested structure with multiple flex containers is redundant. Consider simplifying:
- <div className="py-2 flex flex-wrap justify-between"> - <div className="flex flex-col flex-wrap justify-between items-start gap-4"> - <div className="flex flex-wrap items-center gap-4 py-2"> + <div className="py-2 flex flex-wrap items-start justify-between gap-4"> + <div className="flex flex-wrap items-center gap-4">
120-120
: Remove unnecessary flex properties from search input.The flex-wrap properties on both the container and input element don't contribute to the layout:
- <div className="flex flex-wrap w-72"> + <div className="w-72"> <Input placeholder={t("search_by_name")} value={searchQuery} onChange={(e) => { setSearchQuery(e.target.value); setPage(1); }} - className="w-full flex flex-wrap" + className="w-full" />Also applies to: 128-128
139-139
: Remove trailing space in className.The button styling is good for touch targets, but there's an unnecessary trailing space in the className:
- <Button variant="outline" className="py-2 my-2 "> + <Button variant="outline" className="py-2 my-2">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/Facility/settings/locations/LocationView.tsx
(3 hunks)src/pages/Organization/OrganizationUsers.tsx
(2 hunks)src/pages/Organization/components/AddUserSheet.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/pages/Organization/components/AddUserSheet.tsx
- src/pages/Organization/OrganizationUsers.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (1)
src/pages/Facility/settings/locations/LocationView.tsx (1)
98-151
: LGTM! The responsive layout improvements are well implemented.The changes successfully improve the mobile responsiveness of the location view. While there are some opportunities to optimize the flex properties as noted above, the overall direction is good.
Include #10429 as well 👍 |
Proposed Changes
Before Update:
After Update:
EDIT 1: Color of Link User button changed. Now it is same as earlier.
EDIT 2: Fixes #10429
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit