-
-
Notifications
You must be signed in to change notification settings - Fork 815
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
Fixes: The Login and Organization Screens #3309
base: develop-postgres
Are you sure you want to change the base?
Fixes: The Login and Organization Screens #3309
Conversation
WalkthroughThis pull request introduces comprehensive styling updates across multiple components in the Talawa Admin application. The changes focus on enhancing visual consistency by implementing new CSS variables, refactoring color schemes, and improving layout responsiveness. Key modifications include updating background colors, hover effects, and button styles across various screens such as login, organization lists, user sidebars, and profile dropdowns. The changes aim to align the application's visual design more closely with the Figma style guide and support better accessibility. Changes
Assessment against linked 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
|
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: 5
🧹 Nitpick comments (7)
src/components/UserPortal/UserSidebar/UserSidebar.tsx (1)
87-90
: Simplify boolean comparisons in conditional expressions.The explicit comparison with
true
is redundant and can be simplified.- isActive === true + isActive - isActive === true + isActive - isActive === true + isActiveAlso applies to: 115-118, 145-148
src/components/LeftDrawer/LeftDrawer.tsx (1)
72-77
: Consider extracting shared button styles to a common class.The same styling patterns are repeated across multiple buttons. Consider extracting these to a shared class to improve maintainability.
+// Add to your CSS module +.navButton { + &.active { + background-color: #EAEBEF; + color: var(--bs-black); + font-weight: bold; + } + &:not(.active) { + color: var(--bs-secondary); + } +} -className={` ${ - isActive === true - ? 'text-black font-weight-bold' - : 'text-secondary' -}`} +className={`${styles.navButton} ${isActive ? 'active' : ''}`}Also applies to: 102-107, 130-135
src/components/LoginPortalToggle/LoginPortalToggle.module.css (1)
9-16
: Consider adding transition property for smooth color changesThe background and border color changes would benefit from a smooth transition effect.
.navLinkClass { + transition: background-color 0.2s ease, border-color 0.2s ease; color: var(--brown-color); border-radius: 0.3rem; width: 100%; box-sizing: border-box; border: 1px solid var(--grey-light); font-weight: 500; - transition: all 0.25s ease; background-color: var(--grey-light); }src/components/ProfileDropdown/ProfileDropdown.module.css (1)
10-12
: Consider adding max-width constraintThe
.profilebutton
class withwidth: 100%
might stretch too wide on larger screens..profilebutton { width: 100%; + max-width: 300px; }
src/components/OrgListCard/OrgListCard.module.css (1)
87-88
: Consider adding gap property for flex layoutThe flex layout would benefit from proper spacing between items.
.orgadmin { font-size: 0.9rem; display: flex; flex-direction: column; + gap: 0.5rem; }
src/assets/css/app.css (2)
3256-3260
: Consider improving autofill input styles for better theme support.The current implementation forces white backgrounds on autofilled inputs which may not work well with dark themes. Consider these improvements:
input:-webkit-autofill { - background-color: white !important; - -webkit-box-shadow: 0 0 0px 1000px white inset !important; - -webkit-text-fill-color: initial !important; + background-color: var(--bs-body-bg) !important; + -webkit-box-shadow: 0 0 0px 1000px var(--bs-body-bg) inset !important; + -webkit-text-fill-color: var(--bs-body-color) !important; }This change:
- Uses theme variables for better dark mode support
- Maintains consistency with the app's color scheme
- Still preserves the functionality of overriding browser defaults
Line range hint
1-3500
: Consider splitting vendor and custom styles for better maintainability.The current approach of combining Bootstrap and custom styles in a single file could lead to maintenance challenges:
Consider separating the styles into:
- vendor.css (Bootstrap)
- custom.css (Talawa-specific styles)
Benefits:
- Easier Bootstrap version updates
- Better separation of concerns
- Improved caching strategy
- Reduced risk of style conflicts
Implementation approach:
- Use package manager for Bootstrap CSS
- Create separate file for custom overrides
- Use proper import order in your build system
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/images/svg/angleRight.svg
is excluded by!**/*.svg
📒 Files selected for processing (19)
src/assets/css/app.css
(1 hunks)src/components/LeftDrawer/LeftDrawer.tsx
(2 hunks)src/components/LoginPortalToggle/LoginPortalToggle.module.css
(1 hunks)src/components/OrgListCard/OrgListCard.module.css
(3 hunks)src/components/OrgListCard/OrgListCard.tsx
(1 hunks)src/components/ProfileDropdown/ProfileDropdown.module.css
(2 hunks)src/components/ProfileDropdown/ProfileDropdown.tsx
(1 hunks)src/components/SuperAdminScreen/SuperAdminScreen.module.css
(4 hunks)src/components/SuperAdminScreen/SuperAdminScreen.tsx
(0 hunks)src/components/UserPortal/OrganizationCard/OrganizationCard.module.css
(2 hunks)src/components/UserPortal/OrganizationCard/OrganizationCard.tsx
(1 hunks)src/components/UserPortal/UserSidebar/UserSidebar.module.css
(2 hunks)src/components/UserPortal/UserSidebar/UserSidebar.tsx
(2 hunks)src/screens/ForgotPassword/ForgotPassword.tsx
(1 hunks)src/screens/LoginPage/LoginPage.tsx
(6 hunks)src/screens/OrgList/OrgList.tsx
(1 hunks)src/screens/UserPortal/Organizations/Organizations.module.css
(5 hunks)src/screens/UserPortal/Organizations/Organizations.tsx
(2 hunks)src/style/app.module.css
(19 hunks)
💤 Files with no reviewable changes (1)
- src/components/SuperAdminScreen/SuperAdminScreen.tsx
✅ Files skipped from review due to trivial changes (4)
- src/components/UserPortal/OrganizationCard/OrganizationCard.tsx
- src/screens/OrgList/OrgList.tsx
- src/components/ProfileDropdown/ProfileDropdown.tsx
- src/screens/ForgotPassword/ForgotPassword.tsx
🧰 Additional context used
🪛 GitHub Actions: PR Workflow
src/screens/UserPortal/Organizations/Organizations.tsx
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
🪛 Biome (1.9.4)
src/style/app.module.css
[error] 68-68: Unexpected value or character.
Expected one of:
(parse)
🔇 Additional comments (15)
src/screens/LoginPage/LoginPage.tsx (2)
604-604
: Consistent button styling implementation!Good job on maintaining consistency by using the
styles.email_button
class across all icon buttons.Also applies to: 635-635, 787-787
864-864
: Well-structured button styling!Good implementation of button styling using CSS modules and proper class combination using template literals.
Also applies to: 878-878
src/components/OrgListCard/OrgListCard.tsx (1)
115-120
: LGTM! Improved layout structure for admin and member counts.The separation of admin and member counts into distinct divs enhances readability while maintaining proper semantic structure.
src/components/UserPortal/UserSidebar/UserSidebar.tsx (1)
65-67
: LGTM! Improved layout structure with flex container.The new flex container with
leftbarcompheight
class ensures proper spacing and alignment of sidebar elements.src/components/LeftDrawer/LeftDrawer.tsx (1)
67-67
: LGTM! Consistent layout structure with UserSidebar.The implementation maintains consistency with UserSidebar component, properly positioning the ProfileDropdown at the bottom.
Also applies to: 158-160
src/screens/UserPortal/Organizations/Organizations.tsx (2)
387-417
: LGTM! Improved responsive layout with grid system.The new grid layout using Bootstrap's row and col-md-6 classes provides better responsiveness for organization cards.
🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
393-394
:⚠️ Potential issueRemove code coverage disable statement and add tests.
The
istanbul ignore next
comment is causing pipeline failures. Instead of ignoring this condition, add appropriate tests to cover this case.🧰 Tools
🪛 GitHub Actions: PR Workflow
[error] Contains code coverage disable statement. Please remove it and add the appropriate tests.
src/components/LoginPortalToggle/LoginPortalToggle.module.css (1)
20-22
: Verify hover state color contrast ratiosPlease ensure that the hover state colors maintain sufficient contrast ratios for accessibility.
Also applies to: 32-35
✅ Verification successful
Color contrast ratios are sufficient for hover states
The hover state color combinations provide adequate contrast for accessibility:
- Text color (#555555) on light gray background (#eaebef)
- Text color (#555555) on light blue background (#a8c7fa)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CSS variable definitions to verify color values rg -l "(:root|^[^/].*{)" --type css | xargs cat | grep -A 10 "(:root|^[^/].*{)" | grep -E "(--brown-color|--light-blue|--grey-light)"Length of output: 249
Script:
#!/bin/bash # Search for CSS root variables echo "=== CSS Variables ===" rg --type css ":root" -A 10 echo -e "\n=== Complete LoginPortalToggle.module.css ===" cat src/components/LoginPortalToggle/LoginPortalToggle.module.css echo -e "\n=== Hover States ===" rg --type css ":hover" -B 2 -A 2Length of output: 51293
Script:
#!/bin/bash # Search for brown-color and light-blue variable definitions echo "=== Color Variable Definitions ===" rg --type css "(--brown-color|--light-blue)\s*:" -B 1 -A 1 echo -e "\n=== All CSS files that might contain these variables ===" fd -e css -e scss | xargs grep -l "brown-color\|light-blue"Length of output: 941
src/screens/UserPortal/Organizations/Organizations.module.css (1)
114-115
: LGTM: Good use of max-width constraintThe max-width constraint helps maintain readability on larger screens.
src/style/app.module.css (4)
60-61
: LGTM! New background color variables and hover shadow effect.The new CSS variables for backgrounds and hover effects are well-defined and follow consistent naming conventions.
Also applies to: 85-87
Line range hint
2989-3097
: LGTM! Enhanced login screen button styles.The changes improve visual feedback through:
- Consistent hover effects using the new
--hover-shadow
variable- Clear color transitions for better user interaction
- Proper use of CSS variables for maintainability
Line range hint
2038-2170
: LGTM! Enhanced organization screen styles.The changes improve the organization screen through:
- Better layout with flex container and proper spacing
- Consistent styling for create organization dropdown
- Enhanced hover effects using the new shadow variable
Line range hint
4019-4029
: LGTM! Improved responsive design and performance.Good improvements for mobile experience:
- Proper overflow handling with
overscroll-behavior
- Touch scrolling optimization with
-webkit-overflow-scrolling
- Will-change property for better performance
src/components/UserPortal/OrganizationCard/OrganizationCard.module.css (1)
52-56
: LGTM! Enhanced organization card styles.Good improvements:
- Consistent use of color variables for joined button
- Better layout for admin section with flex display
- Proper spacing with gap property
Also applies to: 91-94
src/components/UserPortal/UserSidebar/UserSidebar.module.css (1)
3-3
: LGTM! Enhanced sidebar styles.Good improvements:
- Consistent use of profile background color
- Enhanced hover effects with shadow
- Better height management with flex layout
Also applies to: 59-63, 64-69
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 (3)
src/style/app.module.css (2)
3006-3021
: Consider using CSS variables for all color valuesWhile the hover states are consistent, some color values are hardcoded. Consider extracting these to CSS variables for better maintainability:
.langChangeBtnStyle { - background-color: transparent; + background-color: var(--transparent-bg, transparent); } .langChangeBtnStyle:hover { - background-color: var(--light-blue) !important; + background-color: var(--hover-bg, var(--light-blue)) !important; }
2040-2041
: Improve responsive layout consistencyThe width property could be more flexible for better responsiveness. Consider using a relative unit:
.btnsContainerOrgList { - width: 100%; + width: clamp(300px, 100%, 1200px); }src/components/ProfileDropdown/ProfileDropdown.module.css (1)
8-8
: Consider adding hover and focus statesThe profile container could benefit from interactive states for better user experience:
.profileContainer { background-color: var(--profile-bg) !important; + transition: transform 0.2s ease; } +.profileContainer:hover { + transform: translateY(-1px); + box-shadow: var(--hover-shadow); +} +.profileContainer:focus-visible { + outline: 3px solid var(--focus-outline-color); + outline-offset: 2px; +}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/LeftDrawer/LeftDrawer.spec.tsx
(2 hunks)src/components/ProfileDropdown/ProfileDropdown.module.css
(2 hunks)src/components/SuperAdminScreen/SuperAdminScreen.module.css
(4 hunks)src/components/UserPortal/UserSidebar/UserSidebar.spec.tsx
(1 hunks)src/components/UserPortal/UserSidebar/UserSidebar.tsx
(2 hunks)src/screens/LoginPage/LoginPage.tsx
(5 hunks)src/screens/UserPortal/Organizations/Organizations.module.css
(5 hunks)src/screens/UserPortal/Organizations/Organizations.tsx
(2 hunks)src/style/app.module.css
(20 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/SuperAdminScreen/SuperAdminScreen.module.css
- src/components/UserPortal/UserSidebar/UserSidebar.tsx
- src/screens/UserPortal/Organizations/Organizations.tsx
- src/screens/LoginPage/LoginPage.tsx
- src/screens/UserPortal/Organizations/Organizations.module.css
🧰 Additional context used
📓 Learnings (1)
src/components/LeftDrawer/LeftDrawer.spec.tsx (1)
Learnt from: Doraemon012
PR: PalisadoesFoundation/talawa-admin#1988
File: src/components/LeftDrawerOrg/LeftDrawerOrg.test.tsx:282-282
Timestamp: 2024-11-12T10:40:58.654Z
Learning: In the test 'Component should be rendered properly' within 'Testing LeftDrawerOrg component for SUPERADMIN', setting 'SuperAdmin' to false is intentional.
🔇 Additional comments (4)
src/components/LeftDrawer/LeftDrawer.spec.tsx (1)
97-97
: LGTM! Consistent test updates for button styling.The test assertions have been properly updated to reflect the new active state styling with
font-weight-bold
class for both SUPERADMIN and ADMIN roles.Also applies to: 211-211
src/components/UserPortal/UserSidebar/UserSidebar.spec.tsx (1)
465-469
: Verify the text color change from white to black.The test assertions have been updated to expect:
- Addition of
font-weight-bold
class- Change of text color from
text-white
totext-black
While the font weight change is consistent with other components, please confirm if changing the text color from white to black is intentional and aligns with the Figma design specifications mentioned in the PR description.
src/style/app.module.css (1)
60-61
: Well-structured CSS variable additions!The new CSS variables for backgrounds (
--profile-bg
,--screen-bg
) and hover effects (--hover-shadow
) improve theming consistency and maintainability.Also applies to: 87-89
src/components/ProfileDropdown/ProfileDropdown.module.css (1)
10-12
: LGTM: New profilebutton classThe new profilebutton class with full width improves layout consistency.
Please fix the failing tests |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3309 +/- ##
===================================================
- Coverage 8.43% 8.38% -0.06%
===================================================
Files 310 312 +2
Lines 8069 8120 +51
Branches 1792 1815 +23
===================================================
Hits 681 681
- Misses 7311 7362 +51
Partials 77 77
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@palisadoes test acceptable code coverage is failing but most of the files which the test is showing are not changed . |
I've updated the threshold. Please make a minor commit to restart the workflow |
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.
Many change requests, but they will make the overall app experience better.
: 'text-secondary' | ||
}`} | ||
style={{ | ||
backgroundColor: isActive === true ? '#EAEBEF' : '', |
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.
Use a variable from src/style/app.module.css
. If this is the sidebar color, name it --side-bar
, or whatever this section of code is defining. This will help with future attempts to having Dark Mode
: 'text-secondary' | ||
}`} | ||
style={{ | ||
backgroundColor: isActive === true ? '#EAEBEF' : '', |
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.
Use a variable from src/style/app.module.css
. If this is the sidebar color, name it --side-bar
, or whatever this section of code is defining. This will help with future attempts to having Dark Mode
: 'text-secondary' | ||
}`} | ||
style={{ | ||
backgroundColor: isActive === true ? '#EAEBEF' : '', |
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.
Use a variable from src/style/app.module.css
. If this is the sidebar color, name it --side-bar
, or whatever this section of code is defining. This will help with future attempts to having Dark Mode
fill="none" | ||
stroke={`${ | ||
isActive === true | ||
? 'var(--black)' |
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.
Use a variable from src/style/app.module.css
. If this is the related to an icon, name it accordingly and specific to the purpose of the icon, or whatever this section of code is defining. This will help with future attempts to having Dark Mode
isActive ? 'text-black font-weight-bold' : 'text-secondary' | ||
}`} | ||
style={{ | ||
backgroundColor: isActive ? '#EAEBEF' : '', |
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.
- Make this file use CSS exclusively from
src/style/app.module.css
. - Delete the CSS file it currently uses.
- Use a variable from
src/style/app.module.css
. If this is the sidebar color, name it--side-bar
, or whatever this section of code is defining. This will help with future attempts to having Dark Mode
<SettingsIcon | ||
stroke={`${ | ||
isActive === true | ||
? 'var(--bs-black)' |
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.
Same comment here
@@ -8,6 +8,7 @@ import SettingsIcon from 'assets/svgs/settings.svg?react'; | |||
import TalawaLogo from 'assets/svgs/talawa.svg?react'; | |||
import styles from 'style/app.module.css'; | |||
import useLocalStorage from 'utils/useLocalstorage'; | |||
import ProfileDropdown from 'components/ProfileDropdown/ProfileDropdown'; |
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.
- Make this file use CSS exclusively from
src/style/app.module.css
, with exceptions for package based CSS. - Delete the CSS file it currently uses.
- This will help with future attempts to having Dark Mode
- Use a variable from
src/style/app.module.css
.
{tCommon('admins')}: <span>{admins.length}</span> | ||
{tCommon('members')}: <span>{members.length}</span> | ||
<div> | ||
{tCommon('admins')}: <span>{admins.length}</span> |
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.
- Make this file use CSS exclusively from
src/style/app.module.css
, with exceptions for package based CSS. - Delete the CSS file it currently uses.
- This will help with future attempts to having Dark Mode
- Use a variable from
src/style/app.module.css
.
@@ -191,7 +190,7 @@ const ForgotPassword = (): JSX.Element => { | |||
</div> | |||
<Button | |||
type="submit" | |||
className="mt-4 w-100" | |||
className={`mt-4 w-100 ${styles.login_btn}`} |
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.
- Make this file use CSS exclusively from
src/style/app.module.css
, with the exception of package based CSS. - Delete the CSS file it currently uses.
- This will help with future attempts to having Dark Mode
- Use a variable from
src/style/app.module.css
. - There is a reference to
--bs-primary
@@ -3,7 +3,7 @@ | |||
} | |||
|
|||
.colorWhite { | |||
color: white; | |||
color: var(--brown-color); |
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.
- Make this file use CSS exclusively from
src/style/app.module.css
, with the exception of package based CSS. - Delete the CSS file it currently uses.
- This will help with future attempts to having Dark Mode
- Use a variable from
src/style/app.module.css
. - If this is the sidebar color, name it
--side-bar
, or whatever this section of code is defining.
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 (8)
src/style/app.module.css (4)
60-74
: Consider reusing existing color variables for consistency.Several new color variables are defined with hardcoded values that could reference existing color variables:
--sidebar-icon-stroke-inactive: #808080
could use--gray-white
--sidebar-option-text-inactive: #808080
could use--gray-white
--forgot-password-fill: #555555
could use--brown-color
103-104
: Reuse existing color variables for search-related styles.Consider using existing color variables for consistency:
--search-button-border: #555555
could use--brown-color
--search-outline: #555555
could use--brown-color
Line range hint
495-515
: Enhance visual feedback for button states.While the hover effects are well implemented, consider improving the active state feedback:
- Add a scale transform for better click feedback
- Adjust the shadow for pressed state
.searchButton:active { + transform: scale(0.98); background-color: var(--search-button-bg) !important; border-color: transparent !important; + box-shadow: 0 1px 2px rgba(60, 64, 67, 0.3); }
6451-6910
: Use breakpoint variables for consistent responsive design.The responsive design implementation is good, but consider using the existing breakpoint variables for consistency:
- @media (max-width: 820px) { + @media (max-width: var(--breakpoint-tablet)) { .hideElemByDefault { display: none; } } - @media (max-width: 580px) { + @media (max-width: var(--breakpoint-mobile)) { .orgCard { height: unset; } }src/components/UserPortal/UserSidebar/UserSidebar.tsx (3)
65-67
: Consider semantic class naming for the height utility.The class name
leftbarcompheight
could be more semantic to better describe its purpose.-className={`d-flex align-items flex-column ${styles.leftbarcompheight}`} +className={`d-flex align-items flex-column ${styles.sidebarContentWrapper}`}Also, there's an extra space in
align-items flex-column
.
87-90
: Simplify the conditional expression.The explicit comparison with
true
is unnecessary.-isActive === true +isActive
71-96
: Consider extracting repeated button styles.The button styling pattern is duplicated across multiple NavLinks. Consider extracting it into a reusable component or styled component.
Example approach:
interface StyledNavButtonProps { isActive: boolean; icon: React.ReactElement; children: React.ReactNode; testId: string; } const StyledNavButton: React.FC<StyledNavButtonProps> = ({ isActive, icon, children, testId }) => ( <Button variant={isActive ? 'success' : ''} style={{ backgroundColor: isActive ? 'var(--sidebar-option-bg)' : '', fontWeight: isActive ? 'bold' : 'normal', color: isActive ? 'var(--sidebar-option-text-active)' : 'var(--sidebar-option-text-inactive)', }} data-testid={testId} > <div className={styles.iconWrapper}> {React.cloneElement(icon, { stroke: isActive ? 'var(--sidebar-icon-stroke-active)' : 'var(--sidebar-icon-stroke-inactive)' })} </div> {children} </Button> );src/components/LeftDrawer/LeftDrawer.tsx (1)
67-67
: Maintain consistency in class naming.The class name
sidebarcompheight
differs fromleftbarcompheight
used in UserSidebar. Consider using consistent naming across components.-className={`d-flex flex-column ${styles.sidebarcompheight} `} +className={`d-flex flex-column ${styles.sidebarContentWrapper}`}Also, there's a trailing space in the className string.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/components/LeftDrawer/LeftDrawer.tsx
(2 hunks)src/components/OrgListCard/OrgListCard.module.css
(0 hunks)src/components/OrgListCard/OrgListCard.tsx
(2 hunks)src/components/ProfileDropdown/ProfileDropdown.module.css
(0 hunks)src/components/ProfileDropdown/ProfileDropdown.tsx
(2 hunks)src/components/SuperAdminScreen/SuperAdminScreen.module.css
(0 hunks)src/components/SuperAdminScreen/SuperAdminScreen.tsx
(1 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
(2 hunks)src/screens/ForgotPassword/ForgotPassword.tsx
(3 hunks)src/screens/UserPortal/Organizations/Organizations.module.css
(0 hunks)src/screens/UserPortal/Organizations/Organizations.tsx
(3 hunks)src/style/app.module.css
(21 hunks)
💤 Files with no reviewable changes (6)
- src/components/ProfileDropdown/ProfileDropdown.module.css
- src/components/SuperAdminScreen/SuperAdminScreen.module.css
- src/components/UserPortal/OrganizationCard/OrganizationCard.module.css
- src/components/UserPortal/UserSidebar/UserSidebar.module.css
- src/components/OrgListCard/OrgListCard.module.css
- src/screens/UserPortal/Organizations/Organizations.module.css
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/UserPortal/OrganizationCard/OrganizationCard.tsx
- src/components/OrgListCard/OrgListCard.tsx
- src/components/ProfileDropdown/ProfileDropdown.tsx
- src/screens/UserPortal/Organizations/Organizations.tsx
- src/screens/ForgotPassword/ForgotPassword.tsx
- src/components/SuperAdminScreen/SuperAdminScreen.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (6)
src/style/app.module.css (2)
94-97
: Well-structured shadow implementation!The
--hover-shadow
variable is well-defined with appropriate opacity values, providing consistent depth perception for hover effects across components.
304-306
: LGTM! Consistent hover effect implementation.Good use of the
--hover-shadow
variable for dropdown hover effects, maintaining consistency across interactive elements.src/components/UserPortal/UserSidebar/UserSidebar.tsx (1)
8-9
: LGTM! CSS module import aligned with guidelines.The change to use
app.module.css
aligns with the project's guidelines for centralized styling and dark mode support.src/components/LeftDrawer/LeftDrawer.tsx (3)
72-94
: Duplicate code pattern across navigation buttons.The same button styling pattern is repeated across three NavLinks. This is similar to the issue in UserSidebar.
Consider extracting the common button styling into a reusable component as suggested in the UserSidebar review.
Also applies to: 101-124, 129-152
158-160
: LGTM! Consistent ProfileDropdown placement.The ProfileDropdown component is consistently placed at the bottom of the sidebar using
mt-auto
, matching the implementation in UserSidebar.
11-11
: Verify accessibility considerations.Please ensure that:
- The navigation buttons have sufficient color contrast in both active and inactive states
- The sidebar navigation is keyboard accessible
- The ProfileDropdown component maintains accessibility when placed at different viewport heights
Also applies to: 67-160
What kind of change does this PR introduce?
UI changes in Login and Organization Screens
Issue Number:
Fixes #3169
Snapshots/Videos:
Screen.Recording.2025-01-18.093827.mp4
Screen.Recording.2025-01-18.093931.mp4
Screen.Recording.2025-01-18.094302.mp4
If relevant, did you update the documentation?
NO
Does this PR introduce a breaking change?
NO
Checklist
CodeRabbit AI Review
Test Coverage
Other information
The Changes in the ui are made according to the Figma file provided
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Summary by CodeRabbit
Based on the comprehensive changes, here are the updated release notes:
New Features
UI/UX Improvements
Bug Fixes
Styling Updates
These changes focus on enhancing the visual consistency and user experience of the application.