From 998cfd61d64818492e142deac3174f5f6451a706 Mon Sep 17 00:00:00 2001 From: Guillem Arias Fauste Date: Tue, 2 Jun 2026 23:59:55 +0200 Subject: [PATCH] refactor(charts): extract shared chart-tooltip className to one source (#2106) Closes #2011. time-series and sankey each created their cursor-following tooltip with a duplicated className literal that had already drifted apart: time-series was missing `text-primary` and `z-50`. Move the visual contract into app/javascript/utils/chart_tooltip.js as CHART_TOOLTIP_CLASSES and have both controllers reference it. Each keeps its own behavioural classes (time-series its initial `opacity-0`; both `top-0`; sankey toggles opacity via inline style). `privacy-sensitive` stays bundled so future copies can't drop it. Also exports a createChartTooltip factory for the raw-DOM idiom. goal_projection_chart_controller is not in main yet (it lands with the goals work in #1798); it migrates to the same symbol there. --- .../controllers/sankey_chart_controller.js | 8 +++--- .../time_series_chart_controller.js | 7 +++--- app/javascript/utils/chart_tooltip.js | 25 +++++++++++++++++++ 3 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 app/javascript/utils/chart_tooltip.js diff --git a/app/javascript/controllers/sankey_chart_controller.js b/app/javascript/controllers/sankey_chart_controller.js index cb24edeb9..b7d12c036 100644 --- a/app/javascript/controllers/sankey_chart_controller.js +++ b/app/javascript/controllers/sankey_chart_controller.js @@ -1,6 +1,7 @@ import { Controller } from "@hotwired/stimulus"; import * as d3 from "d3"; import { sankey } from "d3-sankey"; +import { CHART_TOOLTIP_CLASSES } from "utils/chart_tooltip"; import { sankeyNodeHasChildren, zoomSankeyData } from "utils/sankey_zoom"; // Connects to data-controller="sankey-chart" @@ -509,10 +510,9 @@ export default class extends Controller { this.tooltip = d3 .select(dialog || document.body) .append("div") - .attr( - "class", - "bg-container text-primary text-sm font-sans p-2 border border-secondary rounded-lg pointer-events-none absolute z-50 top-0 privacy-sensitive", - ) + // Shared visual contract + this chart's positioning class; opacity is + // toggled via inline style below. + .attr("class", `${CHART_TOOLTIP_CLASSES} top-0`) .style("opacity", 0) .style("pointer-events", "none"); } diff --git a/app/javascript/controllers/time_series_chart_controller.js b/app/javascript/controllers/time_series_chart_controller.js index d4f4988af..1bf6f8d89 100644 --- a/app/javascript/controllers/time_series_chart_controller.js +++ b/app/javascript/controllers/time_series_chart_controller.js @@ -1,5 +1,6 @@ import { Controller } from "@hotwired/stimulus"; import * as d3 from "d3"; +import { CHART_TOOLTIP_CLASSES } from "utils/chart_tooltip"; const parseLocalDate = d3.timeParse("%Y-%m-%d"); @@ -287,10 +288,8 @@ export default class extends Controller { this._d3Tooltip = d3 .select(`#${this.element.id}`) .append("div") - .attr( - "class", - "bg-container text-sm font-sans absolute p-2 border border-secondary rounded-lg pointer-events-none opacity-0 top-0 privacy-sensitive", - ); + // Shared visual contract + this chart's initial-hidden / positioning classes. + .attr("class", `${CHART_TOOLTIP_CLASSES} opacity-0 top-0`); } _trackMouseForShowingTooltip() { diff --git a/app/javascript/utils/chart_tooltip.js b/app/javascript/utils/chart_tooltip.js new file mode 100644 index 000000000..3b3b6f145 --- /dev/null +++ b/app/javascript/utils/chart_tooltip.js @@ -0,0 +1,25 @@ +// Single source of truth for the cursor-following tooltip used by the chart +// controllers (time-series, sankey, and goal-projection once it lands from the +// goals work). Keeping the visual contract here stops the bg / text / border / +// privacy-sensitive classes from drifting apart across the controllers, the way +// they had before (time-series was missing `text-primary` and `z-50`). +// +// This is the VISUAL contract only. Callers append their own behavioural +// classes (initial `opacity-0`, `top-0`, …) or set them via inline styles, +// because how each chart shows/hides and positions its tooltip differs. +// +// Not to be confused with DS::Tooltip — that is the info-icon hint primitive +// (bg-inverse, aria-describedby, anchored to a static trigger). This is a +// data-card surface created and updated inside D3 handler code. +export const CHART_TOOLTIP_CLASSES = + "bg-container text-primary text-sm font-sans absolute p-2 border border-secondary rounded-lg pointer-events-none z-50 privacy-sensitive"; + +// Convenience factory for the raw-DOM idiom (no d3.select). Creates a hidden +// tooltip div carrying the shared contract and appends it to `parent`. +export function createChartTooltip(parent) { + const tooltip = document.createElement("div"); + tooltip.className = CHART_TOOLTIP_CLASSES; + tooltip.style.display = "none"; + parent.appendChild(tooltip); + return tooltip; +}