polishing#5
Conversation
WalkthroughBumped two devDependencies in the root Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/spacings/src/_variables.scss (3)
2-22: Namespace the base variable and add a safe fallback to avoid global collisions.
--spacingis very generic and lives in:root; easy to clash with app/global CSS. Recommend a namespaced base (keep--spacingas an alias for backward compatibility) and use a fallback in derived tokens so partial imports don’t break.Apply:
:root { - --spacing: 0.125rem; + /* Namespaced base + legacy alias */ + --ds-spacing: 0.125rem; + --spacing: var(--ds-spacing); - --spacing-00: 0px; /* 0rem */ - --spacing-01: var(--spacing); /* 0.125rem */ - --spacing-02: calc(var(--spacing) * 2); /* 0.25rem */ - --spacing-03: calc(var(--spacing) * 3); /* 0.375rem */ - --spacing-04: calc(var(--spacing) * 4); /* 0.5rem */ - --spacing-05: calc(var(--spacing) * 6); /* 0.75rem */ - --spacing-06: calc(var(--spacing) * 8); /* 1rem */ - --spacing-07: calc(var(--spacing) * 10); /* 1.25rem */ - --spacing-08: calc(var(--spacing) * 12); /* 1.5rem */ - --spacing-09: calc(var(--spacing) * 16); /* 2rem */ - --spacing-10: calc(var(--spacing) * 20); /* 2.5rem */ - --spacing-11: calc(var(--spacing) * 24); /* 3rem */ - --spacing-12: calc(var(--spacing) * 28); /* 3.5rem */ - --spacing-13: calc(var(--spacing) * 32); /* 4rem */ - --spacing-14: calc(var(--spacing) * 36); /* 4.5rem */ - --spacing-15: calc(var(--spacing) * 40); /* 5rem */ - --spacing-16: calc(var(--spacing) * 48); /* 6rem */ - --spacing-17: calc(var(--spacing) * 60); /* 7.5rem */ - --spacing-18: calc(var(--spacing) * 80); /* 10rem */ + --spacing-00: 0px; /* 0rem */ + --spacing-01: var(--ds-spacing, 0.125rem); /* 0.125rem */ + --spacing-02: calc(var(--ds-spacing, 0.125rem) * 2); /* 0.25rem */ + --spacing-03: calc(var(--ds-spacing, 0.125rem) * 3); /* 0.375rem */ + --spacing-04: calc(var(--ds-spacing, 0.125rem) * 4); /* 0.5rem */ + --spacing-05: calc(var(--ds-spacing, 0.125rem) * 6); /* 0.75rem */ + --spacing-06: calc(var(--ds-spacing, 0.125rem) * 8); /* 1rem */ + --spacing-07: calc(var(--ds-spacing, 0.125rem) * 10); /* 1.25rem */ + --spacing-08: calc(var(--ds-spacing, 0.125rem) * 12); /* 1.5rem */ + --spacing-09: calc(var(--ds-spacing, 0.125rem) * 16); /* 2rem */ + --spacing-10: calc(var(--ds-spacing, 0.125rem) * 20); /* 2.5rem */ + --spacing-11: calc(var(--ds-spacing, 0.125rem) * 24); /* 3rem */ + --spacing-12: calc(var(--ds-spacing, 0.125rem) * 28); /* 3.5rem */ + --spacing-13: calc(var(--ds-spacing, 0.125rem) * 32); /* 4rem */ + --spacing-14: calc(var(--ds-spacing, 0.125rem) * 36); /* 4.5rem */ + --spacing-15: calc(var(--ds-spacing, 0.125rem) * 40); /* 5rem */ + --spacing-16: calc(var(--ds-spacing, 0.125rem) * 48); /* 6rem */ + --spacing-17: calc(var(--ds-spacing, 0.125rem) * 60); /* 7.5rem */ + --spacing-18: calc(var(--ds-spacing, 0.125rem) * 80); /* 10rem */ }
4-4: Nit: Use unitless zero.Prefer
0over0px.- --spacing-00: 0px; /* 0rem */ + --spacing-00: 0; /* 0rem */
5-22: Optional: Generate tokens via SCSS to reduce drift.A tiny map + loop keeps scale definitions and comments in sync if the base changes.
$spacing-mults: ( ); :root { --ds-spacing: 0.125rem; --spacing: var(--ds-spacing); --spacing-00: 0; @each $k, $m in $spacing-mults { --spacing-#{$k}: calc(var(--ds-spacing, 0.125rem) * #{$m}); /* #{0.125rem * $m} */ } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/spacings/bun.lockis excluded by!**/*.lockpackages/spacings/dist/index.cssis excluded by!**/dist/**packages/spacings/dist/index.css.mapis excluded by!**/dist/**,!**/*.mappackages/spacings/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/spacings/package.json(1 hunks)packages/spacings/src/_variables.scss(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/spacings/package.json
🔇 Additional comments (2)
packages/spacings/src/_variables.scss (2)
2-22: Solid tokenization; scale is consistent and math checks out.Deriving the whole scale from a single base is a good move; comments match the computed rem values.
2-2: No repo-wide collisions for--spacingfound.
Searched tracked files (git ls-files → ripgrep) for--spacingand the declaration pattern--spacing\s*:; no matches.
Summary by CodeRabbit