fixed : #2516 Refactor CSS files in src/screens/OrganizationFunds #2743
Conversation
WalkthroughThis pull request focuses on refactoring the CSS files for the Organization Funds screen by removing the local CSS module and consolidating styles into the global Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
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
|
|
Please fix the conflicting file and ensure coderabbit.ai approves your PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/style/app.module.css (1)
646-667: Improve modal styling consistency and responsivenessSeveral improvements can be made to the modal styles:
- Inconsistent naming: rename
.titlemodalto.modalTitle- Use CSS variables for colors
- Consider using relative units or media queries for better responsiveness
Apply this diff:
.fundModal { max-width: 80vw; - margin-top: 2vh; - margin-left: 13vw; + margin: 2vh auto 0; } -.titlemodal { - color: #707070; +.modalTitle { + color: var(--text-secondary); font-weight: 600; font-size: 32px; width: 65%; margin-bottom: 0px; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/screens/OrganizationFunds/OrganizationFunds.module.css(0 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(10 hunks)src/style/app.module.css(1 hunks)
💤 Files with no reviewable changes (1)
- src/screens/OrganizationFunds/OrganizationFunds.module.css
✅ Files skipped from review due to trivial changes (1)
- src/screens/OrganizationFunds/OrganizationFunds.tsx
🔇 Additional comments (2)
src/style/app.module.css (2)
632-644: LGTM! Clean and semantic styling
The styling for fund-specific elements is well-implemented, using appropriate CSS variables and semantic properties.
707-716: LGTM! Accessible table styling
The table styles follow good practices:
- Uses CSS variables for colors
- Maintains good contrast for accessibility
- Clear and focused styling
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/style/app.module.css (2)
1-26: Improve organization of CSS custom propertiesConsider grouping related variables and adding comments to document their purpose:
:root { + /* Colors */ --brown-color: #555555; --dropdown-hover-color: #eff1f7; --grey-bg-color: #eaebef; --subtle-blue-grey: #7c9beb; --subtle-blue-grey-hover: #5f7e91; + /* Dimensions */ --modal-width: 670px; --modal-max-width: 680px; + /* Component-specific */ --input-shadow-color: #dddddd; --delete-button-bg: #f8d6dc; --delete-button-color: #ff4d4f; /* ... rest of the variables ... */ }
829-891: Consolidate and organize media queriesConsider:
- Grouping all media queries at the end of the file
- Using CSS variables for breakpoints
- Combining media queries with the same breakpoint
+ :root { + --breakpoint-sm: 480px; + --breakpoint-md: 768px; + --breakpoint-lg: 1024px; + } /* Move all media queries to the end of the file */ @media (max-width: var(--breakpoint-lg)) { .pageNotFound h1.head { font-size: 200px; } /* Combine other styles for this breakpoint */ .contract { padding-left: calc(250px + 2rem + 1.5rem); } }Also applies to: 893-947
…tha/talawa-admin into issue-2516-fixed
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/style/app.module.css (5)
461-467: Remove commented-out codeThe checkbox button styles are commented out but still present in the codebase. Since these styles are not being used, they should be removed to maintain code cleanliness.
-/* .checkboxButton{ - background-color: transparent; - } - - .checkboxButton:checked{ - background-color: var(--subtle-blue-grey); - color:white - } */
971-974: Rename class to follow camelCase conventionThe class name
list_boxuses snake_case naming convention while the rest of the codebase follows camelCase. This inconsistency should be fixed.-.list_box { +.listBox { height: auto; overflow-y: auto; width: 100%; }
977-978: Use CSS variables for consistent themingThe color
#31bb6bis hardcoded. Consider using CSS variables for better maintainability and consistency..inputFields { - box-shadow: 0 1px 1px #31bb6b; + box-shadow: 0 1px 1px var(--brand-primary); }
1105-1120: Fix duplicate margin-top declarationThe
.manageBtnclass has duplicate margin-top declarations which can lead to confusion..manageBtn { - margin: 1rem 0 0; - margin-top: 15px; + margin-top: 1rem; border: 1px solid #e8e5e5; box-shadow: 0 2px 2px #e8e5e5; padding: 10px 10px; border-radius: 5px; font-size: 16px; color: white; outline: none; font-weight: 600; cursor: pointer; width: 45%; transition: transform 0.2s, box-shadow 0.2s; }
1167-1167: Use CSS variables consistentlyThe class redefines
.inputFieldswith a hardcoded color value. This should use the CSS variable for consistency..inputFields { - box-shadow: 0 1px 1px var(--brand-primary); + box-shadow: 0 1px 1px var(--bs-primary); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/style/app.module.css(5 hunks)
🔇 Additional comments (3)
src/style/app.module.css (3)
986-999: LGTM: Modal styles are well-structured
The modal styles are well-organized and use semantic class names. The use of CSS variables for colors and spacing is consistent with best practices.
1001-1005: LGTM: Button container styles follow flexbox best practices
The button container styles use flexbox appropriately for layout and spacing.
1016-1016: LGTM: Table header styles use semantic variables
The table header styles make good use of CSS variables for colors and maintain consistency with the design system.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2743 +/- ##
=====================================================
+ Coverage 72.46% 87.11% +14.65%
=====================================================
Files 296 313 +17
Lines 7357 8204 +847
Branches 1606 1848 +242
=====================================================
+ Hits 5331 7147 +1816
+ Misses 1766 875 -891
+ Partials 260 182 -78 ☔ View full report in Codecov by Sentry. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Please ensure coderabbit.ai approves your PR |
gurramkarthiknetha
force-pushed
the
issue-2516-fixed
branch
from
5753f58 to
82970a7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/style/app.module.css (4)
962-965: Use consistent naming conventionThe class name uses snake_case while the rest of the codebase uses camelCase.
-.list_box { +.listBox { height: auto; overflow-y: auto; width: 100%; }
968-969: Use CSS variables for consistent themingThe color
#31bb6bis hardcoded. Use the--brand-primaryCSS variable instead..inputFields { - box-shadow: 0 1px 1px #31bb6b; + box-shadow: 0 1px 1px var(--brand-primary); }
1093-1094: Fix duplicate properties and improve consistencyIssues found:
.greenregbtnhas a duplicatewidth: 100%property.manageBtnhas duplicate margin-top properties.greenregbtn { /* ... other properties ... */ width: 100%; - width: 100%; } .manageBtn { margin: 1rem 0 0; - margin-top: 15px; border: 1px solid #e8e5e5; /* ... other properties ... */ }Also applies to: 1096-1111
Line range hint
1158-1167: Use CSS variables for color consistencyThe
.dropdownsclass uses hardcoded color values. Use CSS variables for better maintainability..inputFields { box-shadow: 0 1px 1px var(--brand-primary); } .dropdowns { background-color: white; - border: 1px solid #31bb6b; + border: 1px solid var(--brand-primary); position: relative; display: inline-block; - color: #31bb6b; + color: var(--brand-primary); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/style/app.module.css(4 hunks)
🔇 Additional comments (1)
src/style/app.module.css (1)
972-974: LGTM!
These class definitions look good:
.fundName: Proper styling for clickable text.modalHeader: Clean modal styling.label: Good use of Bootstrap variable.fundModal: Proper modal positioning.btnsContainer .btnsBlock button: Good button alignment.tableHeaders: Proper use of Bootstrap variables
Also applies to: 977-979, 982-984, 986-990, 992-996, 1007-1011
now it's approved |
now is this ok? |
palisadoes
merged commit 3b68136
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Refactoring
Issue Number:
Fixes #2516
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
Summary by CodeRabbit
New Features
Bug Fixes
Chores