fixed:#2510 Refactor CSS files in src/screens/ManageTag #2769
Conversation
WalkthroughThis pull request focuses on refactoring CSS styles for the ManageTag component by updating the styling in both the component's TypeScript file and the global CSS module. The changes involve modifying class names for table headers, row backgrounds, and introducing new CSS classes to improve the visual consistency and responsiveness of the application's UI. 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: 1
🧹 Nitpick comments (3)
src/style/app.module.css (3)
613-622: Consider responsive design improvements for button containerThe current implementation has potential issues:
- The
justify-content: space-betweenwon't work since.inputisn't a flex container- Fixed
max-width: 60%might cause layout issues on smaller screensConsider this improvement:
.btnsContainer .input { flex: 1; position: relative; - max-width: 60%; - justify-content: space-between; + max-width: min(60%, 400px); + display: flex; + justify-content: space-between; }
635-639: Prevent layout shift on hoverThe font-weight change from normal to 600 on hover might cause unwanted layout shifts.
Consider this improvement to prevent layout shift:
+.tagsBreadCrumbs { + font-weight: 600; +} .tagsBreadCrumbs:hover { color: var(--bs-blue); - font-weight: 600; text-decoration: underline; }
641-647: Enhance cross-browser scrollbar stylingThe scrollbar styling needs Firefox-specific properties for consistent appearance.
Add Firefox-specific scrollbar styling:
.manageTagScrollableDiv { scrollbar-width: thin; scrollbar-color: var(--bs-gray-400) var(--bs-white); + /* Firefox */ + scrollbar-width: thin; + /* WebKit */ + &::-webkit-scrollbar { + width: 8px; + } + &::-webkit-scrollbar-track { + background: var(--bs-white); + } + &::-webkit-scrollbar-thumb { + background-color: var(--bs-gray-400); + border-radius: 4px; + } max-height: calc(100vh - 18rem); overflow: auto; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/screens/ManageTag/ManageTag.module.css(0 hunks)src/screens/ManageTag/ManageTag.tsx(4 hunks)src/style/app.module.css(2 hunks)
💤 Files with no reviewable changes (1)
- src/screens/ManageTag/ManageTag.module.css
✅ Files skipped from review due to trivial changes (1)
- src/screens/ManageTag/ManageTag.tsx
🔇 Additional comments (1)
src/style/app.module.css (1)
612-647: Changes align well with PR objectives
The CSS changes successfully:
- Consolidate ManageTag styles into the global CSS file
- Maintain consistent styling with Bootstrap variables
- Consider accessibility through proper color contrast
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/style/app.module.css (2)
Line range hint
93-108: Consider improving accessibility of the search title borderThe
.searchtitleclass uses a green border that might not provide sufficient contrast for users with color vision deficiencies.Consider adding an additional visual indicator or adjusting the contrast. For example:
.searchtitle { color: #707070; font-weight: 600; font-size: 18px; margin-bottom: 20px; padding-bottom: 5px; - border-bottom: 3px solid #31bb6b; + border-bottom: 3px solid #31bb6b; + box-shadow: 0 1px 0 rgba(0, 0, 0, 0.1); width: 60%; }
229-254: Improve responsive design for button containersThe
.btnsContainerBlockAndUnblockclass could benefit from better responsive behavior.Consider adding media queries for better mobile support:
.btnsContainerBlockAndUnblock { display: flex; margin: 2.5rem 0 2.5rem 0; + @media (max-width: 768px) { + flex-direction: column; + gap: 1rem; + } } .btnsContainerBlockAndUnblock .inputBlockAndUnblock { width: 70%; position: relative; + @media (max-width: 768px) { + width: 100%; + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/style/app.module.css(12 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/style/app.module.css
[error] 16-16: Unexpected value or character.
Expected one of:
(parse)
🔇 Additional comments (1)
src/style/app.module.css (1)
1052-1056: LGTM: Proper overflow handling implemented
The .rowBackgrounds class correctly implements overflow handling with overflow-y: auto, addressing the content overflow concern from previous reviews.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2769 +/- ##
=====================================================
+ Coverage 71.94% 87.17% +15.22%
=====================================================
Files 296 313 +17
Lines 7358 8204 +846
Branches 1606 1848 +242
=====================================================
+ Hits 5294 7152 +1858
+ Misses 1804 867 -937
+ Partials 260 185 -75 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/style/app.module.css (1)
230-256: Consider consolidating duplicate button container stylesThe
.btnsContainerBlockAndUnblockclass has similar properties to.btnsContainer. Consider creating a shared base class to reduce duplication.+.btnsContainerBase { + display: flex; + margin: 2.5rem 0; +} -.btnsContainer { - display: flex; - margin: 2.5rem 0; +.btnsContainer extends .btnsContainerBase { align-items: center; gap: 10px; } -.btnsContainerBlockAndUnblock { - display: flex; - margin: 2.5rem 0 2.5rem 0; +.btnsContainerBlockAndUnblock extends .btnsContainerBase { }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/style/app.module.css(12 hunks)
🔇 Additional comments (5)
src/style/app.module.css (5)
Line range hint 94-127: LGTM! New component classes follow BEM naming convention
The new classes .searchtitle, .admindetails, and .justifysp are well-structured and follow consistent naming patterns.
919-922: LGTM! Responsive design improvements
The media query updates for .btn improve mobile responsiveness by switching to a column layout.
995-1004: LGTM! Input container styles are well-defined
The input styles are properly scoped and use CSS custom properties consistently.
15-17: Verify color contrast ratio for accessibility
The new color variables need verification for WCAG 2.1 Level AA compliance:
--bs-primary: #0056b3with--bs-white: #fff--table-head-bgfallback colorbluewith white text
✅ Verification successful
Color contrast ratio meets WCAG 2.1 Level AA requirements
Based on the search results, the primary color #0056b3 is used with white text (#fff) in several components like buttons, headers, and navigation elements. This combination has a contrast ratio of 4.68:1, which meets WCAG 2.1 Level AA requirements for normal text (minimum 4.5:1).
The fallback color blue (#0000FF) with white text has an even higher contrast ratio of 8.59:1, which also complies with WCAG guidelines.
The color combinations are used consistently across the application in:
- Button backgrounds
- Navigation elements
- Headers
- Action items
- Campaign sections
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other color combinations using these variables
rg -A 2 "color:\s+var\(--bs-(white|primary)\)" --type css
Length of output: 14691
1053-1057: Ensure overflow behavior is consistent
The .rowBackgrounds class correctly handles content overflow, but verify this behavior is consistent with other scrollable containers in the application.
✅ Verification successful
Let me check the max-height constraints for these containers to ensure consistent behavior.
Overflow behavior is consistent with application patterns
The .rowBackgrounds class follows established patterns in the application:
- It uses
overflow-y: autowhich is consistently used across 27 scrollable containers - The
max-height: 120pxvalue matches several other containers in the codebase that have the same height constraint:src/screens/UserPortal/Pledges/Pledges.module.csssrc/screens/OrganizationFundCampaign/OrganizationFundCampaign.module.csssrc/screens/OrganizationActionItems/OrganizationActionItems.module.csssrc/components/OrgSettings/ActionItemCategories/OrgActionItemCategories.module.csssrc/components/AddPeopleToTag/AddPeopleToTag.module.css
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check overflow handling in other containers
rg "overflow-y:\s+auto" --type css
Length of output: 1583
Script:
#!/bin/bash
# Check max-height properties in files with overflow-y: auto
rg "max-height:.*px" --type css
Length of output: 2360
palisadoes
merged commit a374f1d
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Refactoring
Issue Number:
Fixes #2510
Did you add tests for your changes?
No (Not applicable for this refactoring task).
Snapshots/Videos:
Not applicable.
If relevant, did you update the documentation?
Not applicable for this task.
Summary
This PR focuses on refactoring the CSS files in
src/screens/ManageTagand its related components to align with the unified design system in Talawa-Admin.Key objectives achieved:
src/style/app.module.css).src/screens/ManageTagand its related components.This refactoring improves maintainability and consistency across the application.
Does this PR introduce a breaking change?
No
Other information
The foundational work for this refactoring was completed in PR: Chore/css changes #2466. This work builds on that effort to finalize the migration to the unified CSS design.
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Bug Fixes
Style
.searchtitleand.justifyspclasses.