Updating to NEW COLOR SCHEME #2984
Conversation
WalkthroughThe pull request introduces extensive modifications to various CSS files, primarily focusing on updating color schemes and class names to enhance visual consistency across the application. Key changes include the adjustment of CSS variables in Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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
|
|
@AVtheking please suggest if any additional changes. |
![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
1011-1017: Review table cell styling for consistencyThe
.customcellclass usesvar(--bg-blue)for background color but sets white text color. This combination might have poor contrast ratio..customcell { - background-color: var(--bg-blue) !important; - color: var(--bs-white) !important; + background-color: var(--bg-blue) !important; + color: #2B2B2B !important; /* Darker text for better contrast */ font-size: medium !important; font-weight: 500 !important; padding-top: 10px !important; padding-bottom: 10px !important; }
Line range hint
1686-1693: Search button styling needs adjustmentThe search button background using
var(--bg-blue)might be too light for a call-to-action element..searchButtonOrgList { - background-color: var(--bg-blue); + background-color: #2B95B5; /* Darker blue for better visibility */ position: absolute; z-index: 10; bottom: 0; inset-inline-end: 0px; height: 100%; display: flex; }
🧹 Nitpick comments (2)
src/style/app.module.css (2)
1037-1037: Ensure consistent table row stylingThe alternating row background color using
var(--bg-blue)might be too bright. Consider using an alpha channel for a softer appearance..table-body > .table-row:nth-child(2n) { - background: var(--bg-blue) !important; + background: rgba(175, 255, 232, 0.3) !important; /* Softer background */ }
1473-1473: Consolidate duplicate hover color definitionsThe same hover color is defined in two places. Consider consolidating these into a single CSS variable.
+ :root { + --hover-blue: var(--bg-blue); + } .toggleBtn:hover { - color: var(--bg-blue) !important; + color: var(--hover-blue) !important; } .toggleBtnPledge:hover { - color: var(--bg-blue) !important; + color: var(--hover-blue) !important; }Also applies to: 860-860
📜 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 (1)
src/style/app.module.css (1)
33-33: New CSS variable definition looks good
The addition of --bg-blue: #afffe8 as a CSS variable follows best practices for maintaining consistent colors across the application.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2984 +/- ##
=====================================================
+ Coverage 8.52% 89.93% +81.41%
=====================================================
Files 309 330 +21
Lines 7805 8517 +712
Branches 1730 1884 +154
=====================================================
+ Hits 665 7660 +6995
+ Misses 7066 603 -6463
- Partials 74 254 +180 ☔ View full report in Codecov by Sentry. |
|
@palisadoes CSS of components like src/components/Advertisments needed to be added to app.module.css so that its green elements are changed |
you can continue in this issue.Make sure you use the existing color schemes which are being used in the application |
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
src/assets/css/app.css (2)
Line range hint
31-45: Update success-related color variables to use the new blue scheme.These Bootstrap color variables still use the green color (#31bb6b) that should be replaced with blue (#afffe8):
- --bs-success: #31bb6b; + --bs-success: #afffe8;This change should cascade to other related variables like:
- --bs-success-rgb
- --bs-success-text-emphasis
- --bs-success-bg-subtle
- --bs-success-border-subtle
Bootstrap color scheme needs comprehensive updates across multiple components
Based on the search results, there are numerous instances of the green color (#31bb6b) that need to be replaced with blue/grey tones across different components and files:
- Bootstrap CSS variables in src/assets/css/app.css:
- Success state colors
- Form validation colors
- Button variants (primary, success)
- Dropdown active states
- Navigation pills
- Pagination active states
- Component-specific styles:
- MemberDetail styles
- UserPortal components
- Organization related components
- Event calendar components
- Advertisement components
- SCSS variables in src/assets/scss/_variables.scss:
- $primary
- $success
- SVG assets in public/images/svg/:
- Multiple SVG files using the green color
Recommendation:
- Update the SCSS variables first since they are the source
- Replace Bootstrap CSS variables systematically
- Update component-specific styles to use the new theme colors
- Modify SVG files to use the new color scheme
🔗 Analysis chain
Line range hint
1-5700: Comprehensive color scheme update needed.The PR objective is to replace green tones with blue/grey tones, but there are still many components using the green color (#31bb6b). A thorough search and replace is needed for:
- Button styles (primary, success)
- Alert components
- Form validation states
- Progress bars
- List groups
- Other Bootstrap components
Consider using CSS variables to make future color scheme changes easier to manage.
Run this script to find all remaining instances of the green color:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all remaining instances of the green color code rg -i '31bb6b|rgb\(49, 187, 107\)'Length of output: 19161
🧹 Nitpick comments (5)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (1)
342-410: Streamline filter and search UI.The large block of markup effectively implements search and sorting controls. However, consider these improvements:
- Use a single, clearly labeled “Search” button to reduce click confusion.
- Leverage existing color variables instead of mixing plain “success” variants if those conflict with your new scheme.
- Verify that the
searchTermstate is properly trimmed or sanitized if user input might contain leading/trailing spaces.src/screens/FundCampaignPledge/FundCampaignPledge.tsx (1)
479-492: Search field improvements.
- Consider debouncing the
setSearchTermhandler to prevent excessive re-renders.- Confirm that
searchBtntriggers only once (some teams require a separate “enter key” or “click” behavior).- Align the styling with the updated color scheme for better consistency.
src/style/app.module.css (3)
448-449: Review the “active” state color.Aligning
searchButton:activebackground to#286fe0might conflict with your newly introduced--bg-blueor other brand blues—double-check design guidelines.
1247-1247: Striped table rows.Applying
var(--bg-blue) !importantfor even rows ensures new color usage, but confirm you’re not overwriting a darker row color that improved readability.
Line range hint
1268-1290: Check the.greenregbtnOrganizationFundCampaignnaming.Now that you’ve swapped in a blue color (
background-color: var(--search-button-bg);), consider renaming or removing “green” references to maintain clarity and avoid confusion.- .greenregbtnOrganizationFundCampaign { - ... - } + .blueRegBtnOrganizationFundCampaign { + ... + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/assets/css/app.css(1 hunks)src/components/OrgListCard/OrgListCard.module.css(1 hunks)src/screens/FundCampaignPledge/FundCampaignPledge.tsx(6 hunks)src/screens/FundCampaignPledge/PledgeModal.tsx(1 hunks)src/screens/OrgList/OrgList.tsx(3 hunks)src/screens/OrganizationFundCampaign/CampaignModal.tsx(1 hunks)src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(3 hunks)src/screens/OrganizationFunds/FundModal.tsx(3 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(9 hunks)src/style/app.module.css(22 hunks)
✅ Files skipped from review due to trivial changes (5)
- src/screens/FundCampaignPledge/PledgeModal.tsx
- src/screens/OrganizationFundCampaign/CampaignModal.tsx
- src/screens/OrganizationFunds/FundModal.tsx
- src/screens/OrgList/OrgList.tsx
- src/screens/OrganizationFunds/OrganizationFunds.tsx
🔇 Additional comments (18)
src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (2)
286-286: Use consistent styling for edit button variant.Here, the Bootstrap
variant="success"suggests a green theme, but the newly added classstyles.editButtonmay override it with a different color or hover effect. Ensure both approaches align or remove one for visual consistency.
315-315: Apply consistent button variant for “view pledges” action.Similarly,
variant="outline-success"combined withstyles.editButtonmight create conflicting or confusing styles. Confirm that the final appearance reflects your updated color scheme and user experience.src/screens/FundCampaignPledge/FundCampaignPledge.tsx (4)
359-359: Align edit button styles with your new color system.You’ve replaced a generic Bootstrap class with
styles.editButton. Make sure it’s consistent throughout the pledge flow, especially if a different color or hover effect is expected post-PR.
446-449: Confirm correct styling of radio buttons.This block updates the class for the “raised” indicator input. Ensure the final design matches your intended color transitions and there’s enough contrast for readability in both states.
468-469: Ensure consistent theming for the progress bar.By applying
className={${styles.progressBar}}, you rely on your.bg-infooverride. Confirm that the color references align with your new blue scheme rather than leftover green or conflicting shading.
[approve]
541-541: Confirm add pledge button’s color and disabled logic.You're disabling it if
endDate < new Date(). That’s great for preventing new pledges to expired campaigns. Just verify that your new color scheme (withclassName={styles.dropdown}) communicates the disabled state clearly (e.g., greyed out).src/components/OrgListCard/OrgListCard.module.css (1)
63-65: Maintain consistent hover feedback.The
.manageBtn:hoverrule sets a border style, which can be subtle on lighter backgrounds. Ensure it’s sufficiently visible against the new or existing color scheme.src/style/app.module.css (11)
100-100: Validate the new--bg-bluecolor.Using
#a8c7fais a significant departure from the old green. Ensure contrast across all components where it’s applied, especially text on or near this background.
188-188: Check contrast for bottom border.Replacing
--light-greenwith--bg-bluefor.logintitlemight reduce visual emphasis. Confirm that it remains clearly visible on lighter backgrounds.
198-198: Consistent bottom border color in.searchtitle.Same caution as
.logintitle. Ensure it’s not too faint against typical background colors.
458-459: Consolidate hover, active, focus states for.addButton.Ensure this block doesn’t conflict with other button states. If your design standard is to remove outlines or rely on color changes, keep it consistent here.
513-528: New.editButtonblock.Nicely done defining background, hover, and focus styles. Confirm that text color remains legible on hover (
var(--subtle-blue-grey)) if it’s a lighter tone.
682-693: Confirm toggle styling.Commented code suggests you previously tested box shadows or highlight states with
var(--bg-blue). If removing them was intentional, ensure no regressions in your toggle UI experience.
719-719: Color contrast for “Page Not Found” heading.
This closely mirrors a past comment regarding the heading color being too light, so re-check the final contrast ratio to maintain accessibility.
1221-1221: Beware of color saturation in.customcell.Using
--bg-bluefor cell backgrounds can appear harsh if the text within is also lighter. Double-check that your foreground color stays legible.
1689-1689: Hover styling for.toggleBtn.Switching to
color: var(--bg-blue)might reduce readability if the default background is also light. Evaluate final contrast on hover.
1902-1902: Adjust.searchButtonOrgList“blue” background.This uses
var(--bg-blue). Validate the final contrast for text and icons, especially if your designs typically rely on a darker or bolder accent color for buttons.
[approve]
3440-3440: Avoid overly light text color.
This update from green tovar(--bg-blue)was flagged before as potentially having low contrast in text contexts. If you must keep it, ensure accessible color contrast.
|
@palisadoes @AVtheking updated color scheme for funds section. working on other sections ... |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/assets/css/app.css (1)
Multiple components still use green color tones that need to be updated
The verification confirms that green color (#31bb6b) is extensively used throughout the codebase in various Bootstrap components and utilities:
- Base color variables:
--bs-success,--bs-success-rgb- Form validation:
--bs-form-valid-color,--bs-form-valid-border-color- Components:
- Buttons:
.btn-success,.btn-outline-success- Tables:
.table-success- Alerts:
.alert-success- List groups:
.list-group-item-success- Dropdowns, nav pills, and pagination active states
- Utility classes:
.text-success,.bg-success.border-success,.link-successThese green color usages should be updated to blue/grey tones to maintain consistency with the PR objectives.
🔗 Analysis chain
Line range hint
11-42: Replace remaining green color variables with blue/grey tones.Several color variables are still using the green color (#31bb6b) which should be replaced with blue/grey tones according to the PR objectives:
--bs-success--bs-success-rgb--bs-form-valid-color--bs-form-valid-border-color--bs-focus-ring-colorRun the following script to verify all color variables:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all color variables that still use green tones rg -i '31bb6b|success' src/assets/css/app.cssLength of output: 4042
♻️ Duplicate comments (1)
src/style/app.module.css (1)
188-188: 🛠️ Refactor suggestionBorder visibility needs improvement
The light blue border (
var(--bg-blue)) used for.logintitleand.searchtitlemight be too subtle against light backgrounds, reducing the visual hierarchy. Consider using a darker blue shade for better visibility.Also applies to: 198-198
🧹 Nitpick comments (2)
src/style/app.module.css (1)
732-739: Improve switch state visibilityThe switch styles have two issues:
- The box shadow using
var(--bg-blue)might be too subtle- The checked state uses a hardcoded blue (#286fe0) instead of the CSS variable
.switch input:checked { - background-color: #286fe0; - border: var(--bs-primary); - box-shadow: 0 0 0.1rem 0.2rem var(--bg-blue); + background-color: var(--bs-primary); + border: var(--bs-primary); + box-shadow: 0 0 0.1rem 0.2rem var(--bs-primary); }src/assets/css/app.css (1)
Line range hint
1-6000: Consider a systematic approach to color scheme updates.The current changes are focused on individual components, but a more systematic approach would ensure better consistency:
- Define new color variables for the blue/grey scheme in the root section
- Replace all success/green color references with the new variables
- Update component-specific colors to use the new variables
- Document the new color scheme for future reference
This would make the color scheme more maintainable and consistent across the application.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/assets/css/app.css(2 hunks)src/components/OrgPeopleListCard/OrgPeopleListCard.tsx(1 hunks)src/screens/FundCampaignPledge/FundCampaignPledge.tsx(6 hunks)src/screens/OrganizationPeople/AddMember.tsx(1 hunks)src/style/app.module.css(23 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/OrgPeopleListCard/OrgPeopleListCard.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/screens/FundCampaignPledge/FundCampaignPledge.tsx
🔇 Additional comments (7)
src/screens/OrganizationPeople/AddMember.tsx (5)
547-547: No functional code changes here.
This is simply the closing tag for the Modal body, so there's nothing to revise.
548-548: Good introduction of a dedicated footer block.
Using<Modal.Footer>clarifies the structure of the modal and separates concerns properly.
551-551: Consistent naming convention for removal actions.
Renaming these classes from borderNone to removeButton highlights intent more clearly.
560-560: Proper shift to a unifying add button style.
Great job removing inline styles in favor of CSS classes to enhance maintainability.
569-569: No functional code changes here.
Just the closing tag for the Modal footer.src/style/app.module.css (1)
3487-3487:⚠️ Potential issueCritical: Text color contrast issue
Using the light blue color (
var(--bg-blue)) for text will fail WCAG accessibility guidelines for color contrast. This needs to be changed to a darker color..entryaction { - color: var(--bg-blue); + color: var(--bs-primary); /* Use the primary blue for better contrast */ }Likely invalid or redundant comment.
src/assets/css/app.css (1)
3903-3903: Ensure consistent blue color shades across components.While the color changes align with the PR objective:
- Dropdown active background: #a8c7fa (light blue)
- Progress bar background: #0056b3 (dark blue)
Consider using consistent shades of blue for better visual harmony.
Also applies to: 5647-5647
|
Please fix the conflicting file |
|
PTAL at coderabbit's comments, fix the failing tests, remove commented code . |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
src/screens/ManageTag/RemoveUserTagModal.tsx (1)
59-59: Ensure color consistency for success actions.
Usingstyles.addButtonhelps unify the styling for positive actions. If intended for success-themed coloring, consider blending or harmonizing with Bootstrap’s green tone or newly introduced color variables.src/screens/ManageTag/UnassignUserTagModal.tsx (1)
59-59: Positive action styling.
Usingstyles.addButtonclarifies button behavior. Keep an eye on ensuring differences between remove vs. add actions are visually distinct.src/screens/ManageTag/EditUserTagModal.tsx (1)
81-86: Submit/Edit button styling.
Applyingstyles.addButtonfor editing differs semantically from “add,” but it may be acceptable if your style guide merges both “positive action” states. Consider clarifying naming for better semantic alignment (e.g.,.confirmButton).src/utils/organizationTagsUtils.ts (1)
41-44: Focused cell outline for accessibility.
Adding a 2px outline helps keyboard users see focused cells clearly. Consider also providing a high-contrast outline color for improved accessibility.src/style/app.module.css (7)
283-291: Dropdown pseudo-classes grouping.
Using the:is()pseudo-class for multiple states is a neat approach. Consider verifying older browser support.
299-300: Focus outline clarity.
Ensuring a2pxoutline color that differs from the button color can improve focus visibility.
308-312: Commented-out dropdown styles.
Keeping them commented may cause confusion; remove the unused code or explain why it’s retained.
441-442: Commented-outbox-shadow.
If no longer in use, consider removing it or describing the rationale for keeping it.
494-497: Redundant style block for.addButton?
.addButton:not(:hover, :active, :focus)sets the same color as line 490. Check if it conflicts or is needed.
522-525: Active states on.regularBtn.
Ensuring the hover color remains distinct from.addButtonor.removeButtonis a good practice.
1359-1360: Commented-out border.
Consider removing it if no longer relevant. Minimizing commented code helps keep CSS clean.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/components/AddPeopleToTag/AddPeopleToTag.module.css(0 hunks)src/components/AddPeopleToTag/AddPeopleToTag.tsx(6 hunks)src/components/TagActions/TagActions.module.css(0 hunks)src/components/TagActions/TagActions.tsx(4 hunks)src/components/TagActions/TagNode.tsx(1 hunks)src/screens/ManageTag/EditUserTagModal.tsx(4 hunks)src/screens/ManageTag/ManageTag.tsx(6 hunks)src/screens/ManageTag/RemoveUserTagModal.tsx(4 hunks)src/screens/ManageTag/UnassignUserTagModal.tsx(4 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(9 hunks)src/screens/OrganizationPeople/AddMember.tsx(1 hunks)src/screens/OrganizationTags/OrganizationTags.tsx(5 hunks)src/screens/SubTags/SubTags.tsx(4 hunks)src/style/app.module.css(25 hunks)src/utils/organizationTagsUtils.ts(2 hunks)
💤 Files with no reviewable changes (2)
- src/components/AddPeopleToTag/AddPeopleToTag.module.css
- src/components/TagActions/TagActions.module.css
✅ Files skipped from review due to trivial changes (3)
- src/components/TagActions/TagNode.tsx
- src/screens/ManageTag/ManageTag.tsx
- src/components/TagActions/TagActions.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/screens/OrganizationPeople/AddMember.tsx
- src/screens/OrganizationFunds/OrganizationFunds.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/AddPeopleToTag/AddPeopleToTag.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2355
File: src/components/AddPeopleToTag/AddPeopleToTag.tsx:50-83
Timestamp: 2024-11-12T10:40:58.655Z
Learning: The project currently does not have a GraphQL type generator, so manually typing the result of the `useQuery` hook is acceptable.
🪛 Biome (1.9.4)
src/style/app.module.css
[error] 1691-1691: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
🔇 Additional comments (49)
src/screens/ManageTag/RemoveUserTagModal.tsx (3)
4-4: Modular CSS import looks good.
Importing thestylesmodule is appropriate for ensuring consistent styling across the component.
37-37: Consistent modal header styling.
Usingstyles.modalHeaderaligns with the broader shift to a custom color scheme, removing direct references to Bootstrap’sbg-primary.
48-48: Button styling approach is consistent.
Appendingstyles.removeButtontobtn btn-dangerprovides a cohesive look. Confirm that theremoveButtonclass does not conflict withbtn-dangerstyles.src/screens/ManageTag/UnassignUserTagModal.tsx (3)
4-4: CSS module import is properly referenced.
Importingstylesallows for modular class usage without polluting global CSS.
38-38: Good practice for modal header customization.
Replacing static classes withstyles.modalHeaderensures consistent theming across modals.
49-49: Removal button styling is consistent with design updates.
Make sure.removeButtontransitions or hover effects are tested for user experience.src/screens/ManageTag/EditUserTagModal.tsx (4)
5-5: CSS modules import is correct.
Consolidating styling intoapp.module.csshelps maintain consistency.
41-41: Modal header style.
Adoptingstyles.modalHeaderavoids reliance on stock Bootstrap background classes.
60-60: Input field styling.
Usingstyles.inputFieldensures a consistent user experience across different modals.
77-77: Cancel/Remove button synergy.
styles.removeButtonfor the cancel action is consistent with the negative action approach.src/utils/organizationTagsUtils.ts (1)
21-25: Consistent hover boxShadow.
Applying the same box shadow for row hover and.Mui-hoveredstate maintains uniform visual feedback in the DataGrid.src/components/AddPeopleToTag/AddPeopleToTag.tsx (7)
10-10: Good modular approach for shared styles.
By importing from../../style/app.module.css, you're ensuring consistent styling across the application.
247-250: Consistent usage of conditional classes.
Replacing the Bootstrap variant with conditional classes is a sound approach for modular CSS. Good job!
269-269: Modal header style consistency check.
You extended the background withbg-primaryplusstyles.modalHeader. Confirm thatbg-primarydoesn't conflict with your custom styling inmodalHeader.
307-307: Improved input field styling.
Usingstyles.inputFieldoffers better control and consistency. Good move!
321-321: Consistent usage of.inputFieldclass.
Aligns with your modular approach. Nicely done!
402-404: Improved button design choice.
Using.removeButtonfor a destructive or cancel action is intuitive and consistent with the new design.
412-412: Button style alignment.
.addButtonclass helps unify CTA styling. Ensure contrast is sufficient on all devices.src/screens/SubTags/SubTags.tsx (4)
271-271: Clear call to action for "Manage Tag".
Applyingstyles.editButtonfor consistency with your new design system. Good usage.
448-448: Modal header update.
className={styles.modalHeader}ensures consistent header styling across modals. Nice centralization.
460-460: Input field with consistent style.
Applyingstyles.inputFieldfor the tag name field indicates a uniform approach to form inputs.
477-477: Cancellation button styling.
Using.removeButtonvisually distinguishes the cancel action. Good consistency with the new classes.src/screens/OrganizationTags/OrganizationTags.tsx (5)
288-288: Button style for manageTagBtn.
Assigning.editButtoninstead of a success or primary variant streamlines your design language.
410-410: Focus outline for row selection.
outline: '2px solid #000'ensures an accessible focus indicator. Confirm color meets accessibility guidelines.
423-423: Focus outline for cells.
Again,outline: '2px solid #000'is good for keyboard navigation accessibility.
465-465: Input field styling.
Includingmb-3 ${styles.inputField}looks consistent with the rest of the UI.
482-482: Non-destructive action visually distinguished.
Using.removeButtonfor the cancel intent in your modal is consistent with newly introduced styles.src/style/app.module.css (22)
101-101: Addition of--bg-blue.
Defining a custom variable for blue promotes maintainability. Verify it’s used consistently across all components requiring blue tones.
195-195: Border color update.
Replacing--light-greenwithvar(--bg-blue)ensures consistent color scheme alignment. Good step!
205-205: Search title border color changed.
Again, the switch tovar(--bg-blue)unifies your new color palette.
401-404: Destructive button hover states.
.deleteButton:is(:hover, :active)is consistent. Good job preserving text color contrast by forcing white text.
444-445: Accessible focus state.
A subtle box-shadow for focused input fields improves the user’s clarity while tabbing.
479-480: Search button active styling.
Shrinking the button (transform: scale(0.95)) is a nice microinteraction. Ensure final border color is noticeable.
489-490: Hover & active states on.addButton.
Darkening background color on interaction helps confirm button activity.
499-504:.removeButtonintroduction.
Visually clarifying destructive or negative actions is key to usability. Good job.
506-510: Hover states for.removeButton.
Darkening the background color conveys the destructive action more clearly.
517-520:.regularBtnfor neutral actions.
This approach helps differentiate from.addButtonor.removeButton.
566-570: Scrollable container style.
A fixed height plusoverflow-y: autocan enhance usability. Check large content scenarios.
572-579:.memberBadgeimprovements.
Box shadow and other attributes create a neat little badge. Verify it looks good in dark mode (if applicable).
581-586: Loading div styling.
Centering a spinner or loader can be beneficial. This is consistent with typical UI patterns.
588-595: Tag badge style.
Similar approach to.memberBadgefor consistency. Good practice.
603-608: New.editButtonclass.
Makes sense for an edit or primary action. Good introduction.
609-613: Hover style for.editButton.
Shifting background tovar(--subtle-blue-grey)is a subtle yet effective highlight.
614-617: Focus states for.editButton.
Reverting tovar(--bs-primary)on focus is consistent. Good for keyboard navigation.
1312-1312:.customcellcolor change.
Switching tovar(--bg-blue)is in line with the new color scheme. Verify contrast on lighter backgrounds.
1338-1338: Even-row background color.
Applyingvar(--bg-blue)to the nth-child(2n) row can create a pleasing striped table. Confirm readability of text.
1364-1364: Fund Campaign background color.
Usingvar(--search-button-bg)extends the newly established palette. Good alignment.
1377-1380: Hover effect for.greenregbtnOrganizationFundCampaign.
Darkening to#286fe0gives a clear interactive signal. Ensure consistent color transitions across similar classes.
3533-3533: Color usage for.entryaction.
Usingvar(--bg-blue)for text might reduce readability if the background is light. Check color contrast.
|
Please fix the failing tests |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
src/assets/css/app.css (1)
Line range hint
1-5800: Update remaining green color values for consistency.There are still several Bootstrap components using green color values that should be updated to maintain consistency with the new blue/grey color scheme:
- Success colors (--bs-success)
- Primary colors (--bs-primary)
- Button states
- Form validation states
This creates visual inconsistency across the UI. Consider updating these color values to align with the new color scheme.
Apply this diff to update the remaining green colors:
:root, [data-bs-theme='light'] { - --bs-success: #31bb6b; - --bs-primary: #31bb6b; + --bs-success: #0056b3; + --bs-primary: #0056b3; ... - --bs-primary-rgb: 49, 187, 107; - --bs-success-rgb: 49, 187, 107; + --bs-primary-rgb: 0, 86, 179; + --bs-success-rgb: 0, 86, 179; ... - --bs-primary-text-emphasis: #144b2b; - --bs-success-text-emphasis: #144b2b; + --bs-primary-text-emphasis: #002b59; + --bs-success-text-emphasis: #002b59; ... - --bs-primary-bg-subtle: #d6f1e1; - --bs-success-bg-subtle: #d6f1e1; + --bs-primary-bg-subtle: #cce5ff; + --bs-success-bg-subtle: #cce5ff; ... - --bs-primary-border-subtle: #ade4c4; - --bs-success-border-subtle: #ade4c4; + --bs-primary-border-subtle: #99caff; + --bs-success-border-subtle: #99caff; }src/components/OrgPostCard/OrgPostCard.tsx (1)
Line range hint
41-41: Update PushPin icon color to match new color scheme.The PushPin icons still use
color="success"which typically represents green tones. This should be updated to match the new blue color scheme.-<PushPin color="success" fontSize="large" className="fs-5" /> +<PushPin color="primary" fontSize="large" className="fs-5" />Also applies to: 141-141, 250-250
🧹 Nitpick comments (3)
src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.tsx (1)
102-102: LGTM! Consistent styling updates across componentsThe styling changes maintain consistency across the card, buttons, and modal components.
Consider simplifying the view button's className by consolidating
styles.entryactionandstyles.addButtoninto a single class if they share similar styles:-className={`${styles.entryaction} ${styles.addButton}`} +className={styles.addButton}Also applies to: 166-166, 192-194, 200-200
src/style/app.module.css (2)
58-58: Use CSS variables for better maintainability.The hardcoded color values reduce maintainability. Consider using CSS variables:
- --tablerow-bg-color: #ffffff; + --tablerow-bg-color: var(--white-color); - --table-head-bg: #eaebef; + --table-head-bg: var(--grey-bg-color); - --table-header-color: #000000; + --table-header-color: var(--black-color);Also applies to: 94-94, 96-96
482-483: Use CSS variables for hover states.Hardcoded color values in hover states make maintenance difficult:
- background-color: var(--search-button-hover-bg, #286fe0) !important; + background-color: var(--dark-blue) !important; border-color: transparent !important;.addButton:is(:hover, :active) { - background-color: var(--dark-blue) !important; + background-color: var(--hover-blue, var(--dark-blue)) !important; border-color: var(--search-button-border); }Also applies to: 492-494
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/assets/css/app.css(2 hunks)src/components/AddOn/core/AddOnEntry/AddOnEntry.tsx(1 hunks)src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.module.css(0 hunks)src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.tsx(4 hunks)src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.tsx(8 hunks)src/components/EventCalendar/EventCalendar.tsx(1 hunks)src/components/EventCalendar/EventHeader.tsx(1 hunks)src/components/EventListCard/EventListCardModals.tsx(19 hunks)src/components/OrgPeopleListCard/OrgPeopleListCard.tsx(1 hunks)src/components/OrgPostCard/DeletePostModal.tsx(2 hunks)src/components/OrgPostCard/OrgPostCard.tsx(5 hunks)src/components/Venues/VenueCard.tsx(1 hunks)src/components/Venues/VenueModal.tsx(4 hunks)src/screens/BlockUser/BlockUser.tsx(4 hunks)src/screens/FundCampaignPledge/FundCampaignPledge.tsx(6 hunks)src/screens/ManageTag/ManageTag.tsx(6 hunks)src/screens/OrgList/OrgList.tsx(2 hunks)src/screens/OrgPost/OrgPost.tsx(5 hunks)src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx(1 hunks)src/screens/OrganizationEvents/OrganizationEvents.tsx(8 hunks)src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(6 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(8 hunks)src/screens/OrganizationPeople/AddMember.tsx(1 hunks)src/screens/OrganizationTags/OrganizationTags.tsx(5 hunks)src/screens/SubTags/SubTags.tsx(4 hunks)src/style/app.module.css(27 hunks)
💤 Files with no reviewable changes (1)
- src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.module.css
✅ Files skipped from review due to trivial changes (6)
- src/components/Venues/VenueCard.tsx
- src/components/EventCalendar/EventHeader.tsx
- src/components/Venues/VenueModal.tsx
- src/components/EventCalendar/EventCalendar.tsx
- src/screens/OrgPost/OrgPost.tsx
- src/components/EventListCard/EventListCardModals.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/OrgPeopleListCard/OrgPeopleListCard.tsx
- src/screens/ManageTag/ManageTag.tsx
- src/screens/OrgList/OrgList.tsx
- src/screens/OrganizationPeople/AddMember.tsx
- src/screens/FundCampaignPledge/FundCampaignPledge.tsx
- src/screens/OrganizationFunds/OrganizationFunds.tsx
- src/screens/OrganizationTags/OrganizationTags.tsx
- src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx
- src/screens/SubTags/SubTags.tsx
🧰 Additional context used
📓 Learnings (1)
src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx (1)
Learnt from: Chaitanya1672
PR: PalisadoesFoundation/talawa-admin#2049
File: src/screens/OrganizationActionItems/ActionItemUpdateModal.tsx:112-138
Timestamp: 2024-11-12T10:40:58.654Z
Learning: The `istanbul ignore next` comments in the `ActionItemUpdateModal.tsx` file were added as part of a commit that introduced tests for the `ActionItemUpdateModal` component. Removing these comments and writing tests to cover the previously ignored lines is recommended to ensure code integrity and improve test coverage.
🪛 Biome (1.9.4)
src/style/app.module.css
[error] 1712-1712: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
🔇 Additional comments (15)
src/screens/OrganizationEvents/OrganizationEvents.tsx (3)
305-305: LGTM! Consistent styling applied to input fields.The addition of
styles.inputFieldclass to Form.Control components maintains consistency with the new color scheme.Also applies to: 321-321
429-429: LGTM! Consistent styling pattern for form switches.Good use of template literals to combine the Bootstrap margin class
me-4with the newstyles.switchclass, maintaining both spacing and the updated color scheme.Also applies to: 440-440, 453-453, 466-466, 481-481
505-505: LGTM! Button styling standardized.The change from component-specific
createButtonOrganizationEventsto the genericaddButtonclass promotes better reusability and consistency across the application.src/components/OrgPostCard/DeletePostModal.tsx (1)
50-50: LGTM! Button styling changes align with new color schemeThe modal's button styling has been updated consistently, using
removeButtonfor the negative action andaddButtonfor the positive action, maintaining clear visual hierarchy.Also applies to: 56-56
src/components/AddOn/core/AddOnEntry/AddOnEntry.tsx (1)
116-120: LGTM! Conditional styling provides clear visual feedbackThe button styling intelligently switches between
addButtonandremoveButtonbased on installation status, providing clear visual feedback to users.src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.tsx (1)
252-252: LGTM! Comprehensive styling updates maintain consistencyThe changes thoroughly update all form inputs with
inputFieldclass and apply consistent button styling throughout the component.Also applies to: 289-289, 316-316, 372-372, 391-391, 407-407, 416-416, 433-433
src/style/app.module.css (1)
1711-1714:⚠️ Potential issueFix conflicting padding properties in modal header.
The padding properties conflict with each other:
.modalHeader { border: none; - padding-bottom: 0; background-color: var(--white-color) !important; - padding: 1rem 1rem; + padding: 1rem; color: #000000; }Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1712-1712: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
src/assets/css/app.css (2)
3904-3904: Color change aligns with PR objective.The dropdown active background color has been updated from green to light blue (#a8c7fa), which aligns with the PR objective of replacing green tones with blue/grey tones.
5648-5648: Color change aligns with PR objective.The progress bar background color has been updated from green to blue (#0056b3), which aligns with the PR objective of replacing green tones with blue/grey tones.
src/screens/BlockUser/BlockUser.tsx (2)
188-194: LGTM! CSS class changes align with the new color schemeThe changes appropriately utilize the centralized styles from app.module.css, maintaining consistency with the new color scheme across the application.
Also applies to: 209-209
Line range hint
313-334: Verify button variant colors match the new schemeThe buttons still use Bootstrap's
variant="success"which typically has green styling. Consider updating these variants to align with the new blue/grey color scheme.src/components/OrgPostCard/OrgPostCard.tsx (2)
Line range hint
448-465: LGTM! Modal and input field styling changes align with the new color scheme.The changes to use
styles.modalHeaderandstyles.inputFieldclasses maintain consistency with the updated styling system.
583-595: LGTM! Button styling updates maintain visual consistency.The changes to use
styles.removeButtonandstyles.addButtonclasses align with the new color scheme.src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx (2)
116-126: LGTM! Button styling updates maintain consistency.The changes to use
styles.addButtonandstyles.removeButtonclasses align with the new color scheme and maintain visual consistency across the application.
133-136: LGTM! Consistent button styling.The use of
styles.addButtonclass maintains consistency with other action buttons across the application.
People Section:
Tags Section:
Events Section:
Venue Sections:
Action Items:
Post Section:
Advertisment Section:
Funds Section:
Plugin Section:
|
|
Working on SuperAdmin and User screens... |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
src/style/app.module.css (2)
Line range hint
45-47: Remove unused green color variablesThese color variables still use green tones which conflicts with the PR objective to replace green with blue/grey tones:
--light-dark-green: #08780b; --light-green: #31bb6b; --green-color: #21d015;Consider removing or replacing these with appropriate blue/grey tones to maintain consistency with the new color scheme.
Line range hint
1318-1323: Update customcell background color for consistencyThe customcell class uses
var(--light-blue)but sets white text color which may have contrast issues. Consider using a darker blue background or adjusting the text color for better readability..customcell { - background-color: var(--light-blue) !important; + background-color: var(--dark-blue) !important; color: var(--bs-white) !important; font-size: medium !important; font-weight: 500 !important;src/assets/css/app.css (1)
Line range hint
40-40: Update remaining green color codes for consistency.Several Bootstrap success state colors still use the green tone (#31bb6b) that needs to be replaced with blue:
--bs-successvariable--bs-success-rgbvalues- Success button variants and states
This will ensure complete consistency with the new color scheme.
Apply this diff to update the remaining green colors:
- --bs-success: #31bb6b; + --bs-success: #0056b3; - --bs-success-rgb: 49, 187, 107; + --bs-success-rgb: 0, 86, 179;Also applies to: 45-45, 50-55
src/components/EventCalendar/EventHeader.tsx (1)
Remove Bootstrap's success variant but keep the dropdown class
The
dropdownclass in app.module.css doesn't use any green colors - it uses brown colors for borders and text with a white background. However, the Bootstrapvariant="success"should still be removed as it applies default green styling.Suggested change:
- variant="success"🔗 Analysis chain
Line range hint
92-97: Update button variant to align with new color scheme.The button currently uses Bootstrap's
variant="success"which typically renders in green. This conflicts with the PR objective to replace green tones with blue/grey colors.Additionally, the class change from
createButtonEventHeadertodropdownmight affect styling consistency. Please ensure thedropdownclass in app.module.css uses the new blue color scheme (#afffe8).Let's verify the color scheme in app.module.css:
Suggested changes:
- <Button - variant="success" - className={styles.dropdown} - onClick={showInviteModal} - data-testid="createEventModalBtn" - > + <Button + className={styles.dropdown} + onClick={showInviteModal} + data-testid="createEventModalBtn" + >🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if dropdown class uses the new blue color rg -l "#31bb6b|success" "src/style/app.module.css" # Verify if the new blue color is used rg -l "#afffe8" "src/style/app.module.css"Length of output: 143
Script:
#!/bin/bash # Show context around color usages and dropdown class echo "=== Color Usage Context ===" rg -C 3 "#31bb6b|success|#afffe8" "src/style/app.module.css" echo -e "\n=== Dropdown Class Context ===" rg -C 3 "\.dropdown" "src/style/app.module.css"Length of output: 3211
🧹 Nitpick comments (14)
src/components/AddPeopleToTag/AddPeopleToTag.tsx (4)
247-250: Clean up commented code and simplify class assignment.Remove the commented variant line and consider simplifying the conditional class assignment.
- // variant={!isToBeAssigned ? 'primary' : 'danger'} - className={ - !isToBeAssigned ? styles.editButton : `btn btn-danger btn-sm` - } + className={!isToBeAssigned ? styles.editButton : styles.removeButton}
269-269: Consider consolidating modal header styles.The current implementation mixes Bootstrap's
bg-primarywith custom styles. Consider moving all styling to the CSS module to maintain consistency and prevent potential style conflicts.- className={`bg-primary ${styles.modalHeader}`} + className={styles.modalHeader}
307-307: Consolidate input field styles.Both input fields mix Bootstrap's
bg-lightwith custom styles. Consider moving all styling to the CSS module for better maintainability.- className={`bg-light ${styles.inputField}`} + className={styles.inputField}Also applies to: 321-321
401-403: Maintain consistency in button styling approach.The modal footer buttons use different styling approaches - the cancel button mixes Bootstrap variants with custom styles while the submit button uses only custom styles. Consider using a consistent approach.
- variant="outline-danger" - className={styles.removeButton} + className={styles.cancelButton}Also applies to: 411-411
src/components/OrgPostCard/DeletePostModal.tsx (1)
4-4: Consider preserving Bootstrap's button variants for semantic meaningWhile updating the color scheme, consider keeping Bootstrap's semantic button variants (
variant="danger"andbtn-success) alongside custom styles. This maintains the semantic meaning of actions while allowing custom colors.-className={styles.removeButton} +className={`btn btn-danger ${styles.removeButton}`} -className={`btn ${styles.addButton}`} +className={`btn btn-success ${styles.addButton}`}Also applies to: 50-50, 56-56
src/screens/ManageTag/EditUserTagModal.tsx (1)
77-86: Maintain consistent button orderingThe button order (Cancel/Edit) differs from DeletePostModal (No/Yes). Consider standardizing the button order across modals for better UX.
src/utils/organizationTagsUtils.ts (1)
21-25: Enhance focus visibility for better accessibilityThe current focus outline might be too subtle. Consider:
- Using a higher contrast color for the outline
- Increasing the outline width for better visibility
- outline: '2px solid #000', + outline: '3px solid var(--focus-color)',Also applies to: 41-44
src/components/AddOn/core/AddOnEntry/AddOnEntry.tsx (2)
116-120: Simplify conditional class assignmentThe current conditional class assignment can be simplified for better readability.
-className={ - uninstalledOrgs.includes(currentOrg) - ? styles.addButton - : `btn btn-danger ${styles.removeButton}` -} +className={`btn ${uninstalledOrgs.includes(currentOrg) ? styles.addButton : styles.removeButton}`}
Line range hint
123-137: Clean up commented code and improve icon accessibility
- Remove commented code about the
configurablevariable- Add aria-label to icons for better screen reader support
-// disabled={buttonLoading || !configurable} disabled={buttonLoading} data-testid="AddOnEntry_btn_install" onClick={(): void => { togglePluginInstall(); getInstalledPlugins(); }} > {buttonLoading ? ( <Spinner animation="grow" /> ) : ( <i + aria-label={!isInstalledLocal ? 'Delete' : 'Add'} className={!isInstalledLocal ? 'fa fa-trash' : 'fa fa-plus'} ></i> )} -{/* {installed ? 'Remove' : configurable ? 'Installed' : 'Install'} */}src/screens/FundCampaignPledge/FundCampaignPledge.tsx (1)
429-431: Consider using a single-line event handler.The block statement for the radio button's onChange handler can be simplified to a single line since it only contains one statement.
-onChange={() => { - setProgressIndicator('pledged'); -}} +onChange={() => setProgressIndicator('pledged')}src/screens/ManageTag/RemoveUserTagModal.tsx (1)
48-48: Consider semantic class naming for the "No" buttonWhile the styling looks correct, using
styles.removeButtonfor a "No" button might be semantically confusing. Consider using a more semantically appropriate class name likecancelButtonorsecondaryButton.src/components/OrgListCard/OrgListCard.module.css (1)
63-65: Consider removing redundant !important declarationThe hover state already inherits specificity from the parent selector. The
!importantdeclaration might be unnecessary and could make future style overrides more difficult..manageBtn:hover { - border: 1px solid var(--dropdown-border-color) !important; + border: 1px solid var(--dropdown-border-color); }src/style/app.module.css (2)
Line range hint
104-109: Use breakpoint variables consistentlyThe file defines breakpoint variables but they're not consistently used in media queries throughout the file. Consider using these variables for all breakpoints:
/* breakpoints */ --breakpoint-mobile: 576px; --breakpoint-tablet: 768px; --breakpoint-desktop: 1024px;Replace hardcoded pixel values in media queries with these variables for better maintainability.
Line range hint
1-4000: Consider modularizing CSS for better maintainabilityThe CSS file is quite large and contains styles for many components. Consider:
- Splitting into smaller component-specific modules
- Reducing usage of !important by improving selector specificity
- Creating a separate theme file for color variables
This would improve maintainability and make style updates more manageable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
src/assets/css/app.css(2 hunks)src/components/AddOn/core/AddOnEntry/AddOnEntry.tsx(1 hunks)src/components/AddPeopleToTag/AddPeopleToTag.module.css(0 hunks)src/components/AddPeopleToTag/AddPeopleToTag.tsx(11 hunks)src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.module.css(0 hunks)src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.tsx(4 hunks)src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.tsx(8 hunks)src/components/EventCalendar/EventCalendar.tsx(1 hunks)src/components/EventCalendar/EventHeader.tsx(2 hunks)src/components/EventListCard/EventListCardModals.tsx(19 hunks)src/components/OrgListCard/OrgListCard.module.css(1 hunks)src/components/OrgPeopleListCard/OrgPeopleListCard.tsx(1 hunks)src/components/OrgPostCard/DeletePostModal.tsx(2 hunks)src/components/OrgPostCard/OrgPostCard.tsx(5 hunks)src/components/TagActions/TagActions.module.css(0 hunks)src/components/TagActions/TagActions.tsx(8 hunks)src/components/TagActions/TagNode.tsx(3 hunks)src/components/Venues/VenueCard.tsx(1 hunks)src/components/Venues/VenueModal.tsx(4 hunks)src/screens/BlockUser/BlockUser.tsx(4 hunks)src/screens/FundCampaignPledge/FundCampaignPledge.tsx(7 hunks)src/screens/FundCampaignPledge/PledgeModal.tsx(3 hunks)src/screens/ManageTag/EditUserTagModal.tsx(4 hunks)src/screens/ManageTag/ManageTag.tsx(9 hunks)src/screens/ManageTag/RemoveUserTagModal.tsx(4 hunks)src/screens/ManageTag/UnassignUserTagModal.tsx(4 hunks)src/screens/OrgList/OrgList.tsx(2 hunks)src/screens/OrgPost/OrgPost.tsx(6 hunks)src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx(1 hunks)src/screens/OrganizationEvents/OrganizationEvents.tsx(8 hunks)src/screens/OrganizationFundCampaign/CampaignModal.tsx(3 hunks)src/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx(6 hunks)src/screens/OrganizationFunds/FundModal.tsx(3 hunks)src/screens/OrganizationFunds/OrganizationFunds.tsx(8 hunks)src/screens/OrganizationPeople/AddMember.tsx(1 hunks)src/screens/OrganizationTags/OrganizationTags.tsx(8 hunks)src/screens/SubTags/SubTags.tsx(5 hunks)src/style/app.module.css(27 hunks)src/utils/organizationTagsUtils.ts(2 hunks)
💤 Files with no reviewable changes (3)
- src/components/TagActions/TagActions.module.css
- src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.module.css
- src/components/AddPeopleToTag/AddPeopleToTag.module.css
🧰 Additional context used
📓 Learnings (8)
src/screens/OrganizationFunds/FundModal.tsx (1)
Learnt from: GlenDsza
PR: PalisadoesFoundation/talawa-admin#2064
File: src/screens/OrganizationFunds/OrganizationFunds.tsx:66-72
Timestamp: 2024-11-12T10:40:58.654Z
Learning: Renaming the Enum `Modal` to `ModalState` in the `organizationFunds` component is unnecessary and does not change the functionality.
src/components/TagActions/TagNode.tsx (2)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2362
File: src/components/TagActions/TagNode.tsx:79-109
Timestamp: 2024-11-12T10:41:04.351Z
Learning: In the `TagNode` component at `src/components/TagActions/TagNode.tsx`, the infinite scroll component efficiently manages the calls to `loadMoreSubTags`, preventing multiple simultaneous requests.
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2387
File: src/screens/OrganizationTags/OrganizationTags.tsx:350-376
Timestamp: 2024-11-12T10:41:04.350Z
Learning: In the `src/screens/OrganizationTags/OrganizationTags.tsx` file, the `OrganizationTags` component uses chunk size with the `InfiniteScroll` component and an outer parent div for handling data loading and scrolling.
src/screens/SubTags/SubTags.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2362
File: src/components/TagActions/TagNode.tsx:79-109
Timestamp: 2024-11-12T10:41:04.351Z
Learning: In the `TagNode` component at `src/components/TagActions/TagNode.tsx`, the infinite scroll component efficiently manages the calls to `loadMoreSubTags`, preventing multiple simultaneous requests.
src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx (1)
Learnt from: Chaitanya1672
PR: PalisadoesFoundation/talawa-admin#2049
File: src/screens/OrganizationActionItems/ActionItemUpdateModal.tsx:112-138
Timestamp: 2024-11-12T10:40:58.654Z
Learning: The `istanbul ignore next` comments in the `ActionItemUpdateModal.tsx` file were added as part of a commit that introduced tests for the `ActionItemUpdateModal` component. Removing these comments and writing tests to cover the previously ignored lines is recommended to ensure code integrity and improve test coverage.
src/components/TagActions/TagActions.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2362
File: src/components/TagActions/TagActions.tsx:248-249
Timestamp: 2024-11-12T10:40:58.654Z
Learning: In the `TagActions` component at `src/components/TagActions/TagActions.tsx`, error handling for the `assignToTags` and `removeFromTags` mutation hooks is already implemented, with errors being caught and displayed to the user via toast notifications.
src/screens/ManageTag/ManageTag.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2387
File: src/screens/ManageTag/ManageTag.tsx:113-147
Timestamp: 2024-11-12T10:40:58.654Z
Learning: In the `src/screens/ManageTag/ManageTag.tsx` file, the `InfiniteScroll` component uses the `loadMoreAssignedMembers` function efficiently, and additional error handling is unnecessary.
src/components/AddPeopleToTag/AddPeopleToTag.tsx (2)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2387
File: src/screens/ManageTag/ManageTag.tsx:113-147
Timestamp: 2024-11-12T10:40:58.654Z
Learning: In the `src/screens/ManageTag/ManageTag.tsx` file, the `InfiniteScroll` component uses the `loadMoreAssignedMembers` function efficiently, and additional error handling is unnecessary.
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2355
File: src/components/AddPeopleToTag/AddPeopleToTag.tsx:50-83
Timestamp: 2024-11-12T10:40:58.655Z
Learning: The project currently does not have a GraphQL type generator, so manually typing the result of the `useQuery` hook is acceptable.
src/screens/OrganizationTags/OrganizationTags.tsx (1)
Learnt from: meetulr
PR: PalisadoesFoundation/talawa-admin#2387
File: src/screens/OrganizationTags/OrganizationTags.tsx:350-376
Timestamp: 2024-11-12T10:41:04.350Z
Learning: In the `src/screens/OrganizationTags/OrganizationTags.tsx` file, the `OrganizationTags` component uses chunk size with the `InfiniteScroll` component and an outer parent div for handling data loading and scrolling.
🪛 GitHub Check: codecov/patch
src/screens/FundCampaignPledge/PledgeModal.tsx
[warning] 239-239: src/screens/FundCampaignPledge/PledgeModal.tsx#L239
Added line #L239 was not covered by tests
[warning] 302-302: src/screens/FundCampaignPledge/PledgeModal.tsx#L302
Added line #L302 was not covered by tests
src/screens/SubTags/SubTags.tsx
[warning] 384-384: src/screens/SubTags/SubTags.tsx#L384
Added line #L384 was not covered by tests
src/screens/OrganizationFundCampaign/CampaignModal.tsx
[warning] 267-267: src/screens/OrganizationFundCampaign/CampaignModal.tsx#L267
Added line #L267 was not covered by tests
src/screens/FundCampaignPledge/FundCampaignPledge.tsx
[warning] 395-395: src/screens/FundCampaignPledge/FundCampaignPledge.tsx#L395
Added line #L395 was not covered by tests
[warning] 403-403: src/screens/FundCampaignPledge/FundCampaignPledge.tsx#L403
Added line #L403 was not covered by tests
src/screens/ManageTag/ManageTag.tsx
[warning] 463-463: src/screens/ManageTag/ManageTag.tsx#L463
Added line #L463 was not covered by tests
🪛 Biome (1.9.4)
src/style/app.module.css
[error] 1712-1712: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
🔇 Additional comments (47)
src/components/AddPeopleToTag/AddPeopleToTag.tsx (2)
10-10: LGTM! Good move to centralized styling.Moving to a centralized style module will help maintain consistent styling across the application.
Line range hint
1-421: Overall implementation looks solid!The component demonstrates good practices:
- Proper error handling
- Type safety
- Efficient pagination with InfiniteScroll
- Clear separation of concerns
The only suggestion would be to maintain consistency in the styling approach across the component.
src/screens/ManageTag/EditUserTagModal.tsx (1)
41-41: Verify contrast ratio for modal header and input fieldThe new color scheme changes affect the modal header and input field. Please ensure:
- Modal header text maintains sufficient contrast ratio with the background
- Input field focus state provides clear visual feedback
Also applies to: 60-60
✅ Verification successful
Modal header and input field contrast ratios are compliant
The implementation uses appropriate color combinations:
- Modal header: Black text (#000) on white background provides 21:1 contrast ratio
- Input field: Standard Bootstrap form controls with clear focus state using light-blue outline
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for color definitions in CSS modules rg -A 3 "modalHeader|inputField" "src/style/**/*.css"Length of output: 114
Script:
#!/bin/bash # Find CSS module file in the component directory fd -e css . src/screens/ManageTag # If found, check its contents for modalHeader class and color definitions rg -A 3 "modalHeader|inputField" src/screens/ManageTag/Length of output: 1817
Script:
#!/bin/bash # Find all CSS module files and check for modalHeader fd -e module.css . src/ -x rg -A 3 "modalHeader|inputField|text-white" {} # Check for theme customization fd -e scss -e css . src/ -x rg -A 3 "text-white|--bs-" {}Length of output: 84352
src/components/TagActions/TagActions.tsx (1)
271-271: Consistent use of CSS modules improves style encapsulation.The changes to use CSS modules (
styles.modalHeader,styles.inputField,styles.removeButton,styles.addButton) improve style encapsulation and maintainability.Also applies to: 313-313, 389-400
src/screens/OrganizationTags/OrganizationTags.tsx (1)
Line range hint
394-407: Enhanced keyboard accessibility with improved focus indicators.The addition of clear focus outlines (
2px solid #000) for DataGrid rows and cells improves keyboard navigation accessibility.src/screens/OrganizationEvents/OrganizationEvents.tsx (1)
Line range hint
305-321: Consistent form control and switch styling.The changes maintain styling consistency across the application by:
- Using
styles.inputFieldfor form controls- Using
styles.switchfor form switchesAlso applies to: 429-481
src/screens/FundCampaignPledge/FundCampaignPledge.tsx (1)
474-487: Consistent styling for search functionality.The search input and button styling has been updated to use CSS modules, maintaining consistency with the application's styling patterns.
src/components/OrgPeopleListCard/OrgPeopleListCard.tsx (1)
95-95: Verify semantic meaning of regularBtn classThe class name change from
yesButtontoregularBtnmight affect the semantic meaning of this confirmation button. Ensure this aligns with your design system's button hierarchy.✅ Verification successful
Class name change is appropriate and well-defined
The change from
yesButtontoregularBtnis using a properly defined style from the global stylesheet (app.module.css) that includes hover and active states. The new name is more generic and reusable, which is an improvement over the previous semantically specific name.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other uses of regularBtn class rg "regularBtn" "src/**/*.{tsx,css}"Length of output: 95
Script:
#!/bin/bash # Search for both class names in all relevant files echo "Searching for regularBtn and yesButton..." rg -i "regularBtn|yesButton" --type-add 'styles:*.{css,scss,less,module.css}' --type-add 'tsx:*.tsx' -t styles -t tsx # Find and show content of the CSS module file for this component echo -e "\nChecking CSS module file content..." fd -e css -e module.css "OrgPeopleListCard" --exec cat {}Length of output: 592
src/components/Venues/VenueCard.tsx (1)
93-93: LGTM! Button styling changes align with the new color scheme.The changes to button classes are consistent with the PR's objective of updating the color scheme.
Also applies to: 103-103
src/screens/OrganizationActionItems/ItemUpdateStatusModal.tsx (1)
116-126: LGTM! Button styling changes maintain consistency.The changes to button classes align with the new color scheme while preserving functionality.
Also applies to: 133-133
src/components/TagActions/TagNode.tsx (2)
10-10: LGTM! Style import change aligns with centralized styling.The change to use the global styles module enhances consistency across the application.
64-64: Add test coverage for loadMoreSubTags function.The removal of istanbul ignore comments exposes code paths that need test coverage. Consider adding tests for:
- Handling null fetchMoreResult
- Handling pagination edge cases
Would you like me to help create test cases for these scenarios?
Also applies to: 96-96
src/components/Advertisements/core/AdvertisementEntry/AdvertisementEntry.tsx (1)
102-102: LGTM! Styling changes maintain visual consistency.The updates to card and button classes align with the new color scheme while preserving component functionality.
Also applies to: 166-166, 192-195, 200-200
src/components/Venues/VenueModal.tsx (1)
184-184: LGTM! Form field styling enhances visual consistency.The addition of styles.inputField class to Form.Control components maintains consistent styling across the application.
Also applies to: 202-202, 218-218, 246-246
src/screens/OrganizationFunds/FundModal.tsx (1)
230-230: LGTM! Consistent styling updates for Form.Switch components.The class name updates for Form.Switch components maintain functionality while aligning with the new styling scheme.
Also applies to: 243-243, 261-261
src/screens/OrganizationFundCampaign/CampaignModal.tsx (1)
301-301: LGTM! Consistent button styling.The button class name update aligns with the new color scheme.
src/screens/BlockUser/BlockUser.tsx (2)
Line range hint
188-340: LGTM! Improved layout structure.The restructured layout with updated class names enhances organization while maintaining functionality.
216-217: Remove commented-out code.These commented lines should be removed to maintain code cleanliness.
Also applies to: 256-257
src/screens/OrganizationFunds/OrganizationFunds.tsx (2)
164-164: LGTM! Consistent table header styling.The table header class name updates maintain consistency across all columns.
Also applies to: 178-178, 199-199, 211-211, 229-229, 242-242, 270-270
289-340: LGTM! Enhanced search and filter layout.The restructured search and filter section improves organization and visual hierarchy.
src/screens/FundCampaignPledge/PledgeModal.tsx (2)
336-336: LGTM! Consistent button styling.The button class name update aligns with the new color scheme.
238-243: Add test coverage for form handlers.The onChange handlers for Autocomplete and Select components are not covered by tests.
Run this script to verify existing test coverage:
Also applies to: 301-306
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 239-239: src/screens/FundCampaignPledge/PledgeModal.tsx#L239
Added line #L239 was not covered by testssrc/components/EventCalendar/EventCalendar.tsx (1)
552-552: LGTM! Button styling update aligns with the new color scheme.The change from
styles.btn__todaytostyles.editButtonmaintains functionality while adopting the standardized button styling.src/screens/OrganizationPeople/AddMember.tsx (1)
531-553: LGTM! Modal footer styling updates align with the new design system.The changes appropriately replace Bootstrap variants with custom styles:
- Added Modal.Footer for better structure
- Updated button classes to use
styles.removeButtonandstyles.addButtonsrc/components/OrgPostCard/OrgPostCard.tsx (1)
Line range hint
448-595: LGTM! Comprehensive styling updates maintain consistency across components.The changes successfully implement the new styling system:
- Modal header uses the standardized
styles.modalHeader- Form inputs consistently use
styles.inputField- Buttons adopt the new
styles.addButtonandstyles.removeButtonclassessrc/screens/ManageTag/ManageTag.tsx (2)
Line range hint
289-329: LGTM! Table and button styling updates align with the new design system.The changes consistently apply the new styling classes:
- Table headers use
styles.tableHeader- Buttons use
styles.editButton
462-470: Add test coverage for the Stack component.The Stack component used for the empty state UI needs test coverage.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 463-463: src/screens/ManageTag/ManageTag.tsx#L463
Added line #L463 was not covered by testssrc/components/EventListCard/EventListCardModals.tsx (1)
Line range hint
410-825: LGTM! Comprehensive styling updates ensure consistency across the modal.The changes successfully implement the new styling system throughout the component:
- Form inputs use
styles.inputField- Switch components use
styles.switch- Buttons use
styles.addButtonandstyles.removeButton- Modal headers use
styles.modalHeadersrc/style/app.module.css (1)
1711-1714:⚠️ Potential issueFix padding property override in modalHeader
The modalHeader class has conflicting padding declarations where padding-bottom is overridden by the shorthand padding property.
.modalHeader { border: none; - padding-bottom: 0; - background-color: var(--white-color) !important; - padding: 1rem 1rem; + background-color: var(--white-color) !important; + padding: 0 1rem 1rem; color: #000000; }Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1712-1712: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
src/assets/css/app.css (3)
3904-3904: Dropdown active background color change looks good.The change from green (#31bb6b) to light blue (#a8c7fa) for the dropdown active background aligns with the PR objective of replacing green tones with blue tones.
5648-5648: Progress bar background color change looks good.The change from green (#31bb6b) to blue (#0056b3) for the progress bar background aligns with the PR objective of replacing green tones with blue tones.
Line range hint
130-150: Custom color variables look good.The custom color variables use appropriate blue and grey tones that align with the new color scheme:
- Dropdown colors use grey tones
- Background colors use blue-grey tones
- Hover states use appropriate darker variants
src/screens/ManageTag/UnassignUserTagModal.tsx (3)
4-4: LGTM!The import statement for styles is correctly added.
38-38: LGTM!The modal header class is correctly updated to use the new color scheme.
Line range hint
49-59: LGTM!The button classes are correctly updated to use the new color scheme:
- "No" button uses
styles.removeButton- "Yes" button uses
styles.addButtonsrc/screens/OrganizationFundCampaign/OrganizationFundCampagins.tsx (4)
9-9: LGTM!The Row import is correctly added for the new layout structure.
Line range hint
266-266: LGTM!The button class is correctly updated to use the new color scheme.
Line range hint
343-397: LGTM!The layout structure and styling classes are correctly updated:
- Uses Row component for better layout
- Input field uses the new color scheme
- Search button styling is consistent
- Button group styling is improved
383-391: Add test coverage for the noRowsOverlay component.The noRowsOverlay component lacks test coverage according to the static analysis report.
Run the following script to verify existing test coverage:
src/screens/SubTags/SubTags.tsx (1)
266-266: LGTM!The styling classes are correctly updated to use the new color scheme:
- Button uses
styles.editButton- Modal header uses
styles.modalHeader- Input field uses
styles.inputField- Cancel button uses
styles.removeButtonAlso applies to: 420-420, 432-432, 449-449
src/components/Advertisements/core/AdvertisementRegister/AdvertisementRegister.tsx (1)
252-252: LGTM!The styling classes are correctly updated to use the new color scheme:
- Create button uses
styles.dropdown- All input fields use
styles.inputField- Close button uses
styles.removeButton- Save button uses
styles.addButtonAlso applies to: 289-289, 316-316, 372-372, 391-391, 407-407, 416-416, 433-433
src/screens/OrgPost/OrgPost.tsx (3)
415-415: LGTM! Consistent styling for input fields.The addition of
styles.inputFieldclass to Form.Control components maintains visual consistency across the application.Also applies to: 432-432, 457-457
509-509: LGTM! Switch styling update.The addition of
styles.switchclass to Form.Switch aligns with the new color scheme.
516-516: LGTM! Button styling consistency.The update of button classes to use
styles.removeButtonandstyles.addButtonmaintains consistency with the new color scheme.Also applies to: 526-526
src/screens/OrgList/OrgList.tsx (2)
337-337: LGTM! Consistent input field styling.The update to use
styles.inputFieldmaintains visual consistency across the application.
346-347: Verify search button positioning.The removal of positioning classes (
position-absolute, etc.) might affect the search button's layout. Please ensure the button remains properly aligned with the input field.Run this script to check for similar button implementations across the codebase:
✅ Verification successful
Search button positioning is properly maintained through CSS modules
The removal of inline positioning classes is actually an improvement as the button's positioning is correctly handled by the CSS module (
.searchButtonOrgListclass). This change follows the established pattern across the codebase and maintains proper separation of styling concerns.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for similar search button implementations # Look for search button patterns in other components rg -B 2 -A 2 'className=.*searchButton'Length of output: 7822
Script:
#!/bin/bash # Check the CSS module for search button styling fd -e module.scss -e module.css | xargs rg -B 2 -A 2 "searchButton"Length of output: 1057
src/components/EventCalendar/EventHeader.tsx (1)
55-55: Skip review of empty line.
|
@palisadoes updating SuperAdmin Screen and User Screens has made changes in many file. Should I cover those changes in next PR like (Updating to NEW COLOR SCHEME Part 2) or continue in this PR? I think merge this PR and I will continue changes in next PR to keep Admin updates and User Portal Updates Distinct. |
|
We need to:
I'd like to get the first merge done today if all looks good. |
|
|
@palisadoes Haven't used any new component for popups. But used common CSS class for styling across all components and screens. For example all buttons with create/add functionality have .addButton class, buttons with remove functionality have .removeButton class, toggles have .switch class should this be added as comment in code? |
What kind of change does this PR introduce?
Replace green tones with the pre-existing blue / grey tones.
Issue Number:
#2880
If relevant, did you update the documentation?
N/A
Summary
Replaced #31bb6b with #afffe8 and changed text color wherever required.
Does this PR introduce a breaking change?
No
Have you read the contributing guide?
Yes
Summary by CodeRabbit