Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR expands Semiotic’s theming layer to include semantic “status role” colors (success/danger/warning/error/info plus secondary/surface) and wires them through both CSS custom properties and the streaming pipeline, while also fixing a previously-dead barStyle plumbing path so RealtimeHistogram stroke/strokeWidth/fill/gap actually affect rendering. It adds unit + Playwright coverage and introduces new theming documentation (Semantic Colors) along with doc route/link updates.
Changes:
- Add semantic role colors to
SemioticTheme, presets, CSS var emission (--semiotic-{role}), and token/CSS serializers. - Fix
barStyleplumbing from RealtimeHistogram → StreamXYFrame → PipelineStore → bar scene builder, including theme-aware fill/stroke precedence. - Add tests (Vitest + Playwright) and documentation/navigation updates for semantic colors + RealtimeHistogram route rename.
Reviewed changes
Copilot reviewed 36 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/stream/xySceneBuilders/types.ts | Adds barStyle + themeSemantic to XY scene config. |
| src/components/stream/xySceneBuilders/barScene.ts | Resolves bar fill/stroke/gap from barStyle + themeSemantic.primary. |
| src/components/stream/xySceneBuilders/barScene.test.ts | Adds unit tests for barStyle/themeSemantic precedence and gap/stroke behavior. |
| src/components/stream/StreamXYFrame.tsx | Threads barStyle and themeSemantic into pipelineConfig. |
| src/components/stream/StreamXYFrame.test.tsx | Adds regression test ensuring *Style props reach PipelineStore config. |
| src/components/stream/PipelineStore.ts | Extends PipelineConfig with themeSemantic and barStyle typing/docs. |
| src/components/store/ThemeStore.ts | Extends SemioticTheme.colors and populates built-in presets with semantic roles. |
| src/components/semiotic-themes.ts | Populates branded presets with semantic roles; emits semantic CSS vars + tokens. |
| src/components/ThemeProvider.tsx | Emits semantic role CSS variables inline on provider wrapper. |
| package.json | Adds new histogram-stroke example page to serve-examples scripts. |
| integration-tests/histogram-theme-stroke.spec.ts-snapshots/histogram-stroke-scoped-chromium-darwin.png | New Playwright baseline (scoped override case). |
| integration-tests/histogram-theme-stroke.spec.ts-snapshots/histogram-stroke-light-chromium-darwin.png | New Playwright baseline (light theme). |
| integration-tests/histogram-theme-stroke.spec.ts-snapshots/histogram-stroke-dark-chromium-darwin.png | New Playwright baseline (dark theme). |
| integration-tests/histogram-theme-stroke.spec.ts | New Playwright regression test for histogram stroke + scoped CSS var override. |
| integration-tests/histogram-theme-stroke-examples/index.js | Example harness page exercising histogram stroke + scoped override. |
| integration-tests/histogram-theme-stroke-examples/index.html | HTML entry for the new integration-test example. |
| docs/src/pages/theming/ThemeExplorerPage.js | Updates prev/next chain to include Semantic Colors page. |
| docs/src/pages/theming/SemanticColorsPage.js | New docs page describing semantic roles, swatches, and override patterns. |
| docs/src/pages/features/ThemingPage.js | Links next page from Theme Provider → Semantic Colors. |
| docs/src/pages/charts/RealtimeWaterfallChartPage.js | Updates RealtimeHistogram related links to new slug. |
| docs/src/pages/charts/RealtimeSwarmChartPage.js | Updates prevPage/related links to new RealtimeHistogram slug. |
| docs/src/pages/charts/RealtimeLineChartPage.js | Updates nextPage/related links to new RealtimeHistogram slug. |
| docs/src/pages/charts/RealtimeHistogramPage.js | Updates breadcrumb slug to /charts/realtime-histogram. |
| docs/src/pages/charts/RealtimeHeatmapPage.js | Updates related link + inline docs link to new slug. |
| docs/src/pages/api/ChartsApiPage.js | Updates RealtimeHistogram API slug to realtime-histogram. |
| docs/src/pages/GettingStartedPage.js | Updates RealtimeHistogram link to new slug. |
| docs/src/components/navData.js | Updates nav path for RealtimeHistogram and adds Semantic Colors entry. |
| docs/src/Landing.js | Updates gallery item path for RealtimeHistogram. |
| docs/src/App.js | Adds Semantic Colors route and renames RealtimeHistogram route path. |
| docs/public/llms.txt | Updates RealtimeHistogram doc link to new slug. |
| docs/public/llms-full.txt | Updates theming CSS-var list and adds semantic roles/cascade guidance. |
| ai/system-prompt.md | Syncs AI prompt with semantic role + scoped override guidance. |
| OUTSTANDING_WORK.md | Documents planned Primitive Theming phases and risks; references this plumbing. |
| CLAUDE.md | Updates AI-facing theming section with semantic roles + scoped override guidance. |
| CHANGELOG.md | Updates docs link path reference from /charts/realtime-bar-chart → /charts/realtime-histogram. |
| .windsurfrules | Syncs theming CSS var list + semantic role guidance. |
| .github/copilot-instructions.md | Syncs theming CSS var list + semantic role guidance. |
| .cursorrules | Syncs theming CSS var list + semantic role guidance. |
| .clinerules | Syncs theming CSS var list + semantic role guidance. |
Comment on lines
+43
to
+71
| function Swatch({ role, label }) { | ||
| const cssVar = `var(--semiotic-${role})` | ||
| return ( | ||
| <div | ||
| style={{ | ||
| display: "flex", | ||
| alignItems: "center", | ||
| gap: 12, | ||
| padding: "8px 12px", | ||
| border: "1px solid var(--semiotic-border)", | ||
| borderRadius: 6, | ||
| minWidth: 180, | ||
| }} | ||
| > | ||
| <div | ||
| style={{ | ||
| width: 32, | ||
| height: 32, | ||
| background: cssVar, | ||
| border: "1px solid var(--semiotic-border)", | ||
| borderRadius: 4, | ||
| flexShrink: 0, | ||
| }} | ||
| /> | ||
| <div style={{ fontSize: 13, display: "flex", flexDirection: "column" }}> | ||
| <strong>{label}</strong> | ||
| <code style={{ fontSize: 11, opacity: 0.7 }}>--semiotic-{role}</code> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Swatch/RoleGrid (and the example charts below) rely on CSS vars like --semiotic-primary, --semiotic-danger, etc., but the docs site default CSS only defines --semiotic-bg/text/text-secondary/border/grid (see docs/src/index.css). Without wrapping this page (or at least the demo sections) in ThemeProvider, most swatches will render with missing/incorrect colors. Consider wrapping the page content in a ThemeProvider (tied to the docs light/dark toggle) or adding explicit fallbacks/definitions for the new --semiotic-* vars in the docs CSS so the live examples are deterministic.
| ]} | ||
| prevPage={{ title: "Multi-Axis Line Chart", path: "/charts/multi-axis-line-chart" }} | ||
| nextPage={{ title: "Realtime Bar Chart", path: "/charts/realtime-bar-chart" }} | ||
| nextPage={{ title: "Realtime Bar Chart", path: "/charts/realtime-histogram" }} |
There was a problem hiding this comment.
The page navigation label still says "Realtime Bar Chart" but the linked route is now /charts/realtime-histogram. Updating the title to "Realtime Histogram" would keep the naming consistent with the component name and sidebar/nav entries.
Suggested change
| nextPage={{ title: "Realtime Bar Chart", path: "/charts/realtime-histogram" }} | |
| nextPage={{ title: "Realtime Histogram", path: "/charts/realtime-histogram" }} |
| { label: "RealtimeSwarmChart", path: "/charts/realtime-swarm-chart" }, | ||
| ]} | ||
| prevPage={{ title: "Realtime Bar Chart", path: "/charts/realtime-bar-chart" }} | ||
| prevPage={{ title: "Realtime Bar Chart", path: "/charts/realtime-histogram" }} |
There was a problem hiding this comment.
The previous-page navigation still labels the link as "Realtime Bar Chart" while the route is now /charts/realtime-histogram. Renaming the title to "Realtime Histogram" would avoid confusion and align with the updated slug elsewhere.
Suggested change
| prevPage={{ title: "Realtime Bar Chart", path: "/charts/realtime-histogram" }} | |
| prevPage={{ title: "Realtime Histogram", path: "/charts/realtime-histogram" }} |
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
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.
Theme layer extended:
per-subtree — confirmed visually in the playwright scoped snapshot.
Dead prop fixed:
Tests:
config — catches the next drop.
strokes actually paint (>100 gray pixels per frame). All 4 pass on chromium.
Docs:
role vocabulary with live swatches, using roles on charts, CSS cascade override with side-by-side live example, nested ThemeProvider for
scale overrides, status-semantics worked examples, full CSS-var reference table, "how it works under the hood" pipeline explanation.
Prev/next chain updated on adjacent pages.
AI surface synced:
.windsurfrules, .github/copilot-instructions.md, docs/public/llms-full.txt.