Fix tooltip and SSR rendering parity gaps - #1149
Merged
Merged
Conversation
Restores multi-series tooltip context for custom tooltip functions by preserving hover-root fields (`allSeries`/`xValue`) after datum unwrapping, with new regression coverage for `LineChart` multi-tooltip behavior (including `lineDataAccessor`).
Improves rendering parity and axis customization by adding per-axis grid opt-out (`axis.grid`) to SVG underlay/overlay and static SSR grid rendering, adding `adaptiveTimeTicks(..., { utc: false })` while keeping UTC as the default, and propagating `fillOpacity`/`strokeOpacity` through hierarchy/network SVG output and canvas stroke painting so treemap and network opacity behavior matches canvas/SSR expectations.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes several rendering-parity gaps between client canvas/SVG and server-side static SVG output, and fixes a regression affecting custom tooltip functions in multi-series hover mode. It also adds regression coverage around multi-tooltips (including lineDataAccessor), per-axis grid suppression, time tick formatting, and SSR opacity serialization.
Changes:
- Restores multi-series hover context (
allSeries,xValue) for custom function tooltips by re-attaching hover-root fields after datum unwrapping. - Adds per-axis grid opt-out (
axis.grid = false) and propagates grid dash styling into static SSR grid rendering. - Extends
adaptiveTimeTickswith anutcoption (defaulting to UTC) and propagatesfillOpacity/strokeOpacitythrough hierarchy/network scene → SVG and canvas stroke painting for parity.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/Tooltip/Tooltip.tsx | Re-attaches multi-hover root fields after unwrap so custom tooltip functions can access allSeries/xValue. |
| src/components/Tooltip/Tooltip.test.tsx | Adds regression tests verifying function-tooltips receive allSeries and that real datum fields win over hover context. |
| src/components/stream/xyFrameAxisTypes.ts | Extends axis config with grid?: boolean to allow per-axis grid suppression. |
| src/components/stream/types.ts | Adds strokeOpacity to the shared Style type for separate stroke alpha control. |
| src/components/stream/SVGOverlay.tsx | Applies per-axis grid visibility toggles in both underlay and overlay grid rendering. |
| src/components/stream/SVGOverlay.axisCustomization.test.tsx | Adds tests to ensure per-axis grid suppression works in overlay and underlay. |
| src/components/stream/SceneToSVGNetwork.tsx | Serializes fillOpacity/strokeOpacity for network scene nodes in SVG output. |
| src/components/stream/renderers/canvasRenderHelpers.ts | Combines opacity × strokeOpacity when painting canvas strokes for network/hierarchy nodes. |
| src/components/stream/renderers/canvasRenderHelpers.test.ts | Adds a unit test covering the combined stroke alpha behavior. |
| src/components/stream/layouts/hierarchySceneBuilders.ts | Carries fillOpacity/strokeOpacity into hierarchy scene node styles for treemap/tree rendering. |
| src/components/stream/layouts/hierarchyLayoutPlugin.test.ts | Adds regression coverage for per-fill/per-stroke opacity preservation in treemap scene nodes. |
| src/components/server/staticXY.tsx | Plumbs axis configs into SSR grid rendering so static SVG honors per-axis grid settings. |
| src/components/server/staticSVGChrome.tsx | Updates renderGridSVG to honor per-axis grid suppression and dash styles in SSR output. |
| src/components/server/ssr-iris-parity.test.tsx | Adds SSR parity checks for treemap tile opacity serialization (fill/stroke opacity). |
| src/components/server/serverFeatures.test.tsx | Adds an SSR test ensuring horizontal-only grid rendering and dashed y-grid behavior. |
| src/components/charts/shared/formatUtils.ts | Adds AdaptiveTimeTickOptions and utc support to adaptiveTimeTicks while keeping UTC default. |
| src/components/charts/shared/formatUtils.test.ts | Adds tests for UTC-default behavior and local-time formatting/boundary behavior. |
| src/tests/scenarios/linechart-linedataaccessor-multi-tooltip.test.tsx | Adds end-to-end scenario coverage for LineChart multi-tooltip hover payloads (including lineDataAccessor). |
| .probe/probe.test.tsx | Introduces a probe-style file for SSR output inspection (currently implemented as logging). |
Comment on lines
+1
to
+21
| import { describe, it } from "vitest" | ||
| import { renderChart } from "../src/components/semiotic-server" | ||
|
|
||
| describe("probe", () => { | ||
| it("bar", () => { | ||
| const svg = renderChart("BarChart", { | ||
| data: [{ c: "Kafka", v: 80 }, { c: "Flink", v: 60 }], | ||
| categoryAccessor: "c", valueAccessor: "v", | ||
| }) | ||
| console.log("BAR FONT-SIZE COUNT:", (svg.match(/font-size/g) || []).length) | ||
| ;(svg.match(/<text[^>]*>[^<]*<\/text>/g) || []).slice(0, 12).forEach(t => console.log(t)) | ||
| }) | ||
| it("line", () => { | ||
| const svg = renderChart("LineChart", { | ||
| data: [{ x: 1, y: 2 }, { x: 2, y: 5 }], | ||
| xAccessor: "x", yAccessor: "y", xLabel: "Time", yLabel: "Val", | ||
| }) | ||
| console.log("LINE FONT-SIZE COUNT:", (svg.match(/font-size/g) || []).length) | ||
| ;(svg.match(/<text[^>]*>[^<]*<\/text>/g) || []).slice(0, 10).forEach(t => console.log(t)) | ||
| }) | ||
| }) |
Comment on lines
+201
to
+203
| /** Options for {@link adaptiveTimeTicks}. UTC remains the default so existing | ||
| * server-rendered charts stay deterministic; set `utc: false` to format in | ||
| * the viewer's local timezone. */ |
Replace probe debug logging with real assertions that server-rendered BarChart and LineChart SVGs include concrete `font-size="12"` text and expected axis/category labels. Also refresh cold-consumer bundle size snapshots in benchmarks/README to reflect the latest build output, and clarify the adaptive time tick docs to say formatting uses the runtime’s local timezone when `utc: false`.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/tests/scenarios/linechart-linedataaccessor-multi-tooltip.test.tsx:28
- This ResizeObserver polyfill is redundant: test environments already install a global no-op ResizeObserver in
src/setupTests.ts. Keeping another copy here adds noise and creates another place to maintain if the shared mock changes.
const resizeObserverGlobal = globalThis as typeof globalThis & { ResizeObserver?: typeof ResizeObserver }
if (typeof resizeObserverGlobal.ResizeObserver === "undefined") {
resizeObserverGlobal.ResizeObserver = class {
constructor(_callback: ResizeObserverCallback) {}
observe() {}
unobserve() {}
disconnect() {}
} as typeof ResizeObserver
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
src/components/stream/SVGOverlay.tsx:629
- Same issue as the underlay grid: this overlay grid logic only checks
orient === "bottom"andorient === "left", sogrid: false(orgridStyle) on atop/rightaxis won’t be honored even though the axis config type supports those orientations.
const bottomGridStyle = resolveGridDash(axes?.find(a => a.orient === "bottom")?.gridStyle)
const leftGridStyle = resolveGridDash(axes?.find(a => a.orient === "left")?.gridStyle)
const showXGrid = axes?.find(a => a.orient === "bottom")?.grid !== false
const showYGrid = axes?.find(a => a.orient === "left")?.grid !== false
src/components/stream/SVGOverlay.tsx:249
grid/gridStyleare read only from the bottom/left axes, so settinggrid: false(or agridStyle) on atoporrightaxis has no effect even thoughXYFrameAxisConfig.orientsupports those orientations. If a chart is configured withorient: "top"for the x-axis (or"right"for y), the grid opt-out won’t work as expected.
This issue also appears on line 626 of the same file.
const bottomGridStyle = resolveGridDash(axes?.find(a => a.orient === "bottom")?.gridStyle)
const leftGridStyle = resolveGridDash(axes?.find(a => a.orient === "left")?.gridStyle)
const showXGrid = axes?.find(a => a.orient === "bottom")?.grid !== false
const showYGrid = axes?.find(a => a.orient === "left")?.grid !== false
src/components/server/staticSVGChrome.tsx:416
renderGridSVGonly consults the bottom/left axes forgridandgridStyle. If a static SSR chart usesorient: "top"for the x-axis (or"right"for y), the per-axis grid opt-out and dash styling won’t apply, despiteXYFrameAxisConfigsupporting those orientations.
const showXGrid = axes?.find(axis => axis.orient === "bottom")?.grid !== false
const showYGrid = axes?.find(axis => axis.orient === "left")?.grid !== false
const xGridDash = resolveGridDash(axes?.find(axis => axis.orient === "bottom")?.gridStyle)
const yGridDash = resolveGridDash(axes?.find(axis => axis.orient === "left")?.gridStyle)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (7)
src/components/stream/SVGOverlay.tsx:249
axis.grid(andgridStyle) is currently only read from the bottom/left axis configs. If a consumer uses a top x-axis or right y-axis, setting{ orient: "top", grid: false }/{ orient: "right", grid: false }will not suppress the corresponding grid lines. Consider resolving the x-axis config from bottom or top, and the y-axis config from left or right, so the opt-out works regardless of orientation.
const bottomGridStyle = resolveGridDash(axes?.find(a => a.orient === "bottom")?.gridStyle)
const leftGridStyle = resolveGridDash(axes?.find(a => a.orient === "left")?.gridStyle)
const showXGrid = axes?.find(a => a.orient === "bottom")?.grid !== false
const showYGrid = axes?.find(a => a.orient === "left")?.grid !== false
src/components/stream/SVGOverlay.tsx:629
- Same as the underlay: grid visibility/style is only keyed off bottom/left axis configs. If the chart uses a top/right axis,
{ grid: false }on those axes won’t affect the rendered grid lines.
const bottomGridStyle = resolveGridDash(axes?.find(a => a.orient === "bottom")?.gridStyle)
const leftGridStyle = resolveGridDash(axes?.find(a => a.orient === "left")?.gridStyle)
const showXGrid = axes?.find(a => a.orient === "bottom")?.grid !== false
const showYGrid = axes?.find(a => a.orient === "left")?.grid !== false
src/components/server/staticSVGChrome.tsx:416
- Static SSR grid rendering has the same orientation limitation as the client SVGOverlay:
axis.grid/axis.gridStyleare only read from bottom/left configs. This means{ orient: "top", grid: false }or{ orient: "right", grid: false }won’t suppress grid lines in static SVG output.
const showXGrid = axes?.find(axis => axis.orient === "bottom")?.grid !== false
const showYGrid = axes?.find(axis => axis.orient === "left")?.grid !== false
const xGridDash = resolveGridDash(axes?.find(axis => axis.orient === "bottom")?.gridStyle)
const yGridDash = resolveGridDash(axes?.find(axis => axis.orient === "left")?.gridStyle)
src/components/stream/SceneToSVGNetwork.tsx:46
- This adds
fillOpacity/strokeOpacityserialization for nodes, but network edges still don’t passstrokeOpacitythrough to SVG elements (e.g. ChordDiagram’s edgeStyle usesstrokeOpacity). That can leave SSR output darker/more opaque than canvas for charts that encode edge opacity viastrokeOpacityrather thanopacity.
cx={n.cx} cy={n.cy} r={n.r}
fill={hatch ? `url(#net-circle-${i}-hatch)` : svgFill(n.style.fill)}
stroke={n.style.stroke}
strokeWidth={n.style.strokeWidth}
fillOpacity={n.style.fillOpacity}
strokeOpacity={n.style.strokeOpacity}
opacity={n.style.opacity}
etc/api-surface/semiotic.api.md:86
- The API surface now references
AdaptiveTimeTickOptions, but this type is not declared anywhere in this api-surface snapshot. This usually indicates the api-surface output wasn’t regenerated after introducing the new exported options type, and can cause downstream type consumers/docs to reference an undefined symbol.
function adaptiveTimeTicks(granularity?: TimeGranularity | undefined, options?: AdaptiveTimeTickOptions | undefined): (value: string | number | Date, index?: number, allTicks?: number[]) => string
etc/api-surface/semiotic-utils.api.md:19
- The api-surface snapshot references
AdaptiveTimeTickOptionsinadaptiveTimeTicks(...), but the type isn’t declared in this file. This likely means the api-surface output wasn’t regenerated after the change.
function adaptiveTimeTicks(granularity?: TimeGranularity | undefined, options?: AdaptiveTimeTickOptions | undefined): (value: string | number | Date, index?: number, allTicks?: number[]) => string
etc/api-surface/semiotic-utils-core.api.md:18
- The api-surface snapshot references
AdaptiveTimeTickOptionsinadaptiveTimeTicks(...), but the type isn’t declared in this file. This suggests the snapshot is out of sync with the compiled d.ts output.
function adaptiveTimeTicks(granularity?: TimeGranularity | undefined, options?: AdaptiveTimeTickOptions | undefined): (value: string | number | Date, index?: number, allTicks?: number[]) => string
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/components/charts/shared/formatUtils.ts:206
adaptiveTimeTicksnow exposes a namedAdaptiveTimeTickOptionstype in its public signature, but the main entrypoints that re-exportadaptiveTimeTicks(e.g.src/components/semiotic.ts,src/components/semiotic-utils-core.ts) do not re-exportAdaptiveTimeTickOptions. This leaves consumers unable to import the options type from the same module they importadaptiveTimeTicksfrom, and the checked-in API surface files now referenceAdaptiveTimeTickOptionswithout declaring it.
Consider re-exporting AdaptiveTimeTickOptions alongside adaptiveTimeTicks in the relevant public entrypoints (and then regenerating the etc/api-surface/*.api.md files), or alternatively change the function signature to use an inline { utc?: boolean } options type to avoid leaking a separately-named public type.
/** Options for {@link adaptiveTimeTicks}. UTC remains the default so existing
* server-rendered charts stay deterministic; set `utc: false` to format in
* the runtime's local timezone. */
export interface AdaptiveTimeTickOptions {
utc?: boolean
}
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.
Restores multi-series tooltip context for custom tooltip functions by preserving hover-root fields (
allSeries/xValue) after datum unwrapping, with new regression coverage forLineChartmulti-tooltip behavior (includinglineDataAccessor).Improves rendering parity and axis customization by adding per-axis grid opt-out (
axis.grid) to SVG underlay/overlay and static SSR grid rendering, addingadaptiveTimeTicks(..., { utc: false })while keeping UTC as the default, and propagatingfillOpacity/strokeOpacitythrough hierarchy/network SVG output and canvas stroke painting so treemap and network opacity behavior matches canvas/SSR expectations.This pull request adds new tests and improves existing functionality and coverage for chart components, with a focus on tooltips, grid rendering, and time tick formatting. It also fixes a regression with custom multi-series tooltips and enhances server-side SVG rendering for Treemap and grid lines. The most important changes are summarized below.
Tooltip and Hover Behavior Improvements
allSeriesin multi-series (tooltip="multi") mode by ensuring these fields are re-attached after unwrapping the datum innormalizeTooltip, allowing custom tooltips to behave like the built-inMultiPointTooltip(src/components/Tooltip/Tooltip.tsx,Tooltip.test.tsx,scenarios/linechart-linedataaccessor-multi-tooltip.test.tsx). [1] [2] [3]lineDataAccessoronLineChart.Time Tick Formatting Enhancements
adaptiveTimeTicksto support local time formatting (not just UTC) via a new options argument, updated the implementation to use local or UTC calendar boundaries as appropriate, and added tests to verify both modes. [1] [2] [3] [4] [5] [6] [7]Server-side Rendering and Grid Lines
Internal Refactoring
renderGridSVG, preparing for more flexible grid rendering logic. [1] [2] [3]Additional Testing