Skip to content

Refactor render helpers and SVG font fallbacks - #1148

Merged
emeeks merged 6 commits into
mainfrom
cleanup-5
Jul 29, 2026
Merged

Refactor render helpers and SVG font fallbacks#1148
emeeks merged 6 commits into
mainfrom
cleanup-5

Conversation

@emeeks

@emeeks emeeks commented Jul 28, 2026

Copy link
Copy Markdown
Member

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.

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:

  • Extracted chart family sets (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]
  • Centralized known color scheme definitions by referencing COLOR_SCHEMES directly, ensuring consistency between color utilities and configuration validation.

Forecast and Regression Logic Refactoring:

  • Moved linear fit, prediction interval statistics, and confidence z-score logic into reusable functions (fitLinearForForecast, forecastIntervalStats, confidenceZScore) in leastSquaresRegression.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:

  • Standardized legend and label font sizes by explicitly setting the fontSize attribute 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:

  • Added and documented a distinctCategories utility 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]
  • Improved comments and API documentation for style rule utilities, clarifying intended usage and public API surface.

Other Cleanups:

  • Minor code cleanups, such as removing unused variables and updating type annotations for better type safety and clarity. [1] [2] [3] [4]

These changes collectively improve maintainability, reduce duplication, and enhance the consistency of chart rendering and configuration across the codebase.

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 fontSize presentation-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/paintNetworkStroke helpers (especially the stroke === "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.

Comment thread src/components/stream/renderers/canvasRenderHelpers.test.ts
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.
Copilot AI review requested due to automatic review settings July 29, 2026 00:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_BUILDERS as Partial<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.

Copilot AI review requested due to automatic review settings July 29, 2026 00:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • paintNetworkStroke sets ctx.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:85
  • resolveLabelFn claims to fall back to d.id only when the label field is missing/empty, but the implementation uses d[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 in networkFramePaint.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
  • paintNetworkFill overwrites ctx.globalAlpha using (style.opacity ?? 1) * style.fillOpacity, which drops any existing canvas alpha (e.g. the chart-wide staleness dim set in networkFramePaint.ts). It should apply fillOpacity relative to the current globalAlpha and 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.globalAlpha with c.style.opacity, which drops any pre-existing chart-wide alpha (notably the staleness dim set in networkFramePaint.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.globalAlpha with a.style.opacity, which drops any pre-existing chart-wide alpha (e.g. staleness dim from networkFramePaint.ts). Multiply node opacity into the existing alpha to preserve chart-wide dimming.

Copilot AI review requested due to automatic review settings July 29, 2026 01:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 59 out of 59 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 29, 2026 01:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot AI review requested due to automatic review settings July 29, 2026 02:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@emeeks
emeeks merged commit 8dd9582 into main Jul 29, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants