docs(web): document color handling in graph component #1469
Conversation
Explain how the graph component consumes v4 theme color tokens: - Note that v4 defines CSS variables (e.g. `--border`, `--muted-foreground`, `--chart-1`) as full OKLCH color values in `globals-v4.css` - Clarify that we pass these as `var(--token)` directly to Recharts/SVG props - Make explicit that we do not wrap these tokens in `hsl()`/`oklch()` when using them in the graph This provides future readers with a clear reference on why colors are wired this way and avoids confusion when integrating theme tokens with Recharts.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The change is purely documentary and technically accurate, but the new comment could be slightly clearer about the relationship between defaultColors (static HSL fallbacks) and the separate v4 var(--token) color path. Clarifying this distinction would avoid potential confusion for future maintainers. No functional, performance, or architectural issues are introduced by this diff.
Additional notes (1)
- Readability |
cli/src/registry/graph/graph.tsx:161-167
The added comment is clear and accurately explains how v4 theme CSS variables are intended to be used with Recharts/SVG. One small potential source of confusion is that the documentedvar(--token)usage refers to theme tokens defined elsewhere, while thedefaultColorsarray immediately below still uses hard-codedhsl(...)strings. Future readers might misinterpret this as inconsistent behavior unless they notice that thesedefaultColorsare a fallback separate from the v4 token usage path.
Summary of changes
Summary of Changes
- Expanded the documentation comment above
defaultColorsincli/src/registry/graph/graph.tsx. - Clarified that v4 theme color CSS variables (e.g.,
--border,--muted-foreground,--chart-1) inglobals-v4.cssstore full OKLCH values. - Documented that these variables are passed directly as
var(--token)to Recharts/SVG props, without wrapping them inhsl()/oklch(). - Left the actual
defaultColorsarray and runtime behavior unchanged.
michaelmagan
requested review from
lachieh
and removed request for
CharlieHelps
There was a problem hiding this comment.
The change is documentation-only and technically accurate, but the new comment can be misread as describing defaultColors as v4 token-based colors rather than static HSL fallbacks. Clarifying the separation between v4 var(--token) usage and the defaultColors fallback array will improve maintainability and reduce confusion for future readers. No functional, performance, or architectural issues are introduced by this diff.
Additional notes (1)
- Readability |
cli/src/registry/graph/graph.tsx:158-166
The new comment accurately documents how v4 theme CSS variables are used in Recharts/SVG, but it can be read as if thesedefaultColorsthemselves are coming from v4 tokens, which they are not. That coupling between the comment and the concretehsl(...)fallbacks below is a bit confusing, especially for someone skimming quickly.
Explicitly calling out that:
- v4 themed colors use
var(--token)(full OKLCH values) in other parts of the graph component; and defaultColorsis a static HSL fallback array used when no theme tokens are provided,
would make the relationship clearer and avoid misinterpretation that these HSL values are v4 tokens or OKLCH-based.
This is a documentation-only concern, but tightening it now will save future maintainers time when tracing color behavior.
Summary of changes
Summary of Changes
- Expanded the documentation comment above
defaultColorsincli/src/registry/graph/graph.tsx. - Clarified that v4 theme color CSS variables (e.g.
--border,--muted-foreground,--chart-1) are full OKLCH values defined inglobals-v4.css. - Documented that these tokens are passed directly as
var(--token)to Recharts/SVG props, without wrapping them inhsl()/oklch(). - Left the
defaultColorsarray and all runtime behavior unchanged.
Resolves TAM-822
This PR adds a brief note in our graph/recharts component docs describing how we handle colors, including the expected CSS variable format and how those variables are applied within the SVG context.
Changes:
Notes: