Refactored CSS files in src/screens/Requests #3570
Conversation
WalkthroughThis pull request updates documentation and UI styling without altering core functionality. The documentation for the Changes
Assessment against linked issues
Possibly related issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
![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 (6)
src/screens/Requests/Requests.tsx (1)
3-3: Remove unused Bootstrap Table import.The
Tableimport from 'react-bootstrap' is no longer used after migrating to Material-UI components.-import { Table } from 'react-bootstrap';src/style/app.module.css (5)
242-242: Fix typo in CSS variable name.The variable name
--dorpdownItem-hover-shadowhas a typo - "dorp" should be "drop".Apply this fix:
- --dorpdownItem-hover-shadow: 2.5px 2.5px 2.5px rgba(0, 0, 0, 0.3); + --dropdownItem-hover-shadow: 2.5px 2.5px 2.5px rgba(0, 0, 0, 0.3);
1096-1100: Remove non-standard CSS property usage.The
&::beforesyntax is not valid CSS - it's a preprocessor feature. This will not work in regular CSS.Apply this fix:
-.errorIcon { - /* Add error icon for non-color indication */ - &::before { - content: '⚠️'; - margin-right: 0.5rem; - } -} +.errorIcon::before { + content: '⚠️'; + margin-right: 0.5rem; +}
4063-4077: Remove non-standard global CSS module syntax.The
:global()syntax is not valid CSS - it's specific to CSS modules. This will not work in regular CSS.Apply this fix:
-.selectOrgText { - :global(.MuiOutlinedInput-root.Mui-focused) fieldset { - border-color: var(--primary-color); - } - - :global(.MuiInputLabel-root.Mui-focused) { - color: var(--primary-color); - } - - :global(.MuiAutocomplete-option:hover) { - background-color: var(--primary-color); - color: var(--text-color-light); - } -} +/* Material-UI overrides */ +.MuiOutlinedInput-root.Mui-focused fieldset { + border-color: var(--primary-color); +} + +.MuiInputLabel-root.Mui-focused { + color: var(--primary-color); +} + +.MuiAutocomplete-option:hover { + background-color: var(--primary-color); + color: var(--text-color-light); +}
2184-2204: Consolidate duplicate keyframe animations.There are multiple duplicate keyframe animations for
load8,zoomIn,fadeIn, etc. This violates DRY principle.Consolidate the animations into a single definition for each unique animation:
-@-webkit-keyframes load8 { ... } -@keyframes load8 { ... } -@-webkit-keyframes load8 { ... } -@keyframes load8 { ... } +@keyframes load8 { + 0% { + transform: rotate(0deg); + } + 100% { + transform: rotate(360deg); + } +} -@-webkit-keyframes zoomIn { ... } -@keyframes zoomIn { ... } -@-webkit-keyframes zoomIn { ... } -@keyframes zoomIn { ... } +@keyframes zoomIn { + 0% { + opacity: 0; + transform: scale(0.5); + } + 100% { + opacity: 1; + transform: scale(1); + } +} // Similarly consolidate other duplicate animationsAlso applies to: 2195-2204, 3543-3564, 4493-4516, 4518-4542, 4633-4651, 4686-4696, 8757-8853
525-532: Consolidate duplicate media queries.There are many duplicate media queries scattered throughout the file. This makes maintenance difficult and increases file size.
Consolidate all media queries at the end of the file and group them by breakpoint:
/* Base styles */ ... /* Media Queries */ @media (max-width: 1440px) { /* Consolidate all 1440px breakpoint styles */ } @media (max-width: 1120px) { /* Consolidate all 1120px breakpoint styles */ } @media (max-width: 820px) { /* Consolidate all 820px breakpoint styles */ } @media (max-width: 520px) { /* Consolidate all 520px breakpoint styles */ } /* And so on for other breakpoints */Also applies to: 818-821, 1818-1829, 1990-2010, 2106-2133, 2151-2182, 2257-2260, 3406-3420, 3831-3886, 4256-4284, 4425-4480, 4662-4696, 4818-4853, 5060-5106, 5257-5303, 5405-5415, 5662-5686, 5818-5886, 6626-6634, 7255-7302, 7918-7940, 8262-8303, 8727-8755, 8962-9014
🧹 Nitpick comments (4)
src/screens/Requests/Requests.tsx (1)
36-40: Enhance component documentation.Consider adding more details to the documentation:
- Mention the Material-UI table integration
- Describe the table structure and columns
- Explain the search and filter functionality
/** * The `Requests` component fetches and displays a paginated list of membership requests * for an organization, with functionality for searching, filtering, and infinite scrolling. + * + * The component uses Material-UI's Table components for consistent styling and accessibility. + * It displays member information in columns: serial number, name, email, and action buttons. + * The search functionality filters requests by member's first name. * */src/style/app.module.css (3)
652-657: Consolidate duplicate box-shadow styles.The
.input:activeand.input:focushave identical box-shadow styles. This violates DRY principle.Consolidate the styles using a common selector:
-.input:active { - box-shadow: var(--dorpdownItem-hover-shadow) !important; - background-color: var(--create-button-bg-color); - border-color: var(--input-shadow) !important; - color: var(--input-text-color); -} - -.input:focus { - box-shadow: var(--dorpdownItem-hover-shadow) !important; - border-color: var(--input-shadow) !important; -} +.input:is(:active, :focus) { + box-shadow: var(--dropdownItem-hover-shadow) !important; + border-color: var(--input-shadow) !important; +} + +.input:active { + background-color: var(--create-button-bg-color); + color: var(--input-text-color); +}Also applies to: 659-662
2290-2299: Remove duplicate pseudo-element styles.The
&::afterblocks for.subTagsLinkand.tagsBreadCrumbsare identical. This violates DRY principle.Consolidate the styles using a common selector:
-.subTagsLink { - /* Prevent layout shift */ - &::after { - display: block; - content: attr(data-text); - font-weight: 600; - height: 0; - overflow: hidden; - visibility: hidden; - } -} - -.tagsBreadCrumbs { - /* Prevent layout shift */ - &::after { - display: block; - content: attr(data-text); - font-weight: 600; - height: 0; - overflow: hidden; - visibility: hidden; - } -} +.subTagsLink::after, +.tagsBreadCrumbs::after { + display: block; + content: attr(data-text); + font-weight: 600; + height: 0; + overflow: hidden; + visibility: hidden; +}Also applies to: 2304-2313
1-9014: Consider splitting into multiple files.The CSS file is very large (9000+ lines) which makes it hard to maintain. Consider splitting it into multiple files based on functionality:
- variables.css (CSS variables)
- animations.css (keyframe animations)
- base.css (base styles)
- components/ (component-specific styles)
- layouts/ (layout styles)
- media-queries.css (consolidated media queries)
🧰 Tools
🪛 Biome (1.9.4)
[error] 2231-2231: Unexpected shorthand property padding after padding-bottom
(lint/suspicious/noShorthandPropertyOverrides)
[error] 5949-5949: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.(lint/correctness/noUnknownProperty)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/docs/auto-docs/screens/Requests/Requests/functions/default.md(1 hunks)src/screens/Requests/Requests.tsx(2 hunks)src/style/app.module.css(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/docs/auto-docs/screens/Requests/Requests/functions/default.md
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Check Python Code Style
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (3)
src/screens/Requests/Requests.tsx (1)
280-320: Well-implemented Material-UI table migration!The table implementation:
- Uses proper Material-UI components and patterns
- Maintains accessibility with ARIA labels and roles
- Preserves existing functionality
- Follows consistent styling with Paper component
src/style/app.module.css (2)
1-36: Excellent CSS documentation and methodology.The CSS documentation and methodology is well-structured and provides clear guidelines for:
- Reducing CSS duplication through global classes
- Naming conventions with clear examples
- List of global classes with their purposes
This helps maintain consistency and simplifies maintenance.
38-378: Well-organized CSS variables with semantic naming.The CSS variables are well-organized into logical groups:
- Neutral colors
- Blue shades
- Red shades
- Yellow/Orange shades
- Miscellaneous colors
- Font sizes
- Borders and radius
- Loader sizes
- Image sizes
- Modal dimensions
This makes the codebase more maintainable and consistent.
|
Please fix the failing test |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3570 +/- ##
=================================================
Coverage 85.87% 85.87%
=================================================
Files 342 342
Lines 8882 8882
Branches 1911 1911
=================================================
Hits 7627 7627
Misses 913 913
Partials 342 342 ☔ View full report in Codecov by Sentry. |
palisadoes
merged commit 0116e8b
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Refactoring
Issue Number:
Fixes #2525
Summary
A part of the Issue was already completed like streamlining CSS to a single file. I updated the Requests table to match the styles of the Table used across the application and added CSS variables wherever required.
Does this PR introduce a breaking change?
No
Checklist
CodeRabbit AI Review
Test Coverage
Have you read the contributing guide?
Yes
Summary by CodeRabbit
default()function.Search by Category nametest case to enhance the waiting mechanism.