User portal as per figma design #3197 #3746
Conversation
WalkthroughThe changes update documentation and modify several components and style files. In the documentation, the line reference for the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant PD as ProfileDropdown Component
participant LG as Logout Handler
U->>PD: Click on profile container (div with role="button")
PD->>PD: Display profile text with arrow icon and logout button group
U->>PD: Click logout button
PD->>LG: Trigger logout action
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (5)
✨ Finishing Touches
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: 5
🔭 Outside diff range comments (2)
src/style/app.module.css (2)
681-682: 🛠️ Refactor suggestionFix inconsistent button styling.
The
.leftDrawerclass has inconsistent button styling with duplicate properties and potential specificity issues.Apply this diff to consolidate the button styles:
.leftDrawer button { - position: relative; - margin: 0.7rem 0; - padding: 2.5rem 0.1rem; - border-radius: 16px; + position: relative; + margin: 0.7rem 0; + padding: 2.5rem 0.1rem; + border-radius: 16px; + border: none; + outline: none; + transition: all 0.2s ease; }Also applies to: 683-684, 685-686, 687-688
525-532: 🛠️ Refactor suggestionConsolidate media queries for better maintainability.
The media queries are scattered throughout the file and should be consolidated.
Move all media queries to the end of the file and use a consistent structure:
/* Media Queries */ @media screen and (max-width: 1200px) { /* Desktop */ } @media screen and (max-width: 992px) { /* Tablet Landscape */ } @media screen and (max-width: 768px) { /* Tablet Portrait */ } @media screen and (max-width: 576px) { /* Mobile */ }Also applies to: 1807-1819, 2010-2029, 3581-3600
🧹 Nitpick comments (6)
src/style/app-fixed.module.css (1)
651-656: Consider removing !important and uncommented properties.The changes to
.btnsContainerhave some potential issues:
- Using
!importantmakes styles harder to maintain and override.- Commented properties (
widthandflex-direction) should be either removed or uncommented if needed.Apply this diff to improve maintainability:
- padding-left: 20px !important; + padding-left: 20px; - /* width: 60%; */src/style/app.module.css (5)
883-889: Improve hover state styling for better visual feedback.The hover state styling could be improved for better visual feedback.
Apply this diff to enhance the hover state:
.outlineBtn:is(:hover, :active) { margin-bottom: 10px; color: var(--black-color) !important; background-color: var(--joined-button-bg); border-color: var(--joined-button-bg); - box-shadow: 0px 4px 6px rgba(0, 0, 0, 0.2); + box-shadow: var(--hover-shadow); + transform: translateY(-1px); }
8098-8104: Improve layout styling for better responsiveness.The main container layout could be improved for better responsiveness.
Apply this diff to enhance the layout:
.mainContainerOrganization { display: flex; flex-wrap: wrap; - gap: 16px; /* Space between cards */ - justify-content: space-between; /* Ensures cards are spread out */ - padding-left: 20px !important; - margin-top: 10px; + gap: var(--spacing-lg, 16px); + justify-content: flex-start; + padding: var(--spacing-md, 20px); + margin: var(--spacing-sm, 10px) 0; }
1-36: Enhance CSS documentation for better maintainability.The CSS documentation could be improved to better explain the methodology and provide more examples.
Add more comprehensive documentation:
/** * CSS Methodology for Common Styles: * * This project aims to reduce CSS duplication by merging similar styles across components * into reusable global classes. This ensures consistency and simplifies maintenance. * * Steps for contributors: * 1. Identify duplicate or similar styles in different components (e.g., buttons, modals). * 2. Create a global class with a clear, descriptive name (e.g., .addButton, .removeButton). * 3. Use the new global class in all components requiring that style. * + * CSS Architecture: + * - Use CSS custom properties (variables) for theming and consistent values + * - Follow BEM naming convention for component-specific styles + * - Use utility classes for common patterns + * - Maintain a flat specificity to avoid specificity wars + * + * Responsive Design: + * - Use mobile-first approach + * - Define breakpoints using CSS variables + * - Use flexbox and grid for layouts + * * Example: * Instead of component-specific classes like: * `.greenregbtnOrganizationFundCampaign`, `.greenregbtnPledge` * Use: * `.addButton` (a single reusable class) */
38-231: Improve CSS custom properties organization.The CSS custom properties could be better organized for improved maintainability.
Reorganize the custom properties into logical groups:
:root { /* Theme Colors */ --primary: #1778f2; --secondary: #a8c7fa; --success: #31bb6b; --danger: #ff4d4f; --warning: #febc59; /* Neutral Colors */ --grey-50: #f6f8fc; --grey-100: #eaebef; --grey-500: #707070; --grey-900: #555555; /* Spacing */ --spacing-xs: 0.25rem; --spacing-sm: 0.5rem; --spacing-md: 1rem; --spacing-lg: 1.5rem; --spacing-xl: 2rem; /* Typography */ --font-size-xs: 0.75rem; --font-size-sm: 0.875rem; --font-size-md: 1rem; --font-size-lg: 1.125rem; --font-size-xl: 1.25rem; /* Border Radius */ --border-radius-sm: 4px; --border-radius-md: 8px; --border-radius-lg: 16px; --border-radius-full: 9999px; /* Shadows */ --shadow-sm: 0 1px 2px rgba(0, 0, 0, 0.05); --shadow-md: 0 4px 6px rgba(0, 0, 0, 0.1); --shadow-lg: 0 10px 15px rgba(0, 0, 0, 0.1); /* Z-index */ --z-dropdown: 1000; --z-sticky: 1020; --z-fixed: 1030; --z-modal: 1040; --z-popover: 1050; --z-tooltip: 1060;
1-4640: Improve CSS file organization and reduce file size.The CSS file is very large (4640+ lines) and should be split into smaller, more manageable files.
Consider splitting the CSS into multiple files:
base.css- Reset, typography, and base stylescomponents.css- Reusable component styleslayout.css- Layout and grid stylesutilities.css- Utility classesthemes.css- Theme variables and dark mode stylesmedia-queries.css- Responsive stylesUse CSS modules or a CSS-in-JS solution for component-specific styles to better encapsulate styles and prevent naming conflicts.
🧰 Tools
🪛 Biome (1.9.4)
[error] 2220-2220: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/docs/auto-docs/screens/UserPortal/Organizations/Organizations/functions/default.md(1 hunks)src/components/ProfileDropdown/ProfileDropdown.tsx(3 hunks)src/components/UserPortal/UserSidebar/UserSidebar.tsx(1 hunks)src/screens/UserPortal/Organizations/Organizations.tsx(4 hunks)src/style/app-fixed.module.css(2 hunks)src/style/app.module.css(10 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/docs/auto-docs/screens/UserPortal/Organizations/Organizations/functions/default.md
🧰 Additional context used
🪛 Biome (1.9.4)
src/style/app.module.css
[error] 7966-7967: Unexpected value or character.
Expected one of:
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test Application
🔇 Additional comments (8)
src/components/UserPortal/UserSidebar/UserSidebar.tsx (1)
126-126: LGTM! Improved class name semantics.The change from a static class name "mt-auto" to a dynamic class name
styles.profileContainerimproves maintainability and aligns with the CSS refactoring objectives.src/components/ProfileDropdown/ProfileDropdown.tsx (3)
52-54: LGTM! Improved container structure.The new container structure with semantic class names (
Profilebox,profileContainerarrowright,imageContainerSidebar) improves maintainability and aligns with the Figma style guide.
72-93: LGTM! Enhanced accessibility and user experience.Great improvements to accessibility and UX:
- Added proper ARIA attributes (
role="button",tabIndex={0},aria-label)- Added visual feedback with arrow icon
- Improved text structure with semantic class names
95-103: LGTM! Better logout button structure.The use of
ButtonGroupfor the logout functionality provides a more semantic and maintainable structure.src/screens/UserPortal/Organizations/Organizations.tsx (4)
124-125: Verify pagination removal against Figma design requirements.The pagination functionality has been removed and the number of organizations is now hardcoded to 10. This could impact user experience when dealing with large datasets.
Please confirm if:
- The Figma design specifically calls for removing pagination
- Limiting to 10 organizations aligns with the design requirements
- There's a plan for handling scenarios with more than 10 organizations
Also applies to: 143-143
323-327: Verify alignment with Figma design.The flex container has been replaced with a simpler div structure. While this simplifies the code, ensure it maintains the desired layout from the Figma design.
Please confirm the heading alignment and spacing match the Figma specifications.
391-422: Great improvements to organization card rendering!The changes improve the code by:
- Using proper TypeScript types for better type safety
- Using organization ID as key instead of index
- Implementing responsive grid layout with Bootstrap classes
318-319: CSS changes align well with the refactoring goals!The styling updates improve maintainability by:
- Using semantic class names for sidebar states
- Applying consistent spacing with gap and padding utilities
Also applies to: 383-383
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/style/app.module.css (1)
7694-7695:⚠️ Potential issueFix inconsistent width declarations.
The width declarations are inconsistent and could cause layout issues.
Apply this diff to fix the width declarations:
- width: calc(350px) !important; + width: 350px !important; - width: 300px + 2rem; + width: calc(300px + 2rem);Also applies to: 7966-7967
🧹 Nitpick comments (4)
src/style/app.module.css (4)
877-888: Improve accessibility and user experience for outline buttons.The
.outlineBtnclass has been updated with hover states and focus indicators, which is good for accessibility. However, we can further enhance it.Add hover and active states with visual feedback:
.outlineBtn { margin-bottom: 10px; background-color: var(--joined-button-bg); color: var(--black-color); border-color: var(--joined-button-bg); transition: all 0.2s ease; cursor: pointer; outline: none; + transform: translateY(0); + box-shadow: none; } .outlineBtn:hover { + transform: translateY(-1px); + box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); } .outlineBtn:active { + transform: translateY(0); + box-shadow: none; }
5000-5015: Improve accessibility and user experience for profile container.The
.profileContainerclass has been updated with hover states and focus indicators, which is good for accessibility. However, we can further enhance it.Add hover state and improve focus visibility:
.leftDrawer .profileContainer { background-color: transparent; color: #000; cursor: pointer; width: 100%; justify-content: flex-end; padding: 8px 10px; display: flex; transition: background-color 0.2s ease; border-radius: 8px; outline: none; + box-shadow: none; } .leftDrawer .profileContainer:hover { + background-color: var(--bs-gray-100); } .leftDrawer .profileContainer:focus-visible { outline: 2px solid var(--light-blue); outline-offset: 2px; + box-shadow: 0 0 0 4px rgba(168, 199, 250, 0.2); }
8108-8116: Improve layout responsiveness for main container.The main container layout has been updated with flexbox and gap properties, which is good for responsive design. However, we can further optimize it.
Add responsive grid layout and improve spacing:
.mainContainerOrganization { max-height: 100%; width: 100%; overflow: auto; display: flex; flex-wrap: wrap; gap: 16px; justify-content: space-between; padding-left: 20px !important; margin-top: 10px; + display: grid; + grid-template-columns: repeat(auto-fill, minmax(300px, 1fr)); + align-items: start; + padding: 20px !important; }
9139-9244: Improve profile dropdown component styling and accessibility.The profile dropdown component has been updated with proper spacing and transitions. However, we can enhance its accessibility and user experience.
Add ARIA roles and improve keyboard navigation:
.Profilebox { width: 100% !important; padding-bottom: 30px; + position: relative; } .profileContainerarrowright { border: none; padding: 2.1rem 0.5rem; height: 52px; display: flex; align-items: center; justify-content: space-between; background-color: var(--profile-bg); border-radius: var(--border-radius-lg, 8px); transition: all 0.2s ease; + role: "button"; + aria-haspopup: "true"; + aria-expanded: "false"; + tabindex: "0"; } .logoutBtn { width: 100%; padding: 12px 15px; background-color: #a8c7fa; border: none; border-radius: 13px; color: #000; font-size: 0.9375rem; display: flex; align-items: center; gap: 20px; cursor: pointer; text-align: left; transition: background-color 0.2s ease; + role: "menuitem"; + aria-label: "Log out"; } + .logoutBtn:focus-visible { + outline: 2px solid var(--light-blue); + outline-offset: 2px; + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/UserPortal/OrganizationCard/OrganizationCard.tsx(1 hunks)src/style/app.module.css(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (2)
src/components/UserPortal/OrganizationCard/OrganizationCard.tsx (2)
156-160: LGTM! Robust handling of undefined values.The use of the nullish coalescing operator (
??) to default to0whenlengthis undefined improves the code's reliability and prevents potential rendering issues.
123-163:❓ Verification inconclusive
Verify style classes against Figma design.
Since this PR aims to align with the Figma style guide, please ensure that the following style classes match the design specifications:
styles.orgCardstyles.innerContainerstyles.orgImgContainerstyles.contentstyles.orgNamestyles.orgdescstyles.addressstyles.orgadminstyles.addButtonstyles.withdrawBtnstyles.outlineBtnRun this script to check if all style classes are defined in the consolidated CSS file:
Also applies to: 165-196
🏁 Script executed:
#!/bin/bash # Description: Verify style classes used in OrganizationCard component exist in app.module.css # Find all style classes used in the component echo "Style classes used in OrganizationCard.tsx:" rg -o 'styles\.[a-zA-Z]+' src/components/UserPortal/OrganizationCard/OrganizationCard.tsx | sort -u # Check if these classes exist in app.module.css echo -e "\nMatching classes in app.module.css:" rg '^\.(orgCard|innerContainer|orgImgContainer|content|orgName|orgdesc|address|orgadmin|addButton|withdrawBtn|outlineBtn)\s*{' src/style/app.module.cssLength of output: 935
Action required: Verify consolidated CSS class definitions manually
The PR requires ensuring that the following style classes adhere to the Figma style guide and exist in the consolidated CSS file:
styles.orgCardstyles.innerContainerstyles.orgImgContainerstyles.contentstyles.orgNamestyles.orgdescstyles.addressstyles.orgadminstyles.addButtonstyles.withdrawBtnstyles.outlineBtnIt appears that the automated check encountered a regex error when verifying these classes in
src/style/app.module.css. Please re-run the verification with the corrected regex below or manually confirm that all these class definitions are present in the consolidated CSS file:#!/bin/bash # Verify style classes used in OrganizationCard.tsx exist in app.module.css echo "Style classes used in OrganizationCard.tsx:" rg -o 'styles\.[a-zA-Z]+' src/components/UserPortal/OrganizationCard/OrganizationCard.tsx | sort -u echo -e "\nMatching class definitions in app.module.css:" rg '^\.(orgCard|innerContainer|orgImgContainer|content|orgName|orgdesc|address|orgadmin|addButton|withdrawBtn|outlineBtn)\s*\{' src/style/app.module.cssIf discrepancies are found, please update the CSS accordingly to match the Figma design specifications.
🧰 Tools
🪛 Biome (1.9.4)
[error] 143-143: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
|
|
@palisadoes While updating test cases, I noticed that clicking the "Join Now" button is not working. when i click on "Join Organization," it shows an "Error Occurred" message. After checking, I found that there is no mutation in the backend for joining an organization. All styling has been done according to the Figma style guide, but the "Join" button feature is broken. Should I cover this in this PR as well? |
|
Yes |
|
@khushipatil1523 Please refine the to ensure visual consistency with the existing UI
|
Got it! I'll refine the spacing to ensure visual consistency and align it with the existing UI. I'll refer to the Figma link and use the spacing variables from there. Thanks for the feedback! |
What kind of change does this PR introduce?
UI/UX improvement, refactoring of CSS for better accessibility and maintainability.
Issue Number:
Fixes #3197
Description
This PR addresses the issue "Organization Screen Violates the Figma Style Guide" by implementing the following changes:
Changes Made
Consolidated all CSS into src/style/app.module.css:
here is screenshot of Updated the Organization Screen to match the Figma style guide:
Applied proper spacing, alignment, and colors as per the provided Figma file.
Standardized buttons and dropdowns with hover shadows but no color changes.
Ensured search boxes only show shadows when selected.
Does this PR introduce a breaking change?
No, this PR does not introduce breaking changes. It only refactors styles and updates UI components for consistency and accessibility.
Summary by CodeRabbit
Documentation
UI Enhancements
Style Updates