[REFACTOR] CSS for Events.tsx #2998
Conversation
WalkthroughThis pull request focuses on refactoring the CSS for the Events module in the Talawa-Admin application. The changes involve removing the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/screens/UserPortal/Events/Events.tsx (1)
345-346: Updated checkbox container classes.
Renaming fromcheckboxdiv/dispflextocheckboxdivEvents/dispflexEventsadds clarity, but watch out for potential confusion if other modules also need “checkboxdiv”-like layouts.Also applies to: 357-357, 369-370, 381-381
src/style/app.module.css (1)
3484-3633: Impressive consolidation of event-specific styles.
Introducing classes liketitlemodalEvents,dateboxEvents, andcheckboxdivEventssuccinctly organizes event UI elements in one place. A few suggestions:
• Consider referencing shared color variables (e.g.,#7c9beb) from:rootto centralize brand colors and keep consistency.
• Evaluate if some classes (e.g.,.borderNone,.colorWhite) can be reused project-wide rather than duplicating utility classes.
Overall, this is a clean approach to unifying the styling forEvents.tsx.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/screens/UserPortal/Events/Events.module.css(0 hunks)src/screens/UserPortal/Events/Events.tsx(11 hunks)src/style/app.module.css(1 hunks)
💤 Files with no reviewable changes (1)
- src/screens/UserPortal/Events/Events.module.css
🔇 Additional comments (4)
src/screens/UserPortal/Events/Events.tsx (4)
22-22: Use caution with indirect CSS imports.
Importing global CSS may inadvertently affect other components. Ensure that the references in app.module.css won't cause conflicts or override styles in unrelated parts of the application.
239-239: Well-organized modal title styling.
Switching from titlemodal to titlemodalEvents is clear and consistent.
283-283: Consistent naming for date/time picker classes.
Renaming to dateboxEvents properly differentiates the event-related pickers. Good job maintaining a consistent theme across these elements.
Also applies to: 287-287, 301-301, 317-317, 332-332
395-395: Button color change requires UX sign-off.
You’ve replaced the green register button with blueregbtnEvents. Verify that the updated color meets accessibility guidelines (contrast ratio, brand alignment, etc.).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2998 +/- ##
=====================================================
+ Coverage 31.32% 88.93% +57.61%
=====================================================
Files 299 320 +21
Lines 7429 8290 +861
Branches 1624 1869 +245
=====================================================
+ Hits 2327 7373 +5046
+ Misses 4897 692 -4205
- Partials 205 225 +20 ☔ View full report in Codecov by Sentry. |
…wa-admin into refactor-issue#2892
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/style/app.module.css (2)
33-34: Consider using semantic color variables.Instead of using direct hex color codes, consider:
- Reusing existing color variables if similar colors exist
- Creating semantic variables that describe the purpose (e.g.,
--date-picker-background,--text-dark)This helps maintain consistency and makes it easier to update the theme.
:root { - --bg-date-picker: #f2f2f2; - --grey-bg-color-dark: #707070; + /* Reuse existing variables if available */ + --date-picker-background: var(--grey-bg-color, #f2f2f2); + --text-dark: var(--brown-color, #707070); }
3526-3541: Consider leveraging Bootstrap utility classes.Instead of creating custom utility classes, consider using Bootstrap's built-in utilities:
borderNone→border-0colorWhite→text-whitebackgroundWhite→bg-whitemaxWidth→mw-100with custom max-widthThis reduces CSS bloat and maintains consistency with Bootstrap's utility-first approach.
|
Please make code rabbit approve your PR |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…wa-admin into refactor-issue#2892
…alawa-admin into refactor-issue#2892
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/style/app.module.css (4)
3542-3550: Improve responsive layout handling.The current layout implementation has potential issues:
- Fixed percentage width might not work well on all screen sizes
- Fixed pixel values for gaps and padding reduce flexibility
- Overflow handling might cause content loss on mobile
Consider these improvements:
.mainContainer { margin-top: 2rem; width: 100%; max-width: 800px; flex-grow: 3; - max-height: 90vh; + max-height: min(90vh, 800px); overflow: auto; padding: 0 1rem; + @media (max-width: 768px) { + margin-top: 1rem; + padding: 0 0.5rem; + } }
3575-3582: Enhance form control accessibility.The datePicker class needs accessibility improvements.
Consider these enhancements:
.datePicker { border-radius: 10px; height: 2.5rem; text-align: center; background-color: var(--date-picker-background); border: none; width: 100%; + cursor: pointer; + &:focus { + outline: 2px solid var(--subtle-blue-grey); + outline-offset: 2px; + } + &:focus-visible { + outline: 2px solid var(--subtle-blue-grey); + outline-offset: 2px; + } }
3656-3675: Optimize button styles.The button implementation has a duplicate width property and could benefit from better hover state handling.
Apply these improvements:
.blueregbtnEvents { margin-top: var(--spacing-lg, 1.25rem); border: 1px solid var(--grey-border-box-color); box-shadow: 0 2px 2px var(--grey-border-box-color); padding: var(--spacing-md, 0.625rem); border-radius: 5px; background-color: var(--subtle-blue-grey); - width: 100%; font-size: 16px; color: var(--bs-white); outline: none; font-weight: 600; cursor: pointer; transition: all 0.2s ease-in-out; width: 100%; &:hover { transform: translateY(-1px); box-shadow: 0 4px 6px var(--grey-border-box-color); + background-color: var(--subtle-blue-grey-hover); } + &:focus-visible { + outline: 2px solid var(--subtle-blue-grey); + outline-offset: 2px; + } + &:active { + transform: translateY(0); + } }
3632-3654: Improve form layout structure.The checkbox and input layouts could be more maintainable and accessible.
Consider these improvements:
.checkboxdivEvents { display: flex; + flex-wrap: wrap; + gap: var(--spacing-md, 0.625rem); } .checkboxdivEvents > div { - width: 50%; + flex: 1; + min-width: 200px; } .checkboxdivEvents > label { - margin-right: 50px; + margin-right: var(--spacing-lg, 1.25rem); + display: flex; + align-items: center; + gap: var(--spacing-sm, 0.5rem); } .dispflexEvents { display: flex; align-items: center; + gap: var(--spacing-sm, 0.5rem); } .dispflexEvents > input { border: none; box-shadow: none; - margin-top: 5px; + margin: 0; + padding: var(--spacing-sm, 0.5rem); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/style/app.module.css(2 hunks)
🔇 Additional comments (2)
src/style/app.module.css (2)
33-34: LGTM! Well-defined CSS variables.
The new CSS variables are appropriately named and their values are suitable for their intended use in the Events component.
3526-3540: LGTM! Well-organized utility classes.
The utility classes are well-structured and follow a clear naming convention.
palisadoes
merged commit a788c5e
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Streamlined all CSS for Events.tsx into a single global file, fixed UI bugs on the Events.tsx , and ensured no conflicts with other pages.
Issue Number:
Fixes #2892
Did you add tests for your changes?
No
Does this PR introduce a breaking change?
No
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Style Changes
Refactor