Add chart datapoint links via metadata tuples #1255
+133
−11
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,7 +36,7 @@ sqlpage_chart = (() => { | |
| ); | ||
| const isDarkTheme = document.body?.dataset?.bsTheme === "dark"; | ||
|
|
||
| /** @typedef { { [name:string]: {data:{x:number|string|Date,y:number,z?:number|string,l?:string}[], name:string} } } Series */ | ||
|
|
||
| /** | ||
| * Aligns series data points by their x-axis categories, ensuring all series have data points | ||
|
|
@@ -78,19 +78,37 @@ sqlpage_chart = (() => { | |
| series_idxs.splice(series_idxs.indexOf(idx_of_xmin), 1); | ||
| } | ||
| } | ||
| // Create a map of category -> point for each series and rebuild | ||
| return series.map((s) => { | ||
| const valueMap = new Map(s.data.map((point) => [point.x, point])); | ||
| return { | ||
| name: s.name, | ||
| data: Array.from( | ||
| categoriesSet, | ||
| (category) => | ||
| valueMap.get(category) || { | ||
| x: category, | ||
| y: 0, | ||
| }, | ||
| ), | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| function resolvePointLink(data, opts, pieLinks) { | ||
| if (opts.dataPointIndex == null || opts.dataPointIndex < 0) | ||
| return undefined; | ||
| if (data.type === "pie") return pieLinks[opts.dataPointIndex]; | ||
| if (opts.seriesIndex == null || opts.seriesIndex < 0) return undefined; | ||
| const series = opts.w?.config?.series?.[opts.seriesIndex]; | ||
| const point = series?.data?.[opts.dataPointIndex]; | ||
| return point?.l; | ||
| } | ||
|
|
||
| function navigateIfLink(link) { | ||
| if (typeof link === "string" && link) window.location.href = link; | ||
| } | ||
|
|
||
| /** @param {HTMLElement} c */ | ||
| function build_sqlpage_chart(c) { | ||
| const [data_element] = c.getElementsByTagName("data"); | ||
|
|
@@ -100,7 +118,27 @@ sqlpage_chart = (() => { | |
| const is_timeseries = !!data.time; | ||
| /** @type { Series } */ | ||
| const series_map = {}; | ||
| const pieLinks = []; | ||
| let warnedLegacyPointMetadata = false; | ||
| for (const point of data.points) { | ||
| const [name, old_x, old_y] = point; | ||
| let meta; | ||
| if (point.length === 4) { | ||
| if ( | ||
| typeof point[3] === "object" && | ||
| point[3] != null && | ||
| !Array.isArray(point[3]) | ||
| ) { | ||
| meta = point[3]; | ||
| } else if (!warnedLegacyPointMetadata) { | ||
| warnedLegacyPointMetadata = true; | ||
| console.warn( | ||
| "Chart point metadata must be an object in the 4th tuple slot. Legacy formats are ignored.", | ||
| ); | ||
| } | ||
|
Comment on lines
+133
to
+138
Collaborator
Author
There was a problem hiding this comment. no warning just throw an error There was a problem hiding this comment. Summary
Testing
|
||
| } | ||
| const z = meta?.z; | ||
| const link = meta?.l; | ||
| series_map[name] = series_map[name] || { name, data: [] }; | ||
| let x = old_x; | ||
| let y = old_y; | ||
|
|
@@ -110,7 +148,11 @@ sqlpage_chart = (() => { | |
| y = y.map((y) => new Date(y).getTime()); | ||
| else x = new Date(x); | ||
| } | ||
| const seriesPoint = { x, y }; | ||
| if (z != null) seriesPoint.z = z; | ||
| if (link != null) seriesPoint.l = link; | ||
| series_map[name].data.push(seriesPoint); | ||
| pieLinks.push(link); | ||
| } | ||
| if (data.xmin == null) data.xmin = undefined; | ||
| if (data.xmax == null) data.xmax = undefined; | ||
|
|
@@ -135,6 +177,16 @@ sqlpage_chart = (() => { | |
| series = align_categories(series); | ||
|
|
||
| const chart_type = data.type || "line"; | ||
| let skipNextChartClick = false; | ||
| const onDataPointInteraction = (_event, _chartContext, opts) => { | ||
| const link = resolvePointLink(data, opts, pieLinks); | ||
| if (!link) return; | ||
| skipNextChartClick = true; | ||
| navigateIfLink(link); | ||
| setTimeout(() => { | ||
| skipNextChartClick = false; | ||
| }, 0); | ||
| }; | ||
| const options = { | ||
| chart: { | ||
| type: chart_type, | ||
|
|
@@ -151,6 +203,13 @@ sqlpage_chart = (() => { | |
| zoom: { | ||
| enabled: false, | ||
| }, | ||
| events: { | ||
| dataPointSelection: onDataPointInteraction, | ||
| click: (event, chartContext, opts) => { | ||
| if (skipNextChartClick) return; | ||
| onDataPointInteraction(event, chartContext, opts); | ||
| }, | ||
| }, | ||
| }, | ||
| theme: { | ||
| palette: "palette4", | ||
|
|
||
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
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
Oops, something went wrong.
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.
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.
Instead of adding new examples, add links to the existing examples where it makes the most sense. Update the tests accordingly and run them
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.
@codex Instead of adding new examples, add links to the existing examples where it makes the most sense. Update the tests accordingly and run them
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.
Summary
linkusage into the existingAnswerspie chart andQuarterly Resultsbar chart documentation examples, instead of keeping separate linked-example entries. examples/official-site/sqlpage/migrations/01_documentation.sqlL690-L696TypeErrorimmediately when the 4th tuple slot is not an object, matching the review request to error instead of warn. sqlpage/apexcharts.jsL121-L135Quarterly Resultsheadings with exact role matching so the updated tests pass. tests/end-to-end/official-site.spec.tsL27-L602adc7e1and recorded a follow-up PR title/body with the requested summary.Testing
✅
npx biome format --write sqlpage/apexcharts.js tests/end-to-end/official-site.spec.ts✅
npx biome check sqlpage/apexcharts.js tests/end-to-end/official-site.spec.ts✅
cargo run(fromexamples/official-site)✅
npx playwright install chromium(fromtests/end-to-end)✅
npx playwright install-deps chromium(fromtests/end-to-end)✅
npm run test -- official-site.spec.ts -g 'chart point links|chart links - no-link datapoint'(fromtests/end-to-end)View task →