Consistent Button Placement and Layout for Users Display Page (#10639) #10643
Conversation
WalkthroughThis pull request updates the layout and styling of the Changes
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Users/UserListAndCard.tsx (2)
80-81: Consider optimizing nested flex containers.While the layout works, there are redundant flex properties. The nested
flex-colandflex-growon both parent and child could be simplified.- <CardContent className="p-4 sm:p-6 flex flex-col flex-grow"> - <div className="flex flex-col flex-grow gap-4"> + <CardContent className="p-4 sm:p-6 flex flex-col flex-grow gap-4"> + <div className="h-full">
96-96: Remove redundant whitespace class.The
truncateutility already includeswhite-space: nowrap. Addingwhitespace-nowrapis redundant.- <span className="text-sm text-gray-500 truncate whitespace-nowrap"> + <span className="text-sm text-gray-500 truncate">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Users/UserListAndCard.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
- GitHub Check: lint
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
src/components/Users/UserListAndCard.tsx (2)
79-79: LGTM! Good use of flexbox for card layout.The addition of
flex flex-colclasses to the Card component establishes a proper foundation for consistent vertical layout.
119-121: LGTM! Good button placement and spacing.The combination of
mt-auto,pt-2, andjustify-endensures consistent button placement at the bottom of the card with proper spacing.
There was a problem hiding this comment.
Hey @abdulazeespr, the department users card is being redesigned in another PR, which is almost ready for merging. Can we apply the same design as you’re now working on the user card?
but , Only proceed if the maintainers confirm it, though 😅
cc: @Jacobjeevan @rithviknishad
Edit: Here is that PR #10568
| @@ -116,7 +116,9 @@ const UserCard = ({ user }: { user: UserBase }) => { | |||
| </div> | |||
| </div> | |||
| </div> | |||
| <div className="mt-auto pt-2">{GetDetailsButton(user.username)}</div> | |||
| <div className="mt-auto pt-2 justify-end"> | |||
| {GetDetailsButton(user.username)} | |||
There was a problem hiding this comment.
although was there previously, let's switch to using proper casings
| {GetDetailsButton(user.username)} | |
| {getDetailsButton(user.username)} |
There was a problem hiding this comment.
Currently the function uses a hook, #10659 moves it out to the parent component. 🤔 So could copy over the logic or let that PR take care of it.
cc: @rithviknishad
|
Closing due to inactivity. Feel free to reopen. |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit