fixed:#2527 Refactored CSS files in src/screens/UserPortal #2946
Conversation
WalkthroughThis pull request focuses on refactoring CSS files within the UserPortal screens by consolidating multiple module-specific CSS files into a single global stylesheet ( Changes
Assessment against linked issues
Possibly related 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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![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
🔭 Outside diff range comments (1)
src/screens/UserPortal/Chat/Chat.tsx (1)
Line range hint
190-200: Validate user array length.
The code indexeschat.users[1]andchat.users[0], assuming two users exist. If a chat group grows or if data changes in the future, this could lead to undefined references. Consider a fallback approach or a safe check for array length before accessing these elements.Here is a possible fix:
- image: chat.isGroup - ? chat.users[1]?.image - : chat.users[0]?.image, + image: chat.isGroup + ? chat.users[1]?.image ?? 'defaultGroupImage.png' + : chat.users[0]?.image ?? 'defaultUserImage.png',
🧹 Nitpick comments (3)
src/screens/UserPortal/Chat/Chat.tsx (3)
31-36: Ensure user data remains consistent withInterfaceUser.
This interface is a solid definition for user data, but if the user schema evolves (e.g., additional fields such as email or roles), remember to update it here as well to maintain strong typing throughout the codebase.
62-62: Minor documentation mismatch.
The docstring referencesORGANIZATIONS_MEMBER_CONNECTION_LIST, but the actual query used isCHATS_LIST. Consider revising the documentation to accurately reflect the current GraphQL query.
116-116: Refetch strategy looks fine, but consider error handling.
In theuseEffectblock, you handlechatsListDatabut do not handle potential errors from the GraphQL query. Consider adding logic to handle and log errors for better fault tolerance.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
src/screens/UserPortal/Campaigns/Campaigns.module.css(0 hunks)src/screens/UserPortal/Campaigns/Campaigns.tsx(2 hunks)src/screens/UserPortal/Campaigns/PledgeModal.tsx(1 hunks)src/screens/UserPortal/Chat/Chat.module.css(0 hunks)src/screens/UserPortal/Chat/Chat.tsx(6 hunks)src/screens/UserPortal/Donate/Donate.module.css(0 hunks)src/screens/UserPortal/Donate/Donate.tsx(2 hunks)src/screens/UserPortal/Events/Events.module.css(0 hunks)src/screens/UserPortal/Events/Events.tsx(1 hunks)src/screens/UserPortal/Organizations/Organizations.module.css(0 hunks)src/screens/UserPortal/Organizations/Organizations.tsx(2 hunks)src/screens/UserPortal/People/People.module.css(0 hunks)src/screens/UserPortal/People/People.tsx(1 hunks)src/screens/UserPortal/Pledges/Pledges.module.css(0 hunks)src/screens/UserPortal/Pledges/Pledges.tsx(1 hunks)src/screens/UserPortal/Posts/Posts.module.css(0 hunks)src/screens/UserPortal/Posts/Posts.tsx(1 hunks)src/screens/UserPortal/Settings/Settings.module.css(0 hunks)src/screens/UserPortal/Settings/Settings.tsx(1 hunks)src/screens/UserPortal/UserScreen/UserScreen.module.css(0 hunks)src/screens/UserPortal/UserScreen/UserScreen.tsx(1 hunks)src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx(1 hunks)src/screens/UserPortal/Volunteer/UpcomingEvents/UpcomingEvents.tsx(1 hunks)src/screens/UserPortal/Volunteer/VolunteerManagement.module.css(0 hunks)
💤 Files with no reviewable changes (11)
- src/screens/UserPortal/UserScreen/UserScreen.module.css
- src/screens/UserPortal/Events/Events.module.css
- src/screens/UserPortal/Settings/Settings.module.css
- src/screens/UserPortal/Volunteer/VolunteerManagement.module.css
- src/screens/UserPortal/Chat/Chat.module.css
- src/screens/UserPortal/Posts/Posts.module.css
- src/screens/UserPortal/Organizations/Organizations.module.css
- src/screens/UserPortal/Campaigns/Campaigns.module.css
- src/screens/UserPortal/Donate/Donate.module.css
- src/screens/UserPortal/Pledges/Pledges.module.css
- src/screens/UserPortal/People/People.module.css
✅ Files skipped from review due to trivial changes (11)
- src/screens/UserPortal/Events/Events.tsx
- src/screens/UserPortal/Volunteer/UpcomingEvents/UpcomingEvents.tsx
- src/screens/UserPortal/Settings/Settings.tsx
- src/screens/UserPortal/Campaigns/PledgeModal.tsx
- src/screens/UserPortal/UserScreen/UserScreen.tsx
- src/screens/UserPortal/Volunteer/Invitations/Invitations.tsx
- src/screens/UserPortal/Pledges/Pledges.tsx
- src/screens/UserPortal/Campaigns/Campaigns.tsx
- src/screens/UserPortal/Posts/Posts.tsx
- src/screens/UserPortal/People/People.tsx
- src/screens/UserPortal/Donate/Donate.tsx
🔇 Additional comments (5)
src/screens/UserPortal/Organizations/Organizations.tsx (2)
16-16: Good practice to centralize styles for better maintainability
Importing styles from a central CSS file is consistent with the new design pattern and improves code maintainability.
364-364: Double-check renamed class usage
You changed styles.content to styles.contents. Ensure that the corresponding CSS selector exists correctly in the new central stylesheet and that there are no references to the old class name in the rest of the codebase.
src/screens/UserPortal/Chat/Chat.tsx (3)
9-9: Use of global styles file is appropriate.
Transitioning to a global stylesheet helps ensure consistency across the refactored components. This aligns with the PR's objective to adopt a more maintainable design.
23-29: Good use of strongly typed interfaces for chat data.
Defining InterfaceChat clarifies the expected data structure, making the code more maintainable and less error-prone. Ensure that all relevant fields are included if future features require additional properties.
66-66: Use of typed state ensures clarity in the chat data structure.
By specifying useState<InterfaceChat[]>, future developers will know the array contains only valid chat objects. This practice significantly reduces type-related bugs.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2946 +/- ##
====================================================
- Coverage 95.82% 88.38% -7.44%
====================================================
Files 295 316 +21
Lines 7037 8268 +1231
Branches 1520 1867 +347
====================================================
+ Hits 6743 7308 +565
- Misses 98 742 +644
- Partials 196 218 +22 ☔ View full report in Codecov by Sentry. |
| } | ||
|
|
||
| .btnsContainer .btnsBlock { | ||
| display: flex; | ||
| width: max-content; |
There was a problem hiding this comment.
Why was this removed? Won't it have an impact on other layouts?
| } | ||
|
|
||
| .noOutline:is(:hover, :focus, :active, :focus-visible, .show) { | ||
| outline: none !important; | ||
| } | ||
|
|
||
| .closeButton { | ||
| margin-bottom: 10px; |
There was a problem hiding this comment.
Why was this removed? Won't it have an impact on other layouts?
src/style/app.module.css
Outdated
| margin-bottom: 50px; | ||
| border-color: #e8e5e5; | ||
| width: 80%; | ||
| border-radius: 7px; |
There was a problem hiding this comment.
Why was this removed? Won't it have an impact on other layouts?
| } | ||
|
|
||
| .rowBackgroundPledge { | ||
| background-color: var(--bs-white); |
There was a problem hiding this comment.
Why was this removed? Won't it have an impact on other layouts?
| } | ||
|
|
||
| .TableImagePledge { | ||
| object-fit: cover; | ||
| width: calc(var(--table-image-size) / 2) !important; |
There was a problem hiding this comment.
Why was this removed? Won't it have an impact on other layouts?
| } | ||
|
|
||
| .imageContainerPledge { | ||
| display: flex; | ||
| align-items: center; |
There was a problem hiding this comment.
Why was this removed? Won't it have an impact on other layouts?
| margin: 0.1rem 0.25rem; | ||
| gap: 0.25rem; | ||
| padding: 0.25rem 0.45rem; | ||
| border-radius: 0.35rem; |
There was a problem hiding this comment.
Why was this removed? Won't it have an impact on other layouts?
I will submit a new pull request within 5 hours with all the required changes. |
in the file chart.tsx i getting these error when i try to commit
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/screens/UserPortal/Chat/Chat.tsx (1)
Line range hint
1-248: Maintain single responsibility principle in PR scopeThis PR currently mixes several concerns:
- CSS refactoring (intended scope)
- Type system improvements
- Functional changes to search feature
To maintain cleaner git history and easier review process:
- Keep only CSS-related changes in this PR
- Create separate PRs for:
- Type system improvements
- Chat functionality changes (coordinate with the existing chat fixes PR)
This approach will:
- Make reviews more focused
- Reduce merge conflicts
- Improve traceability
- Make reverting changes easier if needed
src/style/app.module.css (3)
Line range hint
82-87: Remove duplicate margin declarationThe
.btnsContainerclass has duplicatemargindeclarations. The second declaration at line 87 is redundant since it's already defined at line 83..btnsContainer { display: flex; margin: 2.5rem 0; align-items: center; gap: 10px; - /* Adjust spacing between items */ - margin: 2.5rem 0; }
500-519: Standardize media query breakpoints using CSS custom propertiesThe media queries use hardcoded pixel values for breakpoints. Consider using CSS custom properties to maintain consistent breakpoints across the stylesheet.
+:root { + --breakpoint-mobile: 520px; + --breakpoint-tablet: 820px; + --breakpoint-desktop: 1020px; + --breakpoint-large: 1120px; +} -@media (max-width: 520px) { +@media (max-width: var(--breakpoint-mobile)) { /* styles */ } -@media (max-width: 1020px) { +@media (max-width: var(--breakpoint-desktop)) { /* styles */ } -@media (max-width: 1120px) { +@media (max-width: var(--breakpoint-large)) { /* styles */ }Also applies to: 562-583, 895-919
Line range hint
1-2610: Consider modularizing the CSS for better maintainabilityThe CSS file is quite large and contains styles for many different components. Consider:
- Breaking it down into smaller, component-specific modules
- Creating a design system with consistent variables for colors, spacing, and typography
- Implementing a CSS-in-JS solution or CSS Modules for better encapsulation
This would improve maintainability, reduce the risk of style conflicts, and make the codebase more scalable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/UserPortal/Chat/Chat.tsx(1 hunks)src/style/app.module.css(13 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/style/app.module.css
[error] 2604-2604: Unexpected value or character.
Expected one of:
(parse)
[error] 2608-2608: Unexpected value or character.
Expected one of:
(parse)
🔇 Additional comments (3)
src/screens/UserPortal/Chat/Chat.tsx (3)
11-11: CSS import path change looks good!
The modification to use the global stylesheet aligns with the PR's objective of consolidating CSS files.
Line range hint 98-113: Revert functional changes outside of CSS refactoring scope
The commented-out search functionality changes appear to be outside the scope of this CSS refactoring PR. As noted in the previous review, there's already an open PR specifically for chat fixes.
Please:
- Restore the search functionality
- Keep only CSS-related changes in this PR
- Coordinate with the team working on the chat fixes PR to avoid conflicts
Line range hint 17-25: Consider moving type improvements to a separate PR
While the addition of interfaces and type improvements are valuable changes, they appear to be outside the scope of this CSS refactoring PR. Consider:
- Moving these type improvements to a separate PR
- Focusing this PR solely on CSS changes as per the original objective
Let's verify if there are other type-related changes in the codebase:
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/style/app.module.css (1)
Line range hint
2997-3016: Add reduced motion support for accessibility.Consider adding reduced motion support for users who prefer reduced motion:
+@media (prefers-reduced-motion: reduce) { + @keyframes zoomIn { + 0% { + opacity: 0; + } + 100% { + opacity: 1; + } + } +}
🧹 Nitpick comments (4)
src/style/app.module.css (4)
702-725: Consider using relative units for margins.While the responsive layout changes look good, consider using relative units (rem/em) for margins to maintain consistent spacing across different screen sizes.
- margin-bottom: 1rem; + margin-bottom: clamp(0.5rem, 2vw, 1rem);
819-839: Consider adding transform-related performance optimizations.While the animations are properly defined, consider adding performance optimizations:
@keyframes load8 { 0% { -webkit-transform: rotate(0deg); transform: rotate(0deg); + will-change: transform; } 100% { -webkit-transform: rotate(360deg); transform: rotate(360deg); } }
2807-2844: Consider adding fallbacks for older browsers.While the text truncation works well with modern browsers, consider adding fallbacks:
.profileContainer .profileText .primaryText { font-size: 1rem; font-weight: 600; overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; /* Fallback for browsers that don't support line-clamp */ display: -webkit-box; -webkit-line-clamp: 2; line-clamp: 2; -webkit-box-orient: vertical; }
3009-3015: Consider using CSS custom properties for width values.For better maintainability and consistency, consider using CSS custom properties:
+:root { + --sidebar-width: 250px; + --drawer-width: 30px; +} @media (max-width: 1120px) { .collapseSidebarButton { - width: 250px; + width: var(--sidebar-width); } .opendrawer { - width: 30px; + width: var(--drawer-width); } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/style/app.module.css(23 hunks)
🔇 Additional comments (3)
src/style/app.module.css (3)
1-11: LGTM! Improved accessibility with proper outline styles.
The changes look good:
- Added proper accordion styling using CSS custom properties
- Improved accessibility by using visible outlines instead of removing them
Line range hint 223-261: LGTM! Enhanced dropdown styles with proper state handling.
Good improvements:
- Added proper color variables for better maintainability
- Enhanced visual feedback for hover and focus states
- Improved accessibility with proper focus indicators
276-299: LGTM! Improved button container layout.
Good improvements:
- Used flexbox for better alignment and spacing
- Added proper gap between items
- Consistent button styling
| .chatContainer { | ||
| flex-grow: 6; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| flex-direction: column; | ||
| margin: 20px; | ||
| border: 1px solid rgb(220, 220, 220); | ||
| border-radius: 24px; | ||
| overflow-y: scroll; | ||
| margin-left: 0px; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider maintaining scrollbar visibility for accessibility.
While hiding scrollbars might look cleaner, it can impact accessibility. Consider using modern scrollbar styling instead:
- overflow-y: scroll;
-}
-
-.chatContainer::-webkit-scrollbar {
- display: none;
+ overflow-y: auto;
+ scrollbar-width: thin;
+ scrollbar-color: rgba(0, 0, 0, 0.2) transparent;
}Committable suggestion skipped: line range outside the PR's diff.
|
Thanks for submitting your PR, but we must close it.
Please resubmit when you are ready. |
What kind of change does this PR introduce?
Issue Number:
Fixes #2527
Did you add tests for your changes?
Yes
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
The goal is to convert the CSS file in this subdirectory and all the components related to this screen to use this new design pattern.
Does this PR introduce a breaking change?
NO
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Chores