Conversation
Consolidates duplicated logic across chart setup, layout plugins, physics controllers, and canvas renderers by extracting shared helpers (category extraction, label/id resolvers, filter/path utilities, forecast math, quantile, and symbol resolution). It also tightens typings in several registries/configs, removes dead code/tests, and unifies network symbol rendering into the shared symbol canvas renderer. Adds explicit SVG `fontSize` presentation-attribute fallbacks (while keeping CSS-var styling) for overlays/legend/title text so exported or sanitized SVGs without CSS engines still render legibly, and fixes canvas stroke handling so `stroke="none"` no longer paints unintended black outlines.
There was a problem hiding this comment.
Pull request overview
This PR refactors and consolidates shared rendering/configuration utilities across the stream pipelines (XY/ordinal/network/physics) and improves SVG export robustness by adding fontSize presentation-attribute fallbacks to key <text> elements, while also fixing canvas stroke handling so stroke="none" does not paint unintended black outlines.
Changes:
- Extracts/centralizes shared helpers (symbol/category resolution, accessor/id/label helpers, quantile/forecast math, registry typings) to reduce duplicated logic across chart families.
- Adds SVG
fontSizepresentation-attribute fallbacks (alongside existing CSS-var styling) for more reliable rendering in SVG consumers without CSS engines. - Unifies and tightens canvas rendering behavior (shared symbol canvas renderer, network paint helpers, gradient helper rename) and physics controller filtering utilities.
Reviewed changes
Copilot reviewed 56 out of 56 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/components/stream/xySceneBuilders/pointScene.ts | Uses shared makeSymbolResolver for deterministic symbol assignment. |
| src/components/stream/symbolPath.ts | Adds makeSymbolResolver helper shared across XY/ordinal symbol channels. |
| src/components/stream/SVGOverlay.tsx | Adds fontSize presentation-attribute fallbacks for axis ticks/labels and title. |
| src/components/stream/SVGOverlay.axisCustomization.test.tsx | Updates expectations to cover the new font-size attribute fallback. |
| src/components/stream/renderers/trapezoidCanvasRenderer.ts | Guards stroke="none" so canvas doesn’t paint a default-black outline. |
| src/components/stream/renderers/symbolCanvasRenderer.ts | Consolidates XY/ordinal + network symbol painting and adds stroke="none" guard. |
| src/components/stream/renderers/networkSymbolRenderer.ts | Removes legacy network symbol renderer in favor of shared implementation. |
| src/components/stream/renderers/networkRectRenderer.ts | Uses shared network fill/stroke helpers for consistent canvas behavior. |
| src/components/stream/renderers/networkCircleRenderer.ts | Uses shared network fill/stroke helpers for consistent canvas behavior. |
| src/components/stream/renderers/networkArcRenderer.ts | Uses shared network fill/stroke helpers for consistent canvas behavior. |
| src/components/stream/renderers/lineCanvasRenderer.ts | Updates to use the unified linear gradient helper name. |
| src/components/stream/renderers/canvasRenderHelpers.ts | Adds paintNetworkFill/paintNetworkStroke helpers; documents gradient reuse. |
| src/components/stream/renderers/canvasRenderHelpers.test.ts | Removes old gradient helper tests; retains gradient + fill/curve coverage. |
| src/components/stream/renderers/barCanvasRenderer.ts | Refactors bar fill/stroke setup into reusable local helpers. |
| src/components/stream/renderers/areaCanvasRenderer.ts | Updates to use the unified linear gradient helper name. |
| src/components/stream/physics/StreamPhysicsFrame.tsx | Uses plot dimensions for tooltip container sizing; ref housekeeping. |
| src/components/stream/physics/ServiceOperationsControllers.ts | Reuses shared physics path/filter helpers from PhysicsKernel. |
| src/components/stream/physics/PhysicsSVGOverlay.tsx | Adds chart title fontSize presentation-attribute fallback. |
| src/components/stream/physics/PhysicsKernel.ts | Exports shared valueAtPath/matchesPhysicsBodyFilter; refactors friction/restitution pairing. |
| src/components/stream/physics/physicsBodyCanvas.ts | Guards stroke="none" so physics bodies don’t get unintended outlines. |
| src/components/stream/physics/CapacityQueueController.ts | Reuses shared physics path/filter helpers from PhysicsKernel. |
| src/components/stream/OrdinalSVGOverlay.tsx | Adds fontSize presentation-attribute fallbacks for ordinal axis text and title. |
| src/components/stream/ordinalSceneBuilders/sceneBuilderMap.ts | Tightens ordinal scene builder registry typing and documents “custom” exclusion. |
| src/components/stream/ordinalSceneBuilders/pointScene.ts | Uses shared makeSymbolResolver to match XY symbol assignment. |
| src/components/stream/NetworkSVGOverlay.tsx | Adds chart title fontSize presentation-attribute fallback. |
| src/components/stream/networkFramePaint.ts | Switches network symbol rendering to the shared symbol canvas renderer. |
| src/components/stream/layouts/sankeyLayoutPlugin.ts | Centralizes node label + nodeRef id resolution via shared accessor utils. |
| src/components/stream/layouts/index.ts | Tightens networkLayoutRegistry typing to NetworkChartType. |
| src/components/stream/layouts/forceLayoutPlugin.ts | Centralizes node label + nodeRef id resolution via shared accessor utils. |
| src/components/stream/layouts/chordLayoutPlugin.ts | Centralizes node label + nodeRef id resolution via shared accessor utils. |
| src/components/stream/GeoSVGOverlay.tsx | Adds chart title fontSize presentation-attribute fallback. |
| src/components/stream/accessorUtils.ts | Adds shared resolveLabelFn and resolveNodeRefId utilities for layout plugins. |
| src/components/store/useSelection.ts | Reuses shared hasOwnEnumerableKey from the store utilities module. |
| src/components/store/createStore.tsx | Exports hasOwnEnumerableKey for reuse; adds doc comment. |
| src/components/Legend.tsx | Adds fontSize presentation-attribute fallbacks for legend text elements. |
| src/components/data/numericFieldProfiler.ts | Exports quantile for reuse across data auditing utilities. |
| src/components/data/auditData.ts | Reuses exported quantile rather than duplicating the implementation. |
| src/components/charts/xy/BumpChart.tsx | Adds fontSize presentation-attribute fallback and aligns var() fallback size. |
| src/components/charts/value/thresholdSparkline.ts | Simplifies sparkline x-step math based on guaranteed length ≥ 2 path. |
| src/components/charts/shared/withChartWrapper.tsx | Removes deprecated props + dead warnDataShape utility. |
| src/components/charts/shared/withChartWrapper.test.tsx | Removes tests for deleted warnDataShape. |
| src/components/charts/shared/validateProps.ts | Consolidates imports/exports; aligns prop/data shape types; clarifies comments. |
| src/components/charts/shared/useOrdinalBrush.ts | Memoizes brush config to stabilize callback identities. |
| src/components/charts/shared/useNetworkChartSetup.ts | Uses distinctCategories; preserves array colorScheme identity when provided. |
| src/components/charts/shared/useChartSetup.ts | Uses shared distinctCategories; minor typing cleanup. |
| src/components/charts/shared/useChartLegend.ts | Introduces distinctCategories helper and fixes string coercion for legend predicates. |
| src/components/charts/shared/styleRules.ts | Clarifies public API intent/comments for ordinal style-rule helper. |
| src/components/charts/shared/statisticalOverlays.ts | Reuses shared forecast regression helpers to avoid duplicated math. |
| src/components/charts/shared/leastSquaresRegression.ts | Adds forecast-grade linear fit + interval stats + confidence z-score helpers. |
| src/components/charts/shared/hooks.ts | Re-exports distinctCategories via the shared hooks surface. |
| src/components/charts/shared/diagnoseConfig.ts | Derives known color scheme set from COLOR_SCHEMES to avoid drift. |
| src/components/charts/shared/chartFamilySets.ts | Adds ORDINAL_CHARTS set; defines point radius prop mapping. |
| src/components/charts/shared/auditMobileVisualization.ts | Reuses shared family sets + point-radius prop mapping to reduce duplication. |
| src/components/charts/shared/annotationRules.tsx | Reuses shared forecast helpers; tightens curve factory typing. |
| src/components/charts/realtime/RealtimeHistogram.tsx | Removes unused value accessor local. |
| src/components/charts/ordinal/GaugeChart.tsx | Adds fontSize presentation-attribute fallback for gauge label text. |
Comments suppressed due to low confidence (1)
src/components/stream/renderers/canvasRenderHelpers.test.ts:122
- Consider adding tests for the new
paintNetworkFill/paintNetworkStrokehelpers (especially thestroke === "none"guard, which prevents unintended default-black outlines on canvas, and the fillOpacity×opacity alpha behavior). This file already has good coverage for other helpers, so extending it keeps the suite cohesive.
Add focused coverage for network canvas fill/stroke helpers, including the `stroke="none"` canvas trap and opacity/width handling. Remove outdated SafeRender tests that expected diagnostic-hint behavior after that integration was dropped, and clean up minor BumpChart formatting.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/components/charts/shared/useChartLegend.ts:277
- The isolate predicate has the same null/undefined→"" coercion issue as highlight mode, which can cause bodies with missing colorBy values to be treated as part of the empty-string category.
const raw = colorField ? d[colorField] : typeof colorBy === "function" ? colorBy(d) : null
const val = typeof raw === "string" ? raw : String(raw ?? "")
return isolatedCategories.has(val)
src/components/charts/shared/useChartLegend.ts:266
- Legend interaction now coerces null/undefined colorBy values to an empty string (via
String(raw ?? "")), which can incorrectly match the real "" category if your data contains empty-string categories. Null/undefined should never match any highlighted/isolated category.
This issue also appears on line 275 of the same file.
const raw = colorField ? d[colorField] : typeof colorBy === "function" ? colorBy(d) : null
// Legend labels are String(v)-coerced (useChartSetup's category
// extraction), so a non-string colorBy field (number, boolean)
// must be coerced the same way or it never matches.
const val = typeof raw === "string" ? raw : String(raw ?? "")
src/components/stream/ordinalSceneBuilders/sceneBuilderMap.ts:23
- Typing
ORDINAL_SCENE_BUILDERSasPartial<Record<OrdinalChartType, ...>>weakens coverage for all other chart types (not just"custom"). You can exclude"custom"from the key union while still requiring entries for every other ordinal chart type.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
src/components/stream/renderers/canvasRenderHelpers.ts:172
paintNetworkStrokesetsctx.globalAlpha = style.opacity ?? 1, which can override existing canvas alpha (e.g. staleness dim) and can also unintentionally reset an alpha that the caller already composed. It should preserve the current alpha and restore it after stroking.
src/components/stream/accessorUtils.ts:85resolveLabelFnclaims to fall back tod.idonly when the label field is missing/empty, but the implementation usesd[nodeLabel] || d.id, which also treats valid falsy values (e.g. 0, false) as “empty”. This can produce incorrect labels and contradicts the docstring.
src/components/stream/renderers/networkRectRenderer.ts:25- This renderer sets
ctx.globalAlpha = r.style.opacity, which overwrites any existing chart-wide alpha (e.g. the staleness dim applied innetworkFramePaint.ts). Opacity should be multiplied into the existing alpha so global dimming continues to work when per-node opacity is set.
src/components/stream/renderers/canvasRenderHelpers.ts:154 paintNetworkFilloverwritesctx.globalAlphausing(style.opacity ?? 1) * style.fillOpacity, which drops any existing canvas alpha (e.g. the chart-wide staleness dim set innetworkFramePaint.ts). It should applyfillOpacityrelative to the currentglobalAlphaand restore the previous alpha after painting.
This issue also appears on line 168 of the same file.
src/components/stream/renderers/networkCircleRenderer.ts:28
- This renderer overwrites
ctx.globalAlphawithc.style.opacity, which drops any pre-existing chart-wide alpha (notably the staleness dim set innetworkFramePaint.ts). Multiply the node opacity into the existing alpha so the dimming effect remains consistent.
src/components/stream/renderers/networkArcRenderer.ts:28 - This renderer overwrites
ctx.globalAlphawitha.style.opacity, which drops any pre-existing chart-wide alpha (e.g. staleness dim fromnetworkFramePaint.ts). Multiply node opacity into the existing alpha to preserve chart-wide dimming.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 59 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (8)
src/components/stream/renderers/canvasRenderHelpers.test.ts:175
- paintNetworkStroke no longer needs to set ctx.globalAlpha (it should preserve any chart-wide dim already in the context). These tests should set an initial globalAlpha and assert that the helper resolves strokeStyle/lineWidth while leaving globalAlpha unchanged.
src/components/stream/renderers/canvasRenderHelpers.test.ts:186 - With paintNetworkStroke no longer setting ctx.globalAlpha, this test should stop asserting it defaults to 1 (that’s the caller’s responsibility) and instead verify that lineWidth defaults to 1 while globalAlpha is preserved.
src/components/stream/renderers/canvasRenderHelpers.ts:154 - paintNetworkFill currently overwrites ctx.globalAlpha using only style.opacity/fillOpacity, which drops any existing chart-wide alpha (e.g. networkFramePaint's staleness dim) and also treats fill="none" as a real fill (falling back to the default). It should (1) short-circuit on fill === "none" and (2) multiply the existing ctx.globalAlpha by fillOpacity for the fill pass, restoring it afterward so stroke/pulse use the node-level alpha.
src/components/stream/renderers/canvasRenderHelpers.ts:172 - paintNetworkStroke sets ctx.globalAlpha = style.opacity ?? 1, which clobbers any existing chart-wide alpha (e.g. staleness dim) and also overrides any prior per-node alpha the caller may have established. It should leave ctx.globalAlpha alone and only resolve strokeStyle/lineWidth (opacity should be applied by the caller via ctx.globalAlpha multiplication).
src/components/stream/renderers/networkRectRenderer.ts:22 - This renderer overwrites ctx.globalAlpha with r.style.opacity, which drops any existing chart-wide alpha (e.g. the staleness dim applied in networkFramePaint). Use multiplication so per-node opacity composes with chart-wide dim instead of replacing it.
src/components/stream/renderers/networkCircleRenderer.ts:28 - Because networkFramePaint applies staleness dim via ctx.globalAlpha before calling node renderers, the per-node opacity handling needs to compose with the existing alpha. Right now the renderer sets ctx.globalAlpha = c.style.opacity (replacing the current alpha), so stale charts won’t dim nodes with an explicit opacity. Use multiplication so opacity composes with chart-wide dim.
src/components/stream/renderers/networkArcRenderer.ts:28 - Because networkFramePaint applies staleness dim via ctx.globalAlpha before calling node renderers, the per-node opacity handling needs to compose with the existing alpha. Right now the renderer sets ctx.globalAlpha = a.style.opacity (replacing the current alpha), so stale charts won’t dim arc nodes with an explicit opacity. Use multiplication so opacity composes with chart-wide dim.
src/components/stream/renderers/canvasRenderHelpers.test.ts:144 - These tests currently assume paintNetworkFill applies style.opacity itself and leaves ctx.globalAlpha modified after the call. With the renderer now composing opacity into ctx.globalAlpha (to preserve chart-wide dim) and paintNetworkFill only applying fillOpacity temporarily, the test should assert the alpha during paint() and that globalAlpha is restored afterward.
This issue also appears in the following locations of the same file:
- line 171
- line 181
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 59 out of 71 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/components/charts/shared/useChartLegend.ts:277
- Same null/undefined-to-empty-string coercion issue as the highlight branch: String(raw ?? "") can cause null/undefined categories to match an intentionally-empty-string category when isolating. This can make isolate mode include rows that had no category value.
Short-circuit raw==null before coercion; otherwise String(raw) for non-strings so numeric/boolean categories still match the legend labels.
const raw = colorField ? d[colorField] : typeof colorBy === "function" ? colorBy(d) : null
const val = typeof raw === "string" ? raw : String(raw ?? "")
return isolatedCategories.has(val)
src/components/charts/shared/useChartLegend.ts:266
- Legend interaction coercion can accidentally treat null/undefined colorBy values as the empty-string category. If any datum has colorBy=="" (so highlightedCategory/isolation set contains ""), then rows with null/undefined will currently coerce to "" via String(raw ?? "") and incorrectly match/participate in highlight/isolate.
Fix by short-circuiting null/undefined before string coercion, while still string-coercing non-string values (number/boolean) to match distinctCategories' String(v) behavior.
This issue also appears on line 275 of the same file.
const raw = colorField ? d[colorField] : typeof colorBy === "function" ? colorBy(d) : null
// Legend labels are String(v)-coerced (useChartSetup's category
// extraction), so a non-string colorBy field (number, boolean)
// must be coerced the same way or it never matches.
const val = typeof raw === "string" ? raw : String(raw ?? "")
return val === highlightedCategory
Consolidates duplicated logic across chart setup, layout plugins, physics controllers, and canvas renderers by extracting shared helpers (category extraction, label/id resolvers, filter/path utilities, forecast math, quantile, and symbol resolution). It also tightens typings in several registries/configs, removes dead code/tests, and unifies network symbol rendering into the shared symbol canvas renderer.
Adds explicit SVG
fontSizepresentation-attribute fallbacks (while keeping CSS-var styling) for overlays/legend/title text so exported or sanitized SVGs without CSS engines still render legibly, and fixes canvas stroke handling sostroke="none"no longer paints unintended black outlines.This pull request introduces several improvements and refactorings across the charting components, focusing on consistency, code reuse, and maintainability. The most notable changes include extracting common chart family sets and point radius props, centralizing color scheme definitions, refactoring forecast and regression logic for reusability, and enhancing legend and label rendering. Additionally, several small cleanups and API documentation improvements are included.
Refactoring and Code Reuse:
XY_WITH_AXES_CHARTS,ORDINAL_CHARTS) and point radius property mappings into a shared module (chartFamilySets.ts), and updated all references to use these shared constants. This reduces duplication and makes chart categorization more maintainable. [1] [2] [3]COLOR_SCHEMESdirectly, ensuring consistency between color utilities and configuration validation.Forecast and Regression Logic Refactoring:
fitLinearForForecast,forecastIntervalStats,confidenceZScore) inleastSquaresRegression.ts. Updated both annotation rules and statistical overlays to use these helpers, eliminating duplicated math and ensuring consistent forecast calculations. [1] [2] [3] [4]Legend and Label Rendering Consistency:
fontSizeattribute in various SVG text elements across legend and chart components, improving visual consistency and accessibility. [1] [2] [3] [4] [5] [6]API and Documentation Enhancements:
distinctCategoriesutility to consistently extract unique category values for legends and color scales. Updated chart legend hooks to use this utility, improving clarity and reliability. [1] [2] [3]Other Cleanups:
These changes collectively improve maintainability, reduce duplication, and enhance the consistency of chart rendering and configuration across the codebase.