Updating to NEW COLOR SCHEME 2 #3293
Conversation
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
|
WalkthroughThis pull request focuses on implementing a comprehensive CSS strategy across multiple components in the Talawa Admin application. The changes involve moving common styles to a global CSS file ( Changes
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/style/app.module.css (2)
Line range hint
541-554: Update button styles to use CSS variables consistently.The
.addButtonand.removeButtonclasses have inconsistent color value usage:.addButton { margin-bottom: 10px; - color: var(--brown-color); + color: var(--button-text-color); background-color: var(--light-blue); - border-color: var(--grey-bg-color); + border-color: var(--button-border-color); } .removeButton { margin-bottom: 10px; background-color: var(--delete-button-bg); color: var(--remove-button-color); margin-right: 10px; - --bs-btn-border-color: #dc3545; + border-color: var(--delete-button-color); }
Line range hint
1-1500: Consolidate media queries for better maintainability.Media queries are scattered throughout the file. Consider consolidating them at the end of the file and using consistent breakpoints:
/* At the end of the file */ @media (max-width: var(--breakpoint-mobile)) { /* Mobile styles */ } @media (max-width: var(--breakpoint-tablet)) { /* Tablet styles */ } @media (max-width: var(--breakpoint-desktop)) { /* Desktop styles */ }
🧹 Nitpick comments (3)
src/style/app.module.css (3)
1-38: Documentation could be enhanced with additional guidelines.The CSS methodology documentation is well-structured. Consider adding:
- Browser compatibility requirements
- Responsive design principles and breakpoint usage
- Performance considerations for CSS selectors
Line range hint
39-148: Improve CSS variable naming consistency and value references.Consider these improvements:
- Standardize color variable naming:
- Some use format
--color-name(e.g.,--grey-light)- Others use format
--name-color(e.g.,--brown-color)- Reference existing variables instead of hardcoding values:
- --bs-primary: #0056b3; + --bs-primary: var(--blue-primary); - --table-bg-color: var(--grey-bg-color); + --table-bg: var(--grey-bg-color);
Line range hint
1-1500: Move component-specific styles to their respective component files.Several classes appear to be component-specific (e.g.,
.cardOrgPostCard,.modalOrgPostCard). These should be moved to their respective component files to maintain better separation of concerns and reduce the size of the global CSS file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
src/assets/css/app.css(1 hunks)src/components/AddOn/core/AddOnEntry/AddOnEntry.tsx(1 hunks)src/components/AddPeopleToTag/AddPeopleToTag.tsx(1 hunks)src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.tsx(1 hunks)src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.tsx(1 hunks)src/components/EventCalendar/EventCalendar.tsx(1 hunks)src/components/EventCalendar/EventHeader.tsx(1 hunks)src/components/EventListCard/EventListCardModals.tsx(1 hunks)src/components/OrgPeopleListCard/OrgPeopleListCard.tsx(1 hunks)src/components/OrgPostCard/DeletePostModal.tsx(1 hunks)src/components/OrgPostCard/OrgPostCard.tsx(1 hunks)src/components/TagActions/TagActions.tsx(1 hunks)src/components/TagActions/TagNode.tsx(1 hunks)src/components/Venues/VenueCard.tsx(1 hunks)src/components/Venues/VenueModal.tsx(1 hunks)src/screens/BlockUser/BlockUser.tsx(1 hunks)src/screens/FundCampaignPledge/FundCampaignPledge.tsx(1 hunks)src/screens/FundCampaignPledge/PledgeModal.tsx(1 hunks)src/screens/ManageTag/EditUserTagModal.tsx(1 hunks)src/screens/ManageTag/ManageTag.tsx(1 hunks)src/screens/ManageTag/RemoveUserTagModal.tsx(1 hunks)src/screens/ManageTag/UnassignUserTagModal.tsx(1 hunks)src/screens/OrgList/OrgList.tsx(1 hunks)src/screens/OrgPost/OrgPost.tsx(1 hunks)src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx(1 hunks)src/screens/OrganizationEvents/OrganizationEvents.tsx(1 hunks)src/screens/OrganizationFundCampaign/CampaignModal.tsx(1 hunks)src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(1 hunks)src/screens/OrganizationFunds/FundModal.tsx(1 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(1 hunks)src/screens/OrganizationPeople/AddMember.tsx(1 hunks)src/screens/OrganizationTags/OrganizationTags.tsx(1 hunks)src/screens/SubTags/SubTags.tsx(1 hunks)src/style/app.module.css(4 hunks)
✅ Files skipped from review due to trivial changes (33)
- src/components/OrgPeopleListCard/OrgPeopleListCard.tsx
- src/screens/BlockUser/BlockUser.tsx
- src/screens/ManageTag/RemoveUserTagModal.tsx
- src/screens/OrganizationFunds/FundModal.tsx
- src/screens/SubTags/SubTags.tsx
- src/components/AddPeopleToTag/AddPeopleToTag.tsx
- src/components/OrgPostCard/DeletePostModal.tsx
- src/components/EventCalendar/EventCalendar.tsx
- src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.tsx
- src/screens/ManageTag/UnassignUserTagModal.tsx
- src/screens/OrganizationPeople/AddMember.tsx
- src/components/Venues/VenueModal.tsx
- src/components/Venues/VenueCard.tsx
- src/components/TagActions/TagNode.tsx
- src/components/EventListCard/EventListCardModals.tsx
- src/screens/FundCampaignPledge/FundCampaignPledge.tsx
- src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx
- src/components/TagActions/TagActions.tsx
- src/screens/OrgList/OrgList.tsx
- src/components/AddOn/core/AddOnEntry/AddOnEntry.tsx
- src/screens/FundCampaignPledge/PledgeModal.tsx
- src/screens/OrganizationEvents/OrganizationEvents.tsx
- src/screens/ManageTag/EditUserTagModal.tsx
- src/screens/OrgPost/OrgPost.tsx
- src/components/OrgPostCard/OrgPostCard.tsx
- src/components/EventCalendar/EventHeader.tsx
- src/screens/OrganizationFundCampaign/CampaignModal.tsx
- src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.tsx
- src/screens/ManageTag/ManageTag.tsx
- src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx
- src/screens/OrganizationFunds/OrganizationFunds.tsx
- src/screens/OrganizationTags/OrganizationTags.tsx
- src/assets/css/app.css
|
Make a minor commit. I've updated the countline GitHub action |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/style/app.module.css (2)
1-38: Well-structured CSS methodology documentation!The documentation clearly explains the approach to reducing CSS duplication and provides helpful examples. Consider adding information about CSS modules scoping and how it interacts with these global classes.
Line range hint
1441-1445: Consider using modern CSS features for responsive design.The breakpoint variables are well-defined, but consider enhancing the responsive design implementation with:
- CSS Container Queries for component-level responsiveness
- CSS Grid
auto-fit/auto-fillfor more flexible layoutsclamp()for fluid typography and spacing
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/style/app.module.css(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (2)
src/style/app.module.css (2)
148-148: Verify the relationship between color variables.The new
--remove-button-color(#c8102e) seems to serve a similar purpose to the existing--delete-button-color. Consider:
- Are these intentionally different colors for different purposes?
- Could they be consolidated to reduce color variable duplication?
✅ Verification successful
Color variables serve distinct purposes - separation is justified
The variables are used differently in the UI:
--delete-button-coloris used extensively for delete actions (background, text, borders)--remove-button-coloris used specifically for text coloring in remove contexts🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of both color variables to understand their distinct purposes echo "Searching for delete-button-color usage:" rg --type css "var\(--delete-button-color\)" echo -e "\nSearching for remove-button-color usage:" rg --type css "var\(--remove-button-color\)"Length of output: 1003
541-541: Verify the visual impact of button color changes.The color changes to
.addButtonand.removeButtonclasses might affect the application's visual consistency:
.addButtonnow uses--brown-colorinstead of its previous color.removeButtonuses the new--remove-button-colorvariableAlso applies to: 554-556
✅ Verification successful
Button color changes affect multiple components
The color updates will impact:
- 32 instances of
addButtonacross organization management, events, posts, and other features- 18 instances of
removeButton, consistently used with Bootstrap's danger variant🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components using these button classes to assess impact echo "Components using addButton:" rg "className=.*addButton" echo -e "\nComponents using removeButton:" rg "className=.*removeButton"Length of output: 5429
palisadoes
merged commit 82214d9
into
PalisadoesFoundation:develop-postgres
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3293 +/- ##
=====================================================
+ Coverage 8.43% 89.86% +81.42%
=====================================================
Files 310 331 +21
Lines 8069 8574 +505
Branches 1792 1898 +106
=====================================================
+ Hits 681 7705 +7024
+ Misses 7311 610 -6701
- Partials 77 259 +182
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
* changed green to blue * updated funds section * people section css updated * updated Tags section * venues, action items, advertisments, plugins sections updated * fixed code coverage test * fixed code coverage test * CSS methodology added * fixed linting --------- Co-authored-by: Rahul Chougule <rahul.chougule@flairlabs.com>
What kind of change does this PR introduce?
This PR introduces a methodology that will help to make use of CSS classes present in app.module.css to be used in all components.
Issue Number:
#2880
Summary
There are many elements among the components that have similar CSS properties. Before Merging all the CSS files to app.module.css each component had its seperate CSS file. But has now all components use a common CSS file elements with silimar CSS properties can use common classes. This PR introduce a methodology that will help to achieve this goal.
Does this PR introduce a breaking change?
No
Summary by CodeRabbit
Release Notes
CSS Improvements
Documentation
Styling Updates