-
Notifications
You must be signed in to change notification settings - Fork 477
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: UI changes in shifting and resources pages #9437
Fix: UI changes in shifting and resources pages #9437
Conversation
WalkthroughThe pull request introduces consistent UI improvements across Resource and Shifting components, focusing on layout standardization and component restructuring. The changes primarily involve replacing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (2)
src/components/ui/button.tsx (1)
33-33
: Consider using standard Tailwind height valuesThe height value
h-10.8
is non-standard in Tailwind CSS. Consider using standard height values likeh-11
(44px) or creating a custom height in your Tailwind config if this specific height is required by design.- xl: "h-10.8 px-4 py-2", + xl: "h-11 px-4 py-2",src/components/Resource/ResourceList.tsx (1)
222-245
: Consider adjusting flex container gapThe current gap between flex items could be more consistent. Consider using the same gap value across all breakpoints.
- <div className="flex w-full flex-col items-center justify-between gap-2 pt-2 xl:flex-row"> + <div className="flex w-full flex-col items-center justify-between gap-4 pt-2 xl:flex-row">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Resource/ResourceBoard.tsx
(2 hunks)src/components/Resource/ResourceList.tsx
(3 hunks)src/components/Shifting/ShiftingBoard.tsx
(2 hunks)src/components/Shifting/ShiftingList.tsx
(3 hunks)src/components/ui/button.tsx
(1 hunks)
🔇 Additional comments (10)
src/components/Shifting/ShiftingList.tsx (3)
52-72
: LGTM: Improved layout structure
The new flex layout with proper width constraints provides better organization and alignment of the title section.
73-80
: LGTM: Consistent search input styling
The SearchInput component now has consistent width and spacing, improving the overall layout.
82-88
: LGTM: Button size consistency
The Button now uses the new xl size variant, matching the height of other buttons as per PR objectives.
src/components/Resource/ResourceBoard.tsx (2)
85-85
: LGTM: Consistent search input styling
The SearchInput styling matches the one in ShiftingList.tsx, maintaining consistency across pages.
96-100
: LGTM: Consistent button sizing
The Button component uses the same xl size variant as ShiftingList.tsx, achieving the PR's objective of consistent button heights.
src/components/Shifting/ShiftingBoard.tsx (2)
112-112
: LGTM: Consistent margin for better vertical spacing
The added margin-bottom helps maintain consistent vertical spacing in the layout.
127-131
: LGTM: Button size standardization
The xl
size property ensures consistent button heights across the interface, matching the PR objective to align with Advanced Filters button.
src/components/Resource/ResourceList.tsx (3)
197-220
: LGTM: Improved page header structure
The new PageTitle implementation provides better organization and consistent layout with other pages.
222-229
: LGTM: Consistent search input styling
The SearchInput styling matches the ShiftingBoard component, maintaining UI consistency.
231-237
: LGTM: Standardized button sizing
The xl
size property ensures consistent button heights across the interface, matching the ShiftingBoard implementation.
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 (1)
src/components/Resource/ResourceList.tsx (1)
223-229
: Consider extracting common search input stylesSince both ShiftingList and ResourceList use identical search input styling, consider extracting these common styles into a shared component or utility class.
- className="w-full md:w-60" + className={commonStyles.searchInput} // or a utility class like "search-input-base"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Resource/ResourceBoard.tsx
(1 hunks)src/components/Resource/ResourceList.tsx
(3 hunks)src/components/Shifting/ShiftingBoard.tsx
(1 hunks)src/components/Shifting/ShiftingList.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Shifting/ShiftingBoard.tsx
- src/components/Resource/ResourceBoard.tsx
🔇 Additional comments (2)
src/components/Shifting/ShiftingList.tsx (1)
52-96
: LGTM! Layout structure improvements enhance UI consistency
The new flex layout structure properly aligns components and matches the ResourceList implementation, creating a consistent look across pages.
src/components/Resource/ResourceList.tsx (1)
197-245
: LGTM! Layout changes maintain consistency with ShiftingList
The implementation successfully mirrors the layout structure from ShiftingList.tsx, ensuring consistent UI across both pages:
- Flex container structure matches
- Button heights are aligned
- Search input positioning is consistent
<Button | ||
variant={"primary"} | ||
onClick={onBoardViewBtnClick} | ||
className="h-10.8 px-4 py-2" | ||
> |
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.
💡 Codebase verification
Button heights are inconsistent
The AdvancedFilterButton uses ButtonV2 component which has a default height defined by padding (py-2
) while the button in ShiftingList uses a fixed height class h-10.8
. These different approaches to setting height will likely result in inconsistent heights.
src/components/Shifting/ShiftingList.tsx
: Uses fixed heighth-10.8
src/CAREUI/interactive/FiltersSlideover.tsx
: Uses ButtonV2 with padding-based heightsrc/style/CAREUI.css
: Shows ButtonV2 usespy-2
for default size
🔗 Analysis chain
Verify button height matches Advanced Filters
The button height is set to h-10.8
which should match the Advanced Filters button height as per PR objectives.
Let me check the implementation of the AdvancedFilterButton component to verify its height.
Let me check the ButtonV2 component implementation since AdvancedFilterButton uses it.
Let me check the CSS classes that define the button heights.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent button heights across components
# Search for height classes in Advanced Filters button implementations
rg -A 2 "AdvancedFilterButton"
Length of output: 4835
Script:
#!/bin/bash
# Get the implementation of AdvancedFilterButton to check its height class
ast-grep --pattern 'export const AdvancedFilterButton = ({ onClick }: { onClick: () => void }) => {
$$$
}'
Length of output: 1201
Script:
#!/bin/bash
# Find ButtonV2 component implementation
fd "ButtonV2" --exec cat {}
Length of output: 7084
Script:
#!/bin/bash
# Search for CSS files that might contain button height definitions
fd -e css -e scss | xargs rg "button-size|h-10.8"
Length of output: 262
@bodhish I have now made changes as per your request |
LGTM |
@SathishAdithiyaaSV 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
Video of Solution:
Screencast.from.2024-12-15.13-24-52.webm
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
PageTitle
component for improved layout and organization.ExportButton
for downloading resource requests in CSV format.Improvements
Bug Fixes