-
-
Notifications
You must be signed in to change notification settings - Fork 176
feat(chart): add links to data point tooltips #1438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ sqlpage_chart = (() => { | |
| }; | ||
|
|
||
| /** @typedef {number|string|Date} XValue */ | ||
| /** @typedef { {x:XValue, y:number|null, z?:number, fillColor?:string} } ChartPoint */ | ||
| /** @typedef { {x:XValue, y:number|null, z?:number, fillColor?:string, link?:string} } ChartPoint */ | ||
| /** @typedef { {name:string, data:ChartPoint[]} } ChartSeries */ | ||
| /** @typedef { { [name:string]: ChartSeries } } Series */ | ||
|
|
||
|
|
@@ -182,7 +182,7 @@ sqlpage_chart = (() => { | |
| const reference_rows = data.points.filter((row) => !Array.isArray(row)); | ||
| /** @type { Series } */ | ||
| const series_map = {}; | ||
| for (const [name, old_x, old_y, color, z] of points) { | ||
| for (const [name, old_x, old_y, color, z, link] of points) { | ||
| series_map[name] = series_map[name] || { name, data: [] }; | ||
| let x = old_x; | ||
| let y = old_y; | ||
|
|
@@ -192,7 +192,13 @@ sqlpage_chart = (() => { | |
| y = y.map((y) => new Date(y).getTime()); | ||
| else x = new Date(x); | ||
| } | ||
| series_map[name].data.push({ x, y, z, fillColor: named_color(color) }); | ||
| series_map[name].data.push({ | ||
| x, | ||
| y, | ||
| z, | ||
| link, | ||
| fillColor: named_color(color), | ||
| }); | ||
| } | ||
| if (data.xmin == null) data.xmin = undefined; | ||
| if (data.xmax == null) data.xmax = undefined; | ||
|
|
@@ -332,8 +338,9 @@ sqlpage_chart = (() => { | |
| }, | ||
| tooltip: { | ||
| fillSeriesColor: false, | ||
| custom: | ||
| chart_type === "bubble" || chart_type === "scatter" | ||
| custom: points.some((point) => point[5]) | ||
| ? (args) => chartTooltip(args, points) | ||
| : chart_type === "bubble" || chart_type === "scatter" | ||
|
Comment on lines
+341
to
+343
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ? bubbleTooltip | ||
| : undefined, | ||
| y: { | ||
|
|
@@ -372,9 +379,11 @@ sqlpage_chart = (() => { | |
| c.removeAttribute("data-pre-init"); | ||
| } | ||
|
|
||
| function bubbleTooltip({ seriesIndex, dataPointIndex, w }) { | ||
| const { name, data } = w.config.series[seriesIndex]; | ||
| const point = data[dataPointIndex]; | ||
| function chartTooltip({ seriesIndex, dataPointIndex, w }, raw_points) { | ||
| const series = w.config.series[seriesIndex]; | ||
| const name = series?.name || w.config.labels?.[dataPointIndex] || ""; | ||
| const point = series?.data?.[dataPointIndex] || {}; | ||
| const link = point.link || raw_points[dataPointIndex]?.[5]; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the screenshot also shows that rows without links still show the |
||
|
|
||
| const tooltip = document.createElement("div"); | ||
| tooltip.className = "apexcharts-tooltip-text"; | ||
|
|
@@ -404,9 +413,26 @@ sqlpage_chart = (() => { | |
| axisValue.appendChild(valueSpan); | ||
| tooltip.appendChild(axisValue); | ||
| } | ||
| add_link_to_tooltip(tooltip, link); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] snake case? |
||
| return tooltip.outerHTML; | ||
| } | ||
|
|
||
| function bubbleTooltip(args) { | ||
| return chartTooltip(args, []); | ||
| } | ||
|
|
||
| /** @param {HTMLElement} tooltip @param {string|undefined} link */ | ||
| function add_link_to_tooltip(tooltip, link) { | ||
| if (!link) return; | ||
| const linkContainer = document.createElement("div"); | ||
| linkContainer.className = "apexcharts-tooltip-y-group"; | ||
| const anchor = document.createElement("a"); | ||
| anchor.href = link; | ||
| anchor.textContent = "Open link"; | ||
| linkContainer.appendChild(anchor); | ||
| tooltip.appendChild(linkContainer); | ||
| } | ||
|
|
||
| return sqlpage_chart; | ||
| })(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| SELECT 'chart' AS component, 'test-chart' AS id, 'Chart test fixture' AS title, | ||
| 'rangeBar' AS type, TRUE AS time; | ||
| SELECT 'Design' AS series, 'Alice' AS label, | ||
| '2024-03-01' AS value, '2024-03-05' AS value, | ||
| '/workpackage_edit.sql?workpackage_name=Design' AS link; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -257,6 +257,18 @@ test("leaves a rangeBar chart on a category axis alone", async ({ page }) => { | |
| expect(chart.shapes).toHaveLength(2); | ||
| }); | ||
|
|
||
| test("shows a data point link in its tooltip", async ({ page }) => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be worth to test multiple different kinds of charts since not all of them use the rows in the same way. claude is pointing out that there may be a code smell with pie charts |
||
| await renderChart(page, "link"); | ||
|
|
||
| await page.locator("#test-chart .apexcharts-rangebar-area").hover(); | ||
| const link = page.locator("#test-chart .apexcharts-tooltip a"); | ||
| await expect(link).toHaveText("Open link"); | ||
| await expect(link).toHaveAttribute( | ||
| "href", | ||
| "/workpackage_edit.sql?workpackage_name=Design", | ||
| ); | ||
| }); | ||
|
|
||
| test("leaves a treemap chart alone", async ({ page }) => { | ||
| const chart = await renderChart(page, "treemap"); | ||
|
|
||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when I go to
http://127.0.0.1:7071/component.sql?component=chartI see that there is an open link button, but the tooltip dissappears when I try to click it. also the link seems visually disabled