[FIX] Figma User Portal: Organization Left Drawer Violates The Figma Style Guide #3421
Conversation
|
Warning Rate limit exceeded@hustlernik has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 51 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis pull request introduces several enhancements to the user interface components, focusing on consolidating CSS styles, improving user experience, and ensuring consistency with the Figma design guidelines. The changes include the addition of new components like 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: 8
🔭 Outside diff range comments (1)
src/components/ProfileDropdown/ProfileDropdown.tsx (1)
Line range hint
66-71: Remove code coverage ignore for conditional rendering.The user image conditional rendering should be tested for both cases.
Remove the coverage ignore and add test cases for both scenarios:
- /*istanbul ignore next*/ <img src={userImage} alt={`profile picture`} data-testid="display-img" />🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
🧹 Nitpick comments (6)
src/components/ProfileCard/ProfileCard.tsx (1)
36-41: Extract magic numbers into named constants.The maximum name length is hardcoded and could be moved to a constants file for better maintainability.
Consider creating a constants file:
+// src/constants/ui.ts +export const UI_CONSTANTS = { + MAX_NAME_LENGTH: 20, + ELLIPSIS_OFFSET: 3, +} as const; -const MAX_NAME_LENGTH = 20; +const { MAX_NAME_LENGTH, ELLIPSIS_OFFSET } = UI_CONSTANTS; const fullName = `${firstName} ${lastName}`; const displayedName = fullName.length > MAX_NAME_LENGTH - ? fullName.substring(0, MAX_NAME_LENGTH - 3) + '...' + ? fullName.substring(0, MAX_NAME_LENGTH - ELLIPSIS_OFFSET) + '...' : fullName;src/components/UserPortal/UserSidebarOrg/UserSidebarOrg.tsx (1)
164-171: Simplify button variant logic.The current variant logic can be simplified as it's using empty strings in both cases.
-variant={isActive === true ? '' : ''}Also, consider using a CSS variable for the icon color to maintain consistency with the design system:
-isActive === true ? '#00000' : 'var(--bs-secondary)' +isActive === true ? 'var(--primary-text)' : 'var(--bs-secondary)'src/components/ProfileCard/ProfileCard.spec.tsx (1)
83-124: Consider adding negative test cases.While the current tests cover the happy path scenarios well, consider adding tests for:
- Missing user information
- Invalid image URLs
- Empty name handling
src/style/app.module.css (1)
4311-4319: Enhance keyboard navigation accessibility.The focus styles for the profileContainer could be improved for better accessibility.
Consider adding these focus styles:
.profileContainer { &:focus-within { + outline: 2px solid var(--bs-primary); + outline-offset: 2px; + box-shadow: 0 0 0 4px rgba(0, 123, 255, 0.25); @extend .reusable-focus-visible; } }🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix formatting issues.
docs/docs/auto-docs/components/SignOut/SignOut/functions/default.md (1)
11-14: Improve documentation grammar and consistency.The documentation has some grammatical inconsistencies:
Apply these changes:
-Renders a sign out button. +Renders a sign-out button. -This component helps to logout. +This component helps users to log out.🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: When ‘sign-out’ is used as a noun or modifier, it needs to be hyphenated.
Context: ...nts/SignOut/SignOut.tsx#L20) Renders a sign out button. This component helps to logout...(VERB_NOUN_CONFUSION)
[misspelling] ~13-~13: Did you mean the verb “log out” instead of the noun ‘logout’?
Context: ...gn out button. This component helps to logout. The logout function revokes the refres...(LOG_IN)
src/components/ProfileDropdown/ProfileDropdown.tsx (1)
62-62: Consider using a more semantic class name for styling.Instead of using variant="none", consider adding a custom class for styling the dropdown.
- <Dropdown as={ButtonGroup} variant="none"> + <Dropdown as={ButtonGroup} className={styles.profileDropdown}>🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/docs/auto-docs/components/ProfileCard/ProfileCard/functions/default.md(1 hunks)docs/docs/auto-docs/components/SignOut/SignOut/functions/default.md(1 hunks)docs/docs/auto-docs/components/UserPortal/UserSidebarOrg/UserSidebarOrg/functions/default.md(1 hunks)docs/docs/auto-docs/components/UserPortal/UserSidebarOrg/UserSidebarOrg/interfaces/InterfaceUserSidebarOrgProps.md(1 hunks)src/components/ProfileCard/ProfileCard.spec.tsx(1 hunks)src/components/ProfileCard/ProfileCard.tsx(1 hunks)src/components/ProfileDropdown/ProfileDropdown.tsx(2 hunks)src/components/SignOut/SignOut.spec.tsx(1 hunks)src/components/SignOut/SignOut.tsx(1 hunks)src/components/UserPortal/UserSidebarOrg/UserSidebarOrg.module.css(0 hunks)src/components/UserPortal/UserSidebarOrg/UserSidebarOrg.tsx(4 hunks)src/style/app.module.css(4 hunks)
💤 Files with no reviewable changes (1)
- src/components/UserPortal/UserSidebarOrg/UserSidebarOrg.module.css
✅ Files skipped from review due to trivial changes (3)
- docs/docs/auto-docs/components/UserPortal/UserSidebarOrg/UserSidebarOrg/functions/default.md
- docs/docs/auto-docs/components/ProfileCard/ProfileCard/functions/default.md
- docs/docs/auto-docs/components/UserPortal/UserSidebarOrg/UserSidebarOrg/interfaces/InterfaceUserSidebarOrgProps.md
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
src/components/ProfileDropdown/ProfileDropdown.tsx
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
src/style/app.module.css
[warning] Code style issues found. Run Prettier with --write to fix formatting issues.
🪛 LanguageTool
docs/docs/auto-docs/components/SignOut/SignOut/functions/default.md
[uncategorized] ~11-~11: When ‘sign-out’ is used as a noun or modifier, it needs to be hyphenated.
Context: ...nts/SignOut/SignOut.tsx#L20) Renders a sign out button. This component helps to logout...
(VERB_NOUN_CONFUSION)
[misspelling] ~13-~13: Did you mean the verb “log out” instead of the noun ‘logout’?
Context: ...gn out button. This component helps to logout. The logout function revokes the refres...
(LOG_IN)
🔇 Additional comments (6)
src/components/UserPortal/UserSidebarOrg/UserSidebarOrg.tsx (3)
156-156: LGTM! Header styling aligns with Figma guidelines.The new header implementation with
titleHeaderclass improves visual hierarchy.
189-192: LGTM! Footer addition enhances user experience.The addition of ProfileCard and SignOut components in the footer improves navigation and accessibility.
14-17: Consider implications of moving styles to global CSS module.Moving styles from a component-specific module to a global module might affect component isolation and maintainability. Consider keeping component-specific styles local unless these styles are truly shared across multiple components.
Let's verify the usage of these styles across components:
src/style/app.module.css (1)
7644-7716: LGTM! Well-structured responsive design.The media queries are well organized with appropriate breakpoints for different screen sizes. The styles properly handle layout adjustments for various device sizes.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[warning] Code style issues found. Run Prettier with --write to fix formatting issues.
src/components/SignOut/SignOut.spec.tsx (1)
1-127: Well-structured test implementation with comprehensive coverage!The test suite effectively covers both success and error scenarios for the SignOut component with:
- Proper mocking of dependencies (useSession, react-router-dom, localStorage)
- Clear test cases for successful logout and error handling
- Thorough verification of side effects (localStorage clearing, navigation, session end)
src/components/ProfileDropdown/ProfileDropdown.tsx (1)
Line range hint
1-127: Add the following test cases to achieve full coverage.To resolve the pipeline failure, implement these test scenarios in your spec file:
Error handling test:
test('logs error when console.error is called', () => { const error = new Error('Test error'); const consoleSpy = jest.spyOn(console, 'error'); // Trigger error condition expect(consoleSpy).toHaveBeenCalledWith('Error revoking refresh token:', error); });Name truncation test:
test('truncates long names correctly', () => { // Setup component with long name const longName = 'Very Long Name That Should Be Truncated'; // Render and verify truncation expect(screen.getByTestId('display-name')).toHaveTextContent('Very Long Name Tha...'); });User image rendering test:
test('renders user image when available', () => { // Setup component with userImage const userImage = 'test-image-url'; // Render and verify image expect(screen.getByTestId('display-img')).toHaveAttribute('src', userImage); });🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… into issue#3198
palisadoes
merged commit 00fe16b
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
UI fix according to figma.
Issue Number:
Fixes #3198
Snapshots/Videos:
UserSidebarOrg1.mp4
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Yes