Refactor CSS files in src/components/DynamicDropdown #3426
Conversation
WalkthroughThis pull request involves refactoring CSS files and updating documentation for the Talawa Admin project. The primary changes include removing the 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
|
hars-21
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
src/style/app.module.css (2)
Line range hint
334-344: Improve sidebar positioning and scrolling behavior.The sidebar's absolute positioning and fixed height could cause issues on different screen sizes.
.sidebar:after { background-color: var(--grey-border-box-color); position: absolute; width: 2px; - height: 600px; + height: 100%; top: 10px; left: 94%; display: block; + content: ''; }
Line range hint
885-891: Improve error icon accessibility with ARIA attributes.The error icon needs proper ARIA labeling for screen readers.
.errorIcon { transform: scale(1.5); color: var(--bs-danger, var(--delete-button-color)); margin-bottom: 1rem; - /* Add error icon for non-color indication */ - &::before { - content: '⚠️'; - margin-right: 0.5rem; - } + &::before { + content: '⚠️'; + margin-right: 0.5rem; + speak: none; + aria-hidden: true; + } }
🧹 Nitpick comments (5)
src/components/SignOut/SignOut.tsx (1)
24-39: Consider optimizing the logout implementation.The current implementation could be improved in several ways:
- Move
handleSignOutoutside thelogoutfunction to avoid recreating it on every call- Add a loading state during logout
- Use more specific error handling
Here's a suggested implementation:
+ const handleSignOut = () => { + localStorage.clear(); + endSession(); + navigate('/'); + }; + + const [isLoggingOut, setIsLoggingOut] = useState(false); + const logout = async (): Promise<void> => { - const handleSignOut = () => { - localStorage.clear(); - endSession(); - navigate('/'); - }; try { + setIsLoggingOut(true); await revokeRefreshToken(); handleSignOut(); } catch (error) { - console.error('Error revoking refresh token:', error); + if (error instanceof ApolloError) { + console.error('GraphQL error during logout:', error.message); + } else { + console.error('Unexpected error during logout:', error); + } // Still sign out the user locally even if token revocation fails handleSignOut(); + } finally { + setIsLoggingOut(false); } };🧰 Tools
🪛 eslint
[error] 25-25: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
🪛 GitHub Check: Performs linting, formatting, type-checking, checking for different source and target branch
[failure] 25-25:
Missing return type on function🪛 GitHub Actions: PR Workflow
[error] 25-25: Missing return type on function
src/style/app.module.css (4)
Line range hint
257-271: Enhance card hover effects for better accessibility and performance.The card hover effects can be optimized for better performance and accessibility:
.cards:hover { - filter: brightness(0.8); + opacity: 0.8; + transition: opacity 0.2s ease; } .cards:hover::before { - opacity: 0.5; + opacity: 0.4; + transition: opacity 0.2s ease; } .cards:hover::after { opacity: 1; mix-blend-mode: normal; + transition: all 0.2s ease; }
Line range hint
3337-3342: Optimize animation performance with GPU acceleration.Add GPU acceleration to improve animation performance.
@media (prefers-reduced-motion: no-preference) { .talawa_logo { - -webkit-animation: zoomIn 0.3s ease-in-out; - animation: zoomIn 0.3s ease-in-out; + -webkit-animation: zoomIn 0.3s ease-in-out; + animation: zoomIn 0.3s ease-in-out; + transform: translateZ(0); + will-change: transform; } }
4373-4384: Improve left drawer layout and scrolling behavior.The left drawer needs better layout handling and smooth scrolling.
.leftDrawer { height: 100vh; - /* Ensure it spans the full height */ background-color: var(--profile-bg); overflow-y: auto; - /* Enable vertical scrolling */ transition: transform 0.3s ease; will-change: transform; - /* NEW */ position: fixed; - /* Ensure it's positioned properly */ padding-bottom: 1rem; - /* Prevent last item clipping */ overscroll-behavior: contain; - /* Improve scroll performance */ -webkit-overflow-scrolling: touch; + scrollbar-width: thin; + scrollbar-color: rgba(0, 0, 0, 0.2) transparent; }
7839-7903: Optimize media queries for better maintainability.The media queries can be organized better using CSS custom properties for breakpoints.
+:root { + --breakpoint-sm: 650px; + --breakpoint-md: 900px; +} -@media (max-height: 900px) { +@media (max-height: var(--breakpoint-md)) { .leftDrawer { width: calc(300px + 1rem); } /* ... other styles ... */ } -@media (max-height: 650px) { +@media (max-height: var(--breakpoint-sm)) { .leftDrawer { padding: 0.5rem 0.8rem 0 0.8rem; width: calc(250px); } /* ... other styles ... */ }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/docs/auto-docs/components/SignOut/SignOut/functions/default.md(1 hunks)src/components/DynamicDropDown/DynamicDropDown.module.css(0 hunks)src/components/DynamicDropDown/DynamicDropDown.tsx(1 hunks)src/components/SignOut/SignOut.tsx(1 hunks)src/style/app.module.css(92 hunks)
💤 Files with no reviewable changes (1)
- src/components/DynamicDropDown/DynamicDropDown.module.css
✅ Files skipped from review due to trivial changes (2)
- docs/docs/auto-docs/components/SignOut/SignOut/functions/default.md
- src/components/DynamicDropDown/DynamicDropDown.tsx
🧰 Additional context used
🪛 eslint
src/components/SignOut/SignOut.tsx
[error] 25-25: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
🪛 GitHub Check: Performs linting, formatting, type-checking, checking for different source and target branch
src/components/SignOut/SignOut.tsx
[failure] 25-25:
Missing return type on function
🪛 GitHub Actions: PR Workflow
src/components/SignOut/SignOut.tsx
[error] 25-25: Missing return type on function
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyse Code With CodeQL (javascript)
| const handleSignOut = () => { | ||
| localStorage.clear(); | ||
| endSession(); | ||
| navigate('/'); | ||
| }; |
There was a problem hiding this comment.
Add return type to handleSignOut function.
The missing return type is causing pipeline failures. This should be addressed to maintain type safety throughout the codebase.
Apply this fix:
- const handleSignOut = () => {
+ const handleSignOut = (): void => {Note: Moving handleSignOut outside the logout function as suggested in the previous comment would also help resolve this issue more cleanly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleSignOut = () => { | |
| localStorage.clear(); | |
| endSession(); | |
| navigate('/'); | |
| }; | |
| const handleSignOut = (): void => { | |
| localStorage.clear(); | |
| endSession(); | |
| navigate('/'); | |
| }; |
🧰 Tools
🪛 eslint
[error] 25-25: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
🪛 GitHub Check: Performs linting, formatting, type-checking, checking for different source and target branch
[failure] 25-25:
Missing return type on function
🪛 GitHub Actions: PR Workflow
[error] 25-25: Missing return type on function
| @extend .reusable-text-ellipsis; | ||
| /* Referencing the reusable class from the general section */ |
There was a problem hiding this comment.
Improve CSS reusability with proper class extension.
The @extend directives are referencing undefined classes.
- @extend .reusable-text-ellipsis;
+ text-overflow: ellipsis;
+ white-space: nowrap;
+ overflow: hidden;
- @extend .reusable-focus-visible;
+ outline: 2px solid var(--focus-color, var(--bs-primary));
+ outline-offset: 2px;
- @extend .reusable-indicator;
+ content: '';
+ position: absolute;
+ left: 0;
+ top: 50%;
+ transform: translateY(-50%);
+ width: 4px;
+ height: 24px;
+ background-color: var(--bs-primary);
+ border-radius: 2px;
- @extend .reusable-arrow-indicator;
+ content: '';
+ position: absolute;
+ right: 1rem;
+ top: 50%;
+ transform: translateY(-50%);
+ width: 8px;
+ height: 8px;
+ border-right: 2px solid currentColor;
+ border-bottom: 2px solid currentColor;
+ transform: rotate(-45deg);Also applies to: 4474-4475, 4499-4500, 4509-4510, 4514-4515
| /* Dynamic Dropdown */ | ||
| .dropwdownToggle { | ||
| background-color: #f1f3f6; | ||
| color: black; | ||
| width: 100%; | ||
| border: none; | ||
| padding: 0.5rem; | ||
| text-align: left; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: space-between; | ||
| min-width: 8rem; | ||
| outline: 1px solid var(--bs-gray-400); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance dropdown toggle accessibility and usability.
The dropdown toggle needs ARIA attributes and keyboard navigation improvements.
.dropwdownToggle {
background-color: #f1f3f6;
color: black;
width: 100%;
border: none;
padding: 0.5rem;
text-align: left;
display: flex;
align-items: center;
justify-content: space-between;
min-width: 8rem;
outline: 1px solid var(--bs-gray-400);
+ cursor: pointer;
+ transition: background-color 0.2s ease;
+ position: relative;
+ z-index: 1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* Dynamic Dropdown */ | |
| .dropwdownToggle { | |
| background-color: #f1f3f6; | |
| color: black; | |
| width: 100%; | |
| border: none; | |
| padding: 0.5rem; | |
| text-align: left; | |
| display: flex; | |
| align-items: center; | |
| justify-content: space-between; | |
| min-width: 8rem; | |
| outline: 1px solid var(--bs-gray-400); | |
| } | |
| /* Dynamic Dropdown */ | |
| .dropwdownToggle { | |
| background-color: #f1f3f6; | |
| color: black; | |
| width: 100%; | |
| border: none; | |
| padding: 0.5rem; | |
| text-align: left; | |
| display: flex; | |
| align-items: center; | |
| justify-content: space-between; | |
| min-width: 8rem; | |
| outline: 1px solid var(--bs-gray-400); | |
| cursor: pointer; | |
| transition: background-color 0.2s ease; | |
| position: relative; | |
| z-index: 1; | |
| } |
What kind of change does this PR introduce?
Refactor css for drodown component
Issue Number:
Fixes #3332
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Transferred all existing to
app.module.cssfileRefactored CSS to make all the css in one global file
Updated source path
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Release Notes
Documentation
SignOutcomponent documentationStyling
Component Updates
SignOutcomponent's logout function