Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughStyling across multiple SCSS modules was refactored to consolidate card, corner, and shadow styles by enhancing the Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Any Component
participant SCSS as SCSS Module
participant Variants as variants.card Mixin
Component->>SCSS: Use .class with variants.card($elevated: true/false)
SCSS->>Variants: Call @mixin card($elevated)
Variants->>SCSS: Apply corners, shadows, and elevation based on $elevated
SCSS->>Component: Render styled component
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
src/style/_variants.scss (1)
3-4: Unreferenced module import – consider dropping@use "/src/style/shadows"unless it produces global CSS
shadowsis now only referenced inside this same file, so keeping the@usehere is fine.
However, if no other rules from/src/style/shadowsemit top-level CSS, you could instead rely on the implicit load that happens when this file itself includesshadows.*mixins later on and remove the explicit@useto keep import noise to a minimum.
Totally optional – feel free to ignore if you prefer explicitness.src/components/generic/DataTable/DataTable.module.scss (1)
1-8:shadowsis no longer used in this fileAfter migrating to
variants.card, the direct@use "/src/style/shadows"on line 4 appears unused.
Removing it will shave a few cycles off the Sass compiler and avoid confusion.src/components/AvatarEditable/AvatarEditable.module.scss (1)
1-8: Unused imports after refactor
@use "/src/style/shadows";and@use "/src/style/corners";are no longer referenced in this module.
Consider deleting them to keep the dependency list lean.src/components/ToastProvider/ToastProvider.module.scss (1)
1-6:cornersnamespace appears unusedSince the wide-corner mixin was removed,
@use "/src/style/corners";(line 1) may now be redundant.src/components/generic/InputDateTime/InputDateTime.module.scss (2)
22-33: Redundant overrides defeat the purpose of the unified mixin
variants.card($elevated: true)already provides background colour and radius.
Re-declaring them (lines 30-31) introduces duplication and a maintenance hotspot.
Unless you need a different radius or colour here, consider deleting those two lines.
1-6: Drop unusedshadowsimportAfter switching to the enhanced
cardmixin, direct calls toshadows.*are gone, so the@use "/src/style/shadows";on line 4 can likely be removed.src/pages/TournamentsPage/TournamentsPage.module.scss (1)
1-4: Drop unusedshadowsimport & avoid dead codeAfter consolidating shadow logic into
variants.card($elevated: true)the explicit@use "/src/style/shadows";is no longer needed and will be flagged by most SCSS linters.-@use "/src/style/shadows";Removing it keeps the dependency graph clean and avoids accidental regression if the mixin name changes later.
Also applies to: 14-15
src/components/generic/InputLocation/InputLocation.module.scss (1)
3-5: Duplicate overrides & unused import
variants.cardalready setsbackground-colorandborder-radius; the explicit declarations a few lines below are redundant.@use "/src/style/shadows";is now unused.-@use "/src/style/shadows"; … - background-color: var(--card-bg); - border-radius: variables.$corner-radius;Removing these keeps styles DRY and prevents divergence if the token changes in the mixin.
Also applies to: 9-10, 19-21
src/components/generic/PopoverMenu/PopoverMenu.module.scss (1)
1-5: Clean-up after mixin consolidation
variants.card($elevated: true)replaces the need for:
- The
shadowsimport (nothing in this file references it now).- The explicit
background-coloroverride.-@use "/src/style/shadows"; … - background-color: var(--card-bg);This trims the bundle and avoids future inconsistencies.
Also applies to: 13-14, 18-19
src/components/AccountMenu/AccountMenu.module.scss (1)
1-6: Remove staleshadowsdependency
variants.card($elevated: true)now encapsulates elevation, so the shadows module is unused in this sheet.-@use "/src/style/shadows";Minor tidy-up that avoids superfluous compilation overhead.
Also applies to: 19-21
src/components/MatchResultPlayers/MatchResultPlayers.module.scss (1)
1-4: UnreferencedshadowsmoduleSimilar to other modules in this PR, the local file no longer calls any mixins from
/src/style/shadows.-@use "/src/style/shadows";Pruning it keeps imports minimal and maintainable.
Also applies to: 111-114
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/components/AccountMenu/AccountMenu.module.scss(1 hunks)src/components/AvatarEditable/AvatarEditable.module.scss(1 hunks)src/components/MatchResultCard/MatchResultCard.module.scss(0 hunks)src/components/MatchResultPlayers/MatchResultPlayers.module.scss(1 hunks)src/components/ToastProvider/ToastProvider.module.scss(1 hunks)src/components/TournamentCard/TournamentCard.module.scss(0 hunks)src/components/generic/Card/Card.module.scss(0 hunks)src/components/generic/DataTable/DataTable.module.scss(1 hunks)src/components/generic/InputDateTime/InputDateTime.module.scss(1 hunks)src/components/generic/InputLocation/InputLocation.module.scss(1 hunks)src/components/generic/PopoverMenu/PopoverMenu.module.scss(1 hunks)src/pages/TournamentsPage/TournamentsPage.module.scss(1 hunks)src/style/_variants.scss(2 hunks)
💤 Files with no reviewable changes (3)
- src/components/TournamentCard/TournamentCard.module.scss
- src/components/MatchResultCard/MatchResultCard.module.scss
- src/components/generic/Card/Card.module.scss
🔇 Additional comments (3)
src/components/generic/DataTable/DataTable.module.scss (1)
30-33: Great consolidation – one mixin call does it all
Replacing three separate includes withvariants.card($elevated: true)improves clarity and keeps the pop-over styling consistent with the new design system.
✅ No issues spotted here.src/components/AvatarEditable/AvatarEditable.module.scss (1)
26-34: Mixin upgrade looks good
variants.card($elevated: true)is the right call here – it removes several redundant declarations and keeps the menu styling in sync with other components.src/components/ToastProvider/ToastProvider.module.scss (1)
27-38: Potentially duplicated styling
variants.card($elevated: true)already setsbackground-color: var(--card-bg)and an elevated shadow.
Lines 37-40 re-declare both, which will override whatever the mixin emits.
If the intention is to customise the toast’s shadow, leave as-is; otherwise you can drop the extra declarations to rely on the centralised mixin.
| @mixin card($elevated: false) { | ||
| @include borders.normal; | ||
| @include corners.normal; | ||
| @include shadows.surface; | ||
|
|
||
| background-color: var(--card-bg); | ||
| border-color: var(--border-color-default); | ||
|
|
||
| @if $elevated { | ||
| @include shadows.elevated; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
card() applies both surface and elevated shadows – likely unintended
Because @include shadows.surface; is unconditional and @include shadows.elevated; is appended when $elevated is true, the elevated version overrides the surface box-shadow only if the shadows.* mixins both set box-shadow.
If either mixin appends to box-shadow (comma-separated shadows) you’ll end up with two shadows instead of one, which wasn’t the behaviour of the previous code.
A safer pattern is to choose exactly one shadow set:
@include borders.normal;
@include corners.normal;
- @include shadows.surface;
-
- @if $elevated {
- @include shadows.elevated;
- }
+ @if $elevated {
+ @include shadows.elevated;
+ } @else {
+ @include shadows.surface;
+ }This preserves the original visual intent and avoids accidental double shadows.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @mixin card($elevated: false) { | |
| @include borders.normal; | |
| @include corners.normal; | |
| @include shadows.surface; | |
| background-color: var(--card-bg); | |
| border-color: var(--border-color-default); | |
| @if $elevated { | |
| @include shadows.elevated; | |
| } | |
| } | |
| @mixin card($elevated: false) { | |
| @include borders.normal; | |
| @include corners.normal; | |
| @if $elevated { | |
| @include shadows.elevated; | |
| } @else { | |
| @include shadows.surface; | |
| } | |
| background-color: var(--card-bg); | |
| } |
🤖 Prompt for AI Agents
In src/style/_variants.scss around lines 195 to 205, the card mixin currently
includes both surface and elevated shadows when $elevated is true, causing
double shadows if shadows.* mixins append rather than override box-shadow. To
fix this, remove the unconditional @include shadows.surface and instead use a
conditional to include either shadows.surface or shadows.elevated based on the
$elevated flag, ensuring only one shadow style is applied at a time.
* Update update-project-status.yml * Update updateProjectStatus.js * fix: Toast text does not wrap (#87) * fix: #86 Sanitize sign in/sign up inputs (#91) * feat: #32 Auto generate avatars & refactor users (#90) * feat: Improve <TournamentDetailPage/> default tab * feat: Improve <TournamentCard/> styling * Update mockData.ts * feat: Improve <AccordionItem/> disabled state (#97) #94 * feat: Hide completed pairings from match check-in (#96) #95 * bug: Preserve <TournamentPairingsGrid/> internal state (#98) #93 * feat: #101 Add player count to roster (#103) * feat: Show full player names when tournaments require it * feat: Add activePlayerCount to deep tournaments * feat: Sort tournament competitors by name * task: #100 Clean-up .card mixin (#102) * feat: #99 Improve tournament competitor edit dialog (#104) * feat: #106 Improve signIn error handling (#107) * Update convex/_model/tournamentCompetitors/queries/getTournamentCompetitorsByTournament.ts * Update convex/_model/users/_helpers/checkUserTournamentForcedName.ts * feat: Hide players with 0 matches from rankings
* Update update-project-status.yml * Update updateProjectStatus.js * fix: Toast text does not wrap (#87) * fix: #86 Sanitize sign in/sign up inputs (#91) * feat: #32 Auto generate avatars & refactor users (#90) * feat: Improve <TournamentDetailPage/> default tab * feat: Improve <TournamentCard/> styling * Update mockData.ts * feat: #94 Improve <AccordionItem/> disabled state (#97) * feat: #95 Hide completed pairings from match check-in (#96) * bug: #93 Preserve <TournamentPairingsGrid/> internal state (#98) * feat: #101 Add player count to roster (#103) * feat: Show full player names when tournaments require it * feat: Add activePlayerCount to deep tournaments * feat: Sort tournament competitors by name * task: #100 Clean-up .card mixin (#102) * feat: #99 Improve tournament competitor edit dialog (#104) * feat: #106 Improve signIn error handling (#107) * Update convex/_model/tournamentCompetitors/queries/getTournamentCompetitorsByTournament.ts * Update convex/_model/users/_helpers/checkUserTournamentForcedName.ts * feat: Hide players with 0 matches from rankings * feat: #112 Add more mercenary team options (#113) * feat: #110 Add manual table assignments (#111) * fix: Ensure round 0 rankings can be included * Refactor tournament actions (#114) * refactor: Improve tournament actions * chore: Clean-up Convex errors * fix: Do not try to clean up current round timer on tournament end * fix: Don't allow players to be removed from tournament * chore: Update test tournament banner image * Update TournamentCard.tsx * fix: Ensure round 0 rankings can be included * fix: Fix end tournament round context menu behavior * chore: Improve mock match result creation * feat: Allow matchResult.playedAt to be date string or number * feat: #115 Hide match result battle plans (#116) * feat: Set page title based on <PageWrapper/> title prop * fix: Use <IdentityBadge/> to fix player name spacing on match results * fix: Correctly include match results relevant to a tournament
* Update update-project-status.yml * Update updateProjectStatus.js * fix: Toast text does not wrap (#87) * fix: Sanitize sign in/sign up inputs (#91) #86 * feat: #32 Auto generate avatars & refactor users (#90) * feat: Improve <TournamentDetailPage/> default tab * feat: Improve <TournamentCard/> styling * Update mockData.ts * feat: Improve <AccordionItem/> disabled state (#97) #94 * feat: Hide completed pairings from match check-in (#96) #95 * bug: Preserve <TournamentPairingsGrid/> internal state (#98) #93 * feat: #101 Add player count to roster (#103) * feat: Show full player names when tournaments require it * feat: Add activePlayerCount to deep tournaments * feat: Sort tournament competitors by name * task: Clean-up .card mixin (#102) #100 * feat: #99 Improve tournament competitor edit dialog (#104) * feat: #106 Improve signIn error handling (#107) * Update convex/_model/tournamentCompetitors/queries/getTournamentCompetitorsByTournament.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update convex/_model/users/_helpers/checkUserTournamentForcedName.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: Hide players with 0 matches from rankings * feat: #112 Add more mercenary team options (#113) * feat: #110 Add manual table assignments (#111) * fix: Ensure round 0 rankings can be included * Refactor tournament actions (#114) * refactor: Improve tournament actions * chore: Clean-up Convex errors * fix: Do not try to clean up current round timer on tournament end * fix: Don't allow players to be removed from tournament * chore: Update test tournament banner image * Update TournamentCard.tsx * fix: Ensure round 0 rankings can be included * fix: Fix end tournament round context menu behavior * chore: Improve mock match result creation * feat: Allow matchResult.playedAt to be date string or number * feat: #115 Hide match result battle plans (#116) * feat: Set page title based on <PageWrapper/> title prop * fix: Use <IdentityBadge/> to fix player name spacing on match results * fix: Correctly include match results relevant to a tournament * feat: #57 Implement basic dashboard (#119) * fix: Remove double border on dashboard sections * fix: Add key to dashboard tournaments * feat: Improve <TournamentPairingRow/> styling * fix: Remove extraneous error message * fix: Improve <Form/> isDirty calculation * fix: Render all competitors in <TournamentCompetitorForm/> * fix: Also show empty state if rankings are empty --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
#100
Summary by CodeRabbit