Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new Statistic React component and MDX docs to the Eclipse design system, updates the molecules catalog, broadens Badge label typing, and introduces Mona Sans font assets with CSS and a package export path. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
🍈 Lychee Link Check Report3664 links: ✅ All links are working!Full Statistics Table
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/eclipse/src/styles/globals.css (1)
74-76:--font-mona-sansis misplaced under the Blur section — move it to Typography.The variable belongs logically with
--font-family-displayand its siblings. As written, it sits directly under--blur-background-highwith no section comment separating it, making the@themeblock harder to scan.♻️ Proposed move
/* Blur */ --blur-background-low: 16px; --blur-background: 24px; --blur-background-high: 40px; - --font-mona-sans: - "Mona Sans", -apple-system, "BlinkMacSystemFont", "Segoe UI", "Roboto", - "Helvetica Neue", "Arial", sans-serif; /* Typography - Font Families */ + --font-mona-sans: + "Mona Sans", -apple-system, "BlinkMacSystemFont", "Segoe UI", "Roboto", + "Helvetica Neue", "Arial", sans-serif; --font-family-display: var(--font-mona-sans), Inter, sans-serif;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eclipse/src/styles/globals.css` around lines 74 - 76, The CSS custom property --font-mona-sans is placed under the Blur section (near --blur-background-high) but logically belongs with the Typography variables; move the --font-mona-sans declaration out from beneath --blur-background-high into the Typography group alongside --font-family-display and related font variables so the `@theme` block remains organized and easy to scan. Ensure you preserve the exact variable name and value and keep any surrounding indentation and comments consistent with the Typography section.packages/eclipse/src/styles/fonts.css (1)
1-41: Consider adding WOFF2 format for better load performance.All declarations use only TTF, which is 2–4× larger than WOFF2. WOFF2 is supported across all browsers the design system targets (Chrome 111+, Safari 16.4+, Firefox 128+ per Tailwind v4's requirements). Adding WOFF2 with TTF as a fallback is the modern standard.
🚀 Proposed format additions (pattern for each weight)
`@font-face` { font-family: "Mona Sans"; - src: url("../static/fonts/MonaSans-Regular.ttf") format("truetype"); + src: url("../static/fonts/MonaSans-Regular.woff2") format("woff2"), + url("../static/fonts/MonaSans-Regular.ttf") format("truetype"); font-weight: 400; font-style: normal; font-display: swap; }Apply the same pattern to each of the remaining four weights.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eclipse/src/styles/fonts.css` around lines 1 - 41, The font-face blocks for font-family "Mona Sans" currently only load TTFs which are large; add WOFF2 sources before the existing TTF sources in each `@font-face` so browsers load the smaller WOFF2 when supported and fall back to TTF otherwise, keeping the existing font-weight (400, 500, 600, 700, 800), font-style and font-display values; update each `@font-face` block (the ones that reference MonaSans-Regular.ttf, MonaSans-Medium.ttf, MonaSans-SemiBold.ttf, MonaSans-Bold.ttf, MonaSans-ExtraBold.ttf) to include a src entry for the corresponding .woff2 file (same basename) followed by the existing .ttf entry.packages/eclipse/src/components/statistic.tsx (1)
11-17: Prefer a named export to stay consistent with the rest of the component library.
Badge,badgeVariants, and every other component inpackages/eclipse/src/componentsuse named exports. A default export breaks the consistent pattern and makes tree-shaking and re-exporting slightly more cumbersome.♻️ Proposed change
-export default function Statistic({ +export function Statistic({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eclipse/src/components/statistic.tsx` around lines 11 - 17, The component is currently exported as a default export; change it to a named export to match the library pattern by replacing "export default function Statistic(...)" with "export function Statistic(...)" (keep the same Statistic function signature and props), and then update any local re-exports/consumers to import { Statistic } instead of default — ensure the named export is used consistently with other components like Badge and badgeVariants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/eclipse/content/design-system/molecules/statistic.mdx`:
- Around line 187-252: The Typography & Spacing and Design Tokens sections in
statistic.mdx are inconsistent with the shipped Statistic component; update the
MDX to reflect the actual classes used in the implementation (check the
Statistic React component in statistic.tsx) — change Title to text-2xs with
tracking-[1.1px] and the appropriate Eclipse token (e.g.,
text-foreground-neutral-weak), Value to text-2xl font-bold with its token,
Measure to text-sm with its token, remove any px-6 py-5 and border-2
border-dashed rounded-lg entries (document "no padding" and "no border"), and
replace the doc color names (text-gray-*) with the actual token names used by
the component so consumers can rely on correct tokens and sizes.
- Around line 133-138: The MDX checklist incorrectly claims a "Dashed border
design" feature that doesn't exist; remove the "✅ Dashed border design for
visual emphasis" line from
apps/eclipse/content/design-system/molecules/statistic.mdx, or if the dashed
border is intended, add appropriate border classes (e.g., border, border-dashed,
rounded-*) to the Statistic component in statistic.tsx (the Statistic component)
and update its docs to reflect the new prop/styling—pick one action and apply it
so the docs and the Statistic implementation stay in sync.
- Around line 205-232: Update the three section headings that are compound
adjectives by adding hyphens: change the strings "Two Column Grid", "Three
Column Grid", and "Four Column Grid" to "Two-Column Grid", "Three-Column Grid",
and "Four-Column Grid" respectively in the statistic.mdx file so static analysis
no longer flags them.
In `@packages/eclipse/src/components/badge.tsx`:
- Around line 38-42: The prop type for label is currently declared as any;
change it to React.ReactNode in the badge props interface so the label can
accept text, elements, or fragments safely (replace the type for the label
property from any to React.ReactNode). Update the JSDoc above label (and any
description that says "label text") to reflect that it can be any renderable
React node. Ensure you import React types if needed (e.g., add React import or
use the global JSX namespace) and keep the prop name label in the Badge
component signature unchanged.
In `@packages/eclipse/src/components/statistic.tsx`:
- Around line 1-9: The file uses React.ReactNode in the StatisticProps interface
but never imports React, causing a TypeScript error; add an import for React (or
import the React type only) at the top of the component file so the React
namespace is in scope; update the imports in
packages/eclipse/src/components/statistic.tsx so the StatisticProps interface
(which references React.ReactNode) compiles correctly (mirror how other
components like badge.tsx import React).
In `@packages/eclipse/src/styles/globals.css`:
- Around line 74-76: The CSS custom property --font-mona-sans contains unquoted
font family identifiers that fail Stylelint (BlinkMacSystemFont, Roboto, Arial);
update the value of --font-mona-sans so those font names are quoted (e.g.,
"BlinkMacSystemFont", "Roboto", "Arial") to match the existing quoted entries
("Segoe UI", "Helvetica Neue") and satisfy the value-keyword-case rule in the
styles/globals.css declaration.
---
Nitpick comments:
In `@packages/eclipse/src/components/statistic.tsx`:
- Around line 11-17: The component is currently exported as a default export;
change it to a named export to match the library pattern by replacing "export
default function Statistic(...)" with "export function Statistic(...)" (keep the
same Statistic function signature and props), and then update any local
re-exports/consumers to import { Statistic } instead of default — ensure the
named export is used consistently with other components like Badge and
badgeVariants.
In `@packages/eclipse/src/styles/fonts.css`:
- Around line 1-41: The font-face blocks for font-family "Mona Sans" currently
only load TTFs which are large; add WOFF2 sources before the existing TTF
sources in each `@font-face` so browsers load the smaller WOFF2 when supported and
fall back to TTF otherwise, keeping the existing font-weight (400, 500, 600,
700, 800), font-style and font-display values; update each `@font-face` block (the
ones that reference MonaSans-Regular.ttf, MonaSans-Medium.ttf,
MonaSans-SemiBold.ttf, MonaSans-Bold.ttf, MonaSans-ExtraBold.ttf) to include a
src entry for the corresponding .woff2 file (same basename) followed by the
existing .ttf entry.
In `@packages/eclipse/src/styles/globals.css`:
- Around line 74-76: The CSS custom property --font-mona-sans is placed under
the Blur section (near --blur-background-high) but logically belongs with the
Typography variables; move the --font-mona-sans declaration out from beneath
--blur-background-high into the Typography group alongside --font-family-display
and related font variables so the `@theme` block remains organized and easy to
scan. Ensure you preserve the exact variable name and value and keep any
surrounding indentation and comments consistent with the Typography section.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/eclipse/src/static/fonts/MonaSans-Bold.ttfis excluded by!**/*.ttfpackages/eclipse/src/static/fonts/MonaSans-ExtraBold.ttfis excluded by!**/*.ttfpackages/eclipse/src/static/fonts/MonaSans-Medium.ttfis excluded by!**/*.ttfpackages/eclipse/src/static/fonts/MonaSans-Regular.ttfis excluded by!**/*.ttfpackages/eclipse/src/static/fonts/MonaSans-SemiBold.ttfis excluded by!**/*.ttf
📒 Files selected for processing (7)
apps/eclipse/content/design-system/molecules/meta.jsonapps/eclipse/content/design-system/molecules/statistic.mdxpackages/eclipse/package.jsonpackages/eclipse/src/components/badge.tsxpackages/eclipse/src/components/statistic.tsxpackages/eclipse/src/styles/fonts.csspackages/eclipse/src/styles/globals.css
carlagn
requested review from
AmanVarshney01,
ArthurGamby,
aidankmcalister,
ankur-arch,
mhartington and
mhessdev
aidankmcalister
approved these changes
Feb 24, 2026
Add statistic component Add doc for statistic component
carlagn
force-pushed
the
feat/DR-6919-statistic
branch
from
bd402dc to
b57e124
Compare
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add statistic component
Add doc for statistic component
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores