Figma user portal organization screen violates the figma style guide #3197 #3394
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 217 files out of 300 files are above the max files limit of 75. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis pull request focuses on consolidating CSS files into a single global stylesheet ( Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
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: 5
🔭 Outside diff range comments (2)
src/components/UserPortal/OrganizationCard/OrganizationCard.tsx (1)
Line range hint
87-114: Enhance error handling in joinOrganization function.The error handling can be improved to handle more specific error cases and provide better user feedback.
async function joinOrganization(): Promise<void> { try { if (props.userRegistrationRequired) { await sendMembershipRequest({ variables: { organizationId: props.id, }, }); toast.success(t('MembershipRequestSent') as string); } else { await joinPublicOrganization({ variables: { organizationId: props.id, }, }); toast.success(t('orgJoined') as string); } refetch(); } catch (error: unknown) { - if (error instanceof Error) { - if (error.message === 'User is already a member') { - toast.error(t('AlreadyJoined') as string); - } else { - toast.error(t('errorOccured') as string); - } + if (error instanceof Error) { + const errorMessages: Record<string, string> = { + 'User is already a member': t('AlreadyJoined'), + 'Network error': t('networkError'), + 'Invalid organization': t('invalidOrg'), + }; + toast.error((errorMessages[error.message] || t('errorOccured')) as string); + console.error('Join organization error:', error); } } }src/components/ProfileDropdown/ProfileDropdown.tsx (1)
Line range hint
41-58: Improve error handling in logout function.The logout function should handle errors more gracefully and provide user feedback.
const logout = async (): Promise<void> => { try { await revokeRefreshToken(); + localStorage.clear(); + endSession(); + navigate('/'); } catch (error) { - console.error('Error revoking refresh token:', error); + // Still clear session even if token revocation fails + localStorage.clear(); + endSession(); + navigate('/'); + toast.error(t('logoutError')); + console.error('Error during logout:', error); } - localStorage.clear(); - endSession(); - navigate('/'); };
🧹 Nitpick comments (10)
src/components/UserPortal/UserSidebar/UserSidebar.tsx (3)
56-58: Move inline styles to CSS module.Inline styles should be avoided for better maintainability. Consider moving the background color to the CSS module.
- style={{ - backgroundColor: isActive === true ? '#EAEBEF' : '', - }} + className={`${ + isActive === true ? styles.activeNavButton : '' + }`}Add to app.module.css:
.activeNavButton { background-color: #EAEBEF; }
62-90: Standardize icon property usage.The icons use inconsistent properties:
- OrganizationsIcon uses
fill- SettingsIcon uses
strokeThis makes maintenance more difficult and could lead to visual inconsistencies.
Consider standardizing to use either
fillorstrokeconsistently for all icons.
31-40: Consider enhancing accessibility attributes.The sidebar could benefit from additional accessibility attributes:
- Add
role="navigation"to the sidebar container- Add
aria-labelto describe the navigation purpose<div className={`${styles.leftDrawer} ${ hideDrawer === null ? styles.hideElemByDefault : hideDrawer ? styles.inactiveDrawer : styles.activeDrawer }`} + role="navigation" + aria-label="User navigation sidebar" data-testid="leftDrawerContainer"src/components/UserPortal/OrganizationCard/OrganizationCard.tsx (3)
166-170: Improve accessibility for organization name tooltip.The tooltip should include ARIA attributes for better screen reader support.
-<Tooltip title={props.name} placement="top-end"> +<Tooltip + title={props.name} + placement="top-end" + aria-label={`Organization name: ${props.name}`} +>
174-186: Optimize conditional rendering of address.The address check can be simplified using optional chaining as suggested by static analysis.
-{props.address && props.address.city && ( +{props.address?.city && (🧰 Tools
🪛 Biome (1.9.4)
[error] 174-174: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
195-206: Consider consolidating membership action buttons.The three separate conditional blocks for buttons can be simplified using a switch statement or object mapping for better maintainability.
-{props.membershipRequestStatus === 'accepted' && ( - <Button /* ... */ /> -)} -{props.membershipRequestStatus === '' && ( - <Button /* ... */ /> -)} -{props.membershipRequestStatus === 'pending' && ( - <Button /* ... */ /> -)} +{(() => { + const buttonConfig = { + accepted: { + variant: "outline-success", + onClick: () => navigate(`/user/organization/${props.id}`), + text: t('visit'), + className: styles.joinedBtn, + 'data-testid': "manageBtn" + }, + '': { + variant: "outline-success", + onClick: joinOrganization, + text: t('joinNow'), + className: styles.joinBtn, + 'data-testid': "joinBtn" + }, + pending: { + variant: "danger", + onClick: withdrawMembershipRequest, + text: t('withdraw'), + className: styles.withdrawBtn, + 'data-testid': "withdrawBtn" + } + }[props.membershipRequestStatus]; + + return buttonConfig && <Button {...buttonConfig}>{buttonConfig.text}</Button>; +})()}Also applies to: 207-216, 217-226
src/screens/UserPortal/Organizations/Organizations.tsx (2)
357-386: Optimize organization list rendering performance.The organization list mapping can be optimized to prevent unnecessary re-renders.
+const OrganizationCardMemo = React.memo(OrganizationCard); <div className={`${styles.OrgList}`}> {organizations.map( (organization: InterfaceOrganization, index) => { const cardProps: InterfaceOrganizationCardProps = { name: organization.name, image: organization.image, id: organization._id, description: organization.description, admins: organization.admins, members: organization.members, address: organization.address, membershipRequestStatus: organization.membershipRequestStatus, userRegistrationRequired: organization.userRegistrationRequired, membershipRequests: organization.membershipRequests, }; return ( <div key={organization._id} // Use unique ID instead of index className={`${styles.cardcontainer}`} style={{ width: '48%', marginBottom: '20px' }} > - <OrganizationCard {...cardProps} /> + <OrganizationCardMemo {...cardProps} /> </div> ); }, )} </div>
295-296: Remove commented code.Remove unnecessary commented code that adds noise to the codebase.
- {/* <div className='SearchandDropdown'> */}src/style/app.module.css (2)
6036-6046: Optimize box-shadow performance.The box-shadow property can impact rendering performance. Consider using transform: translateZ(0) to enable GPU acceleration.
.orgCard { background-color: var(--bs-white); margin: 0.5rem; height: calc(135px + 2rem); padding: 1rem; border-radius: 8px; - box-shadow: 0 1.5px 1.5px rgba(0, 0, 0, 0.3); + box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); transition: box-shadow 0.3s ease; outline: 1px solid var(--bs-gray-200); position: relative; box-sizing: border-box; + transform: translateZ(0); + will-change: transform; }
6169-6180: Enhance responsive design breakpoints.The media queries can be improved to use standardized breakpoints and handle more screen sizes.
-@media (max-width: 1024px) { +@media (max-width: var(--breakpoint-desktop)) { .OrgList > .cardcontainer { flex: 0 1 100%; } } -@media (max-width: 768px) { +@media (max-width: var(--breakpoint-tablet)) { .OrgList > .cardcontainer { flex: 0 1 100%; + margin: 0.5rem 0; } } + +@media (max-width: var(--breakpoint-mobile)) { + .OrgList > .cardcontainer { + margin: 0.25rem 0; + } +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/components/ProfileDropdown/ProfileDropdown.module.css(0 hunks)src/components/ProfileDropdown/ProfileDropdown.tsx(3 hunks)src/components/UserPortal/OrganizationCard/OrganizationCard.module.css(0 hunks)src/components/UserPortal/OrganizationCard/OrganizationCard.tsx(2 hunks)src/components/UserPortal/UserSidebar/UserSidebar.module.css(0 hunks)src/components/UserPortal/UserSidebar/UserSidebar.tsx(1 hunks)src/screens/OrgList/OrgList.tsx(0 hunks)src/screens/UserPortal/Organizations/Organizations.module.css(0 hunks)src/screens/UserPortal/Organizations/Organizations.tsx(3 hunks)src/style/app.module.css(3 hunks)
💤 Files with no reviewable changes (5)
- src/screens/UserPortal/Organizations/Organizations.module.css
- src/components/UserPortal/UserSidebar/UserSidebar.module.css
- src/components/ProfileDropdown/ProfileDropdown.module.css
- src/screens/OrgList/OrgList.tsx
- src/components/UserPortal/OrganizationCard/OrganizationCard.module.css
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/UserPortal/OrganizationCard/OrganizationCard.tsx
[error] 174-174: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
src/components/UserPortal/UserSidebar/UserSidebar.tsx (2)
8-11: LGTM! Import changes align with CSS consolidation.The changes to import statements correctly implement the CSS consolidation strategy mentioned in the PR objectives.
97-99: LGTM! Clean integration of ProfileDropdown.The ProfileDropdown component is well-integrated at the bottom of the sidebar with proper styling wrapper.
src/components/UserPortal/OrganizationCard/OrganizationCard.tsx
Outdated
Show resolved
Hide resolved
|
Please fix the conflicting files |
|
Please fix the conflicting files |
khushipatil1523
force-pushed
the
Figma_User_Portal_Organization_Screen_Violates_The_Figma_Style_Guide_#3197
branch
from
a1a78f5 to
97f2f50
Compare
changes done |
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 video of Updated the Organization Screen to match the Figma style guide:
output.compress-video-online.com.mp4
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
Release Notes
Style Updates
UI Refinements
Performance
These changes focus on improving the visual consistency and user experience of the application's organization and user portal sections.