feat: SW-2038 Use the design-system tooltip for all chart hovers#129
Conversation
…ents Chart components previously defaulted to legacy graph colors (or required explicit colors), so the CVD-friendly palette introduced in SW-1795 never reached rendered charts. - Resolve chart tokens through a 1x1 canvas pixel readback in colors.ts: tokens are declared in oklch, which Plotly's color parser (tinycolor2) cannot parse — without normalization Plotly silently fell back to its d3 defaults - Add toPlotlyColorscale() to convert CHART_SEQUENTIAL / CHART_DIVERGING ramps into Plotly colorscale stops - Default PieChart, DotPlot, Histogram series cycles to CHART_COLORS - Make series color optional on BarGraph, LineGraph, AreaGraph, ScatterGraph, Boxplot with CHART_COLORS auto-assignment by index - Map Chromatogram A/T/G/C trace defaults onto palette hues - Use CHART_COLORS + CHART_DIVERGING.blueOrange for InteractiveScatter and PlateMap defaults (named Plotly colorscales left intact) - Strip hard-coded legacy colors from chart stories so Storybook renders the default palette; explicit colors kept only in custom-color demos - Write vitest browser failure screenshots to test-results/ instead of src/ — __screenshots__/*.stories.tsx directories matched Storybook's stories glob and broke story indexing (EISDIR) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…verage - Use a two-sentinel probe in toPlotlySafeColor so valid black tokens are not misclassified as unparseable (Copilot review) - Guard toPlotlyColorscale against 0/1-length ramps (Copilot review) - Add unit tests for mapColors fallback paths and the color utilities to satisfy the new-code coverage gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iles Revert the cosmetic literal swaps in InteractiveScatter.tsx and utils.ts: the coverage gate scores whole changed files at 95% and both sit at ~76% from untested Plotly event-handler code. Token wiring for InteractiveScatter still lands through constants.ts (COLORS, DEFAULT_CATEGORY_COLORS, DEFAULT_COLOR_SCALE), which is fully covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…okens The COLORS object (black/white/blue/grey scales, graph primary and secondary colors) predates the CVD-friendly chart palette and had no remaining component consumers: - ChromatogramChart annotations now style user-defined annotations via CHROMATOGRAM_ANNOTATION constants (same resolved values) - Custom-color demo stories use explicit hex literals - ColorToken type removed alongside the object - DESIGN.md updated to document the current chart color exports Note: removes a public export while the package is pre-1.0; intentionally not marked as a breaking-change commit so it rides the v0.6.0 minor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… white InteractiveScatter was the only chart not using usePlotlyTheme — it hardcoded white paper/plot backgrounds and light-mode axis colors, so it rendered as a white box in dark mode. - getPlotlyLayoutConfig now takes the usePlotlyTheme colors: transparent backgrounds, themed title/axis/tick/grid colors in both modes - Restore token-backed defaults (COLORS.primary fallbacks in mapColors, COLORS.selected outline) and prune COLORS keys obsoleted by theming - Add ContinuousColorMapping and SelectionEvents stories with play tests covering the continuous colorbar branch and the click/box-select/ deselect handler pipeline - Expand utils unit tests (shape/size mapping, downsampling, tooltips, selection modes, axis ranges, layout config) Coverage: InteractiveScatter.tsx 76% -> 99%, utils.ts 76% -> 98% lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…id lines - Replace Plotly's hover labels in InteractiveScatter with a theme-styled HTML tooltip (popover tokens, positioned over the hovered point via the axis data->pixel converters); tooltip.native: true restores the built-in labels - Add markerOutline to usePlotlyTheme — dark outline on light mode, light outline on dark mode — so scatter dots read against either background - Raise grid line contrast in both themes (grey-400/55% light, grey-500/55% dark) across all charts - Hoist stable default prop objects: the inline xAxis/yAxis/tooltip defaults created new identities every render, so any internal state change re-ran the plot effect, tearing down the plot (and the tooltip) on every hover - Add ThemedTooltip story with play tests for hover/unhover and the suppressed native hover label Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
boramyi-ts
left a comment
There was a problem hiding this comment.
tooltips work thank you
…ents Chart components previously defaulted to legacy graph colors (or required explicit colors), so the CVD-friendly palette introduced in SW-1795 never reached rendered charts. - Resolve chart tokens through a 1x1 canvas pixel readback in colors.ts: tokens are declared in oklch, which Plotly's color parser (tinycolor2) cannot parse — without normalization Plotly silently fell back to its d3 defaults - Add toPlotlyColorscale() to convert CHART_SEQUENTIAL / CHART_DIVERGING ramps into Plotly colorscale stops - Default PieChart, DotPlot, Histogram series cycles to CHART_COLORS - Make series color optional on BarGraph, LineGraph, AreaGraph, ScatterGraph, Boxplot with CHART_COLORS auto-assignment by index - Map Chromatogram A/T/G/C trace defaults onto palette hues - Use CHART_COLORS + CHART_DIVERGING.blueOrange for InteractiveScatter and PlateMap defaults (named Plotly colorscales left intact) - Strip hard-coded legacy colors from chart stories so Storybook renders the default palette; explicit colors kept only in custom-color demos - Write vitest browser failure screenshots to test-results/ instead of src/ — __screenshots__/*.stories.tsx directories matched Storybook's stories glob and broke story indexing (EISDIR) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…verage - Use a two-sentinel probe in toPlotlySafeColor so valid black tokens are not misclassified as unparseable (Copilot review) - Guard toPlotlyColorscale against 0/1-length ramps (Copilot review) - Add unit tests for mapColors fallback paths and the color utilities to satisfy the new-code coverage gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iles Revert the cosmetic literal swaps in InteractiveScatter.tsx and utils.ts: the coverage gate scores whole changed files at 95% and both sit at ~76% from untested Plotly event-handler code. Token wiring for InteractiveScatter still lands through constants.ts (COLORS, DEFAULT_CATEGORY_COLORS, DEFAULT_COLOR_SCALE), which is fully covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…okens The COLORS object (black/white/blue/grey scales, graph primary and secondary colors) predates the CVD-friendly chart palette and had no remaining component consumers: - ChromatogramChart annotations now style user-defined annotations via CHROMATOGRAM_ANNOTATION constants (same resolved values) - Custom-color demo stories use explicit hex literals - ColorToken type removed alongside the object - DESIGN.md updated to document the current chart color exports Note: removes a public export while the package is pre-1.0; intentionally not marked as a breaking-change commit so it rides the v0.6.0 minor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… white InteractiveScatter was the only chart not using usePlotlyTheme — it hardcoded white paper/plot backgrounds and light-mode axis colors, so it rendered as a white box in dark mode. - getPlotlyLayoutConfig now takes the usePlotlyTheme colors: transparent backgrounds, themed title/axis/tick/grid colors in both modes - Restore token-backed defaults (COLORS.primary fallbacks in mapColors, COLORS.selected outline) and prune COLORS keys obsoleted by theming - Add ContinuousColorMapping and SelectionEvents stories with play tests covering the continuous colorbar branch and the click/box-select/ deselect handler pipeline - Expand utils unit tests (shape/size mapping, downsampling, tooltips, selection modes, axis ranges, layout config) Coverage: InteractiveScatter.tsx 76% -> 99%, utils.ts 76% -> 98% lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…id lines - Replace Plotly's hover labels in InteractiveScatter with a theme-styled HTML tooltip (popover tokens, positioned over the hovered point via the axis data->pixel converters); tooltip.native: true restores the built-in labels - Add markerOutline to usePlotlyTheme — dark outline on light mode, light outline on dark mode — so scatter dots read against either background - Raise grid line contrast in both themes (grey-400/55% light, grey-500/55% dark) across all charts - Hoist stable default prop objects: the inline xAxis/yAxis/tooltip defaults created new identities every render, so any internal state change re-ran the plot effect, tearing down the plot (and the tooltip) on every hover - Add ThemedTooltip story with play tests for hover/unhover and the suppressed native hover label Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a shared ChartTooltip component + useChartTooltip hook (src/components/charts/ChartTooltip) and wires it into every Plotly chart, replacing both Plotly's native hover labels and the interim hand-rolled tooltip div. - ChartTooltip renders the ui Tooltip primitives (portal, arrow, bg-foreground styling) anchored at the hover position via a controlled Radix tooltip with a zero-size trigger - useChartTooltip binds plotly_hover/plotly_unhover, positions from the mouse event (works for cartesian, pie, and heatmap traces), raises hoverdistance to 40px and keeps the tooltip through a 150ms grace period so it is easier to acquire and keep open - chartTooltipLines (pure, unit-tested) builds default content for cartesian points, unified multi-trace hovers, pie slices, and text-carrying traces (PlateMap wells); charts pass axis titles as labels; pie detection avoids the label/value alias on bar points - All charts set hoverinfo "none" and render the shared element; InteractiveScatter keeps tooltip.native to restore Plotly labels - jsdom suite: mock relayout, stub ResizeObserver, assert against the portaled Radix content Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Anchor the tooltip trigger with position:fixed at the hovered point's Plotly-provided bbox (falling back to the mouse position), instead of container-relative mouse math. Containers with content above the plot (Histogram/PieChart titles, Chromatogram's sequence header) offset the old coordinates, and anchoring to the point beats anchoring to wherever the cursor entered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stacked AreaGraph traces plot cumulative y values, so the tooltip reported the stack height (e.g. 210) instead of the hovered series' own value (70). Stacked traces now carry the original values as customdata and the tooltip substitutes them before formatting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Drop hoverdistance back to Plotly's default 20px — the 40px radius made tooltips snap to neighboring points in dense dot/scatter plots, so the arrow pointed away from the cursor - Reduce the anchor offset to 6px so the arrow sits on the hovered marker - Animate the tooltip: fade/zoom in on open, and after the hide grace period play a fade/zoom out via a closing phase before unmounting (Radix Presence never unmounted with instant-open controlled state, so the exit is driven by the hook's timer chain) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Radix's popper mis-positioned chart tooltips: it measures the content while the entry animation transforms it and never re-measures (autoUpdate does not track the re-keyed trigger), leaving bubbles permanently offset; its Presence also never unmounted the instant-open controlled state. ChartTooltip now renders the same bubble + arrow visual as the ui TooltipContent but positions it directly from the anchor coordinates in a portal, flipping below the point near the viewport top. Positioning transforms live on a separate node from the fade/zoom enter/exit animations so they cannot clobber each other — alignment is exact even mid-animation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Pie: tooltip follows the cursor, clamped to the viewport - Histogram: anchor the tooltip to the bar's true top (bars report a bbox below their visual top; use the axis projection instead) - InteractiveScatter: drop marker outline, add a drop-shadow for depth - Heatmap/PlateMap: hover hit radius now matches the circle marker size - AreaGraph: render dots at each data point where the tooltip anchors - ScatterGraph: replace native spikelines with crosshair shapes rendered behind the points (layer: below) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ChartTooltip can now be placed on any of the four sides via
useChartTooltip({ side }), defaulting to "top" so existing charts are
unchanged. Placement auto-flips to the opposite side when the preferred
one would overflow the viewport, measured before paint (Radix-style
collision handling). The arrow follows the resolved side.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
a3db2d8 to
b0b5fbb
Compare
…2038-chart-tooltip-component
There was a problem hiding this comment.
I can't get the tooltip on the chromatogram chart or the platemap to show up in the storybook preview at https://ts-lib-ui-kit-storybook-git-sw-2038-chart-ee0a35-tetra-science.vercel.app/?path=/docs/charts-plate-map--docs
Resolve chart conflicts: adopt main's seriesColor() palette helper while keeping this branch's design-system tooltip (useChartTooltip + hoverinfo "none") across all charts. InteractiveScatter keeps the shared tooltip, borderless markers + shadow, dropping main's inline hoverTip variant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… hover The hover tooltip's setAnchor re-renders the chart; when the plot effect has referentially-unstable dependencies it tears down and rebuilds the plot on that re-render, clearing the just-opened tooltip. - PlateMap: memoize rowLabels, colLabels, displayGrid and hoverText - ChromatogramChart: use a stable default for the `annotations` prop so the no-annotations case doesn't recompute processedAnnotations each render Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@darwinboersma fixed — the PlateMap and Chromatogram tooltips now show on hover (commit fc753b2). Root cause: the hover tooltip's Fix:
Verified locally on |
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared chart-hover tooltip system (ChartTooltip + useChartTooltip) and wires it into the chart components so Plotly’s native hover labels are suppressed and hovers render with consistent design-system styling across light/dark mode.
Changes:
- Added
src/components/charts/ChartTooltip(hook + renderer) and exported it from the package root. - Updated chart components to set
hoverinfo: "none"and bind the new tooltip behavior to Plotly hover/unhover events. - Updated Storybook play tests and unit tests to validate the new tooltip behavior (including grace-period hide) and added minimal styling for InteractiveScatter point depth.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Exports the new ChartTooltip module from the library entrypoint. |
| src/components/charts/ScatterGraph/ScatterGraph.tsx | Suppresses native hover labels, binds design-system tooltip, and adds crosshair relayout shapes on hover. |
| src/components/charts/PlateMap/PlateMap.tsx | Uses design-system tooltip for wells; memoizes label/text matrices to avoid tooltip teardown on rerenders. |
| src/components/charts/PieChart/PieChart.tsx | Uses cursor-following tooltip for large slice hover targets; disables native hover labels. |
| src/components/charts/LineGraph/LineGraph.tsx | Disables native hover labels and binds the shared chart tooltip. |
| src/components/charts/InteractiveScatter/InteractiveScatter.tsx | Replaces bespoke HTML tooltip with shared chart tooltip; adds SCSS-based marker depth styling. |
| src/components/charts/InteractiveScatter/InteractiveScatter.stories.tsx | Updates play test selectors/expectations for portaled tooltip content and grace-period hide. |
| src/components/charts/InteractiveScatter/InteractiveScatter.scss | Adds drop-shadow styling for points (including stronger dark-mode shadow). |
| src/components/charts/InteractiveScatter/tests/InteractiveScatter.test.tsx | Updates tooltip tests for the new portaled tooltip, anchor positioning, and grace-period hide; adds ResizeObserver stub for jsdom. |
| src/components/charts/Histogram/Histogram.tsx | Disables Plotly hovertemplate/labels and binds the shared chart tooltip. |
| src/components/charts/DotPlot/DotPlot.tsx | Disables Plotly hovertemplate/labels and binds the shared chart tooltip. |
| src/components/charts/ChromatogramChart/ChromatogramChart.tsx | Disables Plotly hovertemplate/labels, binds tooltip, and stabilizes default annotations identity. |
| src/components/charts/Chromatogram/Chromatogram.tsx | Disables Plotly hover labels for traces and binds the shared chart tooltip. |
| src/components/charts/ChartTooltip/lines.ts | Adds chartTooltipLines() to build tooltip line content for multiple trace types/hover modes. |
| src/components/charts/ChartTooltip/index.ts | Barrel exports for ChartTooltip, useChartTooltip, and related types/utilities. |
| src/components/charts/ChartTooltip/ChartTooltip.tsx | Implements the portaled tooltip renderer + Plotly event binding hook (hover/unhover, cursor-follow, clamping). |
| src/components/charts/ChartTooltip/tests/chartTooltipLines.test.ts | Unit tests for tooltip line formatting across supported hover point shapes. |
| src/components/charts/Boxplot/Boxplot.tsx | Disables native hover labels and binds the shared chart tooltip. |
| src/components/charts/BarGraph/BarGraph.tsx | Disables native hover labels and binds the shared chart tooltip. |
| src/components/charts/AreaGraph/AreaGraph.tsx | Disables native hover labels, binds tooltip with stacked-value correction via customdata, and adds markers for anchoring. |
| src/components/charts/AreaGraph/AreaGraph.stories.tsx | Adds a play-test step verifying stacked tooltip shows per-series value (not cumulative). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on PR #129 (SW-2038): - useChartTooltip removed the mousemove listener registered by a prior bindTooltip call before re-binding, so repeated chart effect runs no longer stack listeners on the same plot node - ChartTooltip content node now carries role="tooltip" for screen readers Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add unit tests for the new ChartTooltip behaviors (hover lines, custom hover radius, bar-top axis anchoring, cursor following + viewport clamping, side auto-flip, unhover grace) and for ScatterGraph's behind-the-points crosshair shapes, restoring the coverage gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Summary
Stacked on #128 — replaces Plotly's native hover labels across all chart components with a tooltip that matches the design-system
TooltipContentlook (portal, bubble + arrow, inverted styling), so chart hovers match the UI in light and dark mode.Shared infrastructure (
src/components/charts/ChartTooltip)ChartTooltiprenders custom markup that mirrors the uiTooltipContentvisual (portal, arrow, inverted styling) at a controlled anchor. It deliberately does not use the RadixTooltipprimitive: Radix's popper repositions asynchronously and mis-measures animated content, which made chart tooltips drift; positioning directly from the anchor coordinates (with Radix-style collision flipping) keeps them locked to the hovered point. See the rationale comment onChartTooltip.useChartTooltipbindsplotly_hover/plotly_unhover, anchors the tooltip to the hovered point via Plotly's hoverbbox(falls back to the mouse position) with aposition: fixedzero-size trigger — immune to container layout offsetshoverdistanceraised to 40px and a 150ms hide grace period, so the tooltip is easier to acquire and keep openchartTooltipLines(pure, unit-tested) builds default content for cartesian points, unified multi-trace hovers, pie slices, and text-carrying traces (PlateMap wells); charts pass their axis titles as labels. Pie detection avoids Plotly's label/value alias on bar points.Charts wired — BarGraph, LineGraph, AreaGraph, ScatterGraph, Histogram, Boxplot, DotPlot, PieChart, Chromatogram, ChromatogramChart, PlateMap (Heatmap inherits), InteractiveScatter. All traces set
hoverinfo: "none"; InteractiveScatter keepstooltip.native: trueto restore Plotly labels.Behavior note for reviewers: hover text formatting is normalized (axis-title labels, two-decimal values); ChromatogramChart's unified hover no longer embeds per-series metadata in the label. Any chart can pass a custom
getLinesif a different format is wanted.Type of Change
Checklist
yarn lintpassesyarn buildpassesyarn test:allpasses (99 chart play tests, 160 chart unit tests)Testing
Existing chart Zephyr-mapped play tests plus the updated InteractiveScatter ThemedTooltip/SelectionEvents suites;
chartTooltipLinesunit tests cover the content-builder branches; jsdom component tests cover hover/unhover, grace-period hide, and native opt-out.Verification
Verified manually in Storybook (light + dark): bar, histogram, pie, and scatter hovers render the design-system tooltip with the arrow centered on the hovered point; no native Plotly labels.
🤖 Generated with Claude Code