Add chart datapoint links via metadata tuples#1255

Open

lovasoa wants to merge 1 commit into

main from

codex/add-clickable-links-to-chart-datapoints

Open

Add chart datapoint links via metadata tuples #1255
lovasoa wants to merge 1 commit into
main from
codex/add-clickable-links-to-chart-datapoints

Conversation

@lovasoa

@lovasoa lovasoa commented Mar 19, 2026

Copy link

Copy Markdown

Collaborator

Motivation

Standardize per-point optional metadata into a compact object and add row-level click links on chart datapoints, removing the legacy numeric 4th-slot format.
Preserve existing payload size for points without metadata while enabling per-point z and link metadata for interactive navigation and documentation examples.

Description

Update template serialization in sqlpage/templates/chart.handlebars to emit either [series, x, y] or [series, x, y, {"z":..., "l":...}] and to avoid any numeric 4th-slot output.
Update runtime in sqlpage/apexcharts.js to parse 3-item tuples and 4th-slot object metadata, extract z and l, preserve existing time/range conversions, attach l to point objects, and build pieLinks for pie slices.
Preserve metadata in multi-series category alignment by changing align_categories to reuse full point objects rather than discarding extra fields.
Add click handling with dataPointSelection and click event hooks, helper functions resolvePointLink and navigateIfLink, and a short-lived guard to avoid double-triggered navigation.
Document the new per-row link parameter in examples/official-site/sqlpage/migrations/01_documentation.sql and add examples for linked bar, linked pie, and combined z+link points.
Add end-to-end tests in tests/end-to-end/official-site.spec.ts covering linked bar navigation, linked pie navigation, and no-op behavior for non-linked datapoints.

Testing

Ran frontend formatting with npm run format which completed successfully.
Ran the full Rust test suite with cargo test -q and all tests passed (136 passed, plus other test groups shown as ok).
Attempted to run targeted Playwright e2e tests but the Playwright runner failed in this environment due to a test-runner/package mismatch, so the new Playwright tests were added but could not be executed here.
Attempted a browser screenshot of the updated chart docs page but the browser container timed out, so visual verification was not captured in this environment.

Codex Task

@lovasoa


 Add chart datapoint links via metadata tuples

5670b69

@lovasoa lovasoa added the codex label

Mar 19, 2026

— with ChatGPT Codex Connector

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Mar 19, 2026

Copy link

Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

lovasoa

lovasoa commented

Mar 19, 2026

View reviewed changes

@lovasoa lovasoa left a comment

Copy link

Copy Markdown

Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex simplify code examples and tests, see comments below

sqlpage/apexcharts.js

Comment on lines +133 to +138

} else if (!warnedLegacyPointMetadata) {

warnedLegacyPointMetadata = true;

console.warn(

"Chart point metadata must be an object in the 4th tuple slot. Legacy formats are ignored.",

);

}

@lovasoa lovasoa Mar 19, 2026

Copy link

Copy Markdown

Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no warning just throw an error

@lovasoa lovasoa Mar 19, 2026

Copy link

Copy Markdown

Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codex no warning, just throw

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 19, 2026

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Updated chart point metadata parsing so a 4th tuple slot must be an object; otherwise the chart code now throws a TypeError immediately instead of logging a one-time warning and continuing. sqlpage/apexcharts.jsL121-L139
Removed the legacy warning-state bookkeeping because invalid metadata now fails fast on the offending datapoint. sqlpage/apexcharts.jsL121-L135
Committed the follow-up change on the current branch as bca97e7 and created a PR message titled Follow-up: throw on invalid chart metadata tuples.

Testing

✅ npm run format
✅ npm test

View task →

examples/official-site/sqlpage/migrations/01_documentation.sql

Comment on lines +696 to +712

('chart', 'A bar chart with clickable datapoints. Each row can set a `link` URL; clicking a datapoint opens that URL.', json('[

{"component":"chart", "type": "bar", "title": "Linked Sales", "labels": true},

{"label": "North", "value": 35, "link": "/documentation.sql?component=table"},

{"label": "South", "value": 22},

{"label": "West", "value": 41, "link": "/documentation.sql?component=map"}

]')),

('chart', 'A pie chart with per-slice links.', json('[

{"component":"chart", "title": "Linked Answers", "type": "pie", "labels": true},

{"label": "Yes", "value": 65, "link": "/documentation.sql?component=form"},

{"label": "No", "value": 35, "link": "/documentation.sql?component=table"}

]')),

('chart', 'A bubble chart demonstrating `z` and `link` metadata together on points.',

json('[

{"component":"chart", "title": "Bubbles with links", "type": "bubble", "ztitle": "Population", "marker": 8},

{"series": "Europe", "x": 2.1, "y": 18.5, "z": 742, "link": "/documentation.sql?component=chart"},

{"series": "Asia", "x": 5.2, "y": 24.1, "z": 4700, "link": "/documentation.sql?component=map"}

]')),

@lovasoa lovasoa Mar 19, 2026

Copy link

Copy Markdown

Collaborator Author

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

@lovasoa lovasoa Mar 19, 2026

Copy link

Copy Markdown

Collaborator Author

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

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 19, 2026

Copy link

Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Folded chart link usage into the existing Answers pie chart and Quarterly Results bar chart documentation examples, instead of keeping separate linked-example entries. examples/official-site/sqlpage/migrations/01_documentation.sqlL690-L696
Updated chart point parsing to throw a TypeError immediately when the 4th tuple slot is not an object, matching the review request to error instead of warn. sqlpage/apexcharts.jsL121-L135
Retargeted the end-to-end coverage to those existing chart examples and disambiguated the duplicate Quarterly Results headings with exact role matching so the updated tests pass. tests/end-to-end/official-site.spec.tsL27-L60
Committed the follow-up changes on the current branch as 2adc7e1 and 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 (from examples/official-site)
✅ npx playwright install chromium (from tests/end-to-end)
✅ npx playwright install-deps chromium (from tests/end-to-end)
✅ npm run test -- official-site.spec.ts -g 'chart point links|chart links - no-link datapoint' (from tests/end-to-end)
⚠️ Screenshot not captured because the browser screenshot tool was not available in this environment.

View task →

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Mar 19, 2026

Copy link

Copy Markdown

On it

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Mar 19, 2026

Copy link

Copy Markdown

Summary

Changed chart metadata parsing to throw a TypeError immediately when the 4th tuple slot is not an object, matching the review request to error instead of warning. sqlpage/apexcharts.jsL121-L135
Simplified the chart documentation examples by folding link usage into the existing Answers pie chart and Quarterly Results bar chart, instead of keeping separate linked-example entries. examples/official-site/sqlpage/migrations/01_documentation.sqlL690-L696
Updated the Playwright coverage to target those existing chart examples and to disambiguate duplicate cards on the documentation page while keeping the linked-bar, linked-pie, and no-link checks. tests/end-to-end/official-site.spec.tsL27-L67

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
⚠️ npm test (fails due to a pre-existing unrelated Biome formatting issue in examples/telemetry/grafana/sqlpage-home.json, which I did not modify)
✅ cargo run (from examples/official-site)
✅ npm run test -- official-site.spec.ts -g 'chart point links|chart links - no-link datapoint'

Committed on the current branch as 8eccd09, and I created the follow-up PR message summarizing these review-driven changes.

View task →

Labels

codex

1 participant

@lovasoa

Uh oh!

Conversation

@lovasoa lovasoa commented Mar 19, 2026

Motivation

Description

Testing

Uh oh!

chatgpt-codex-connector Bot commented Mar 19, 2026

Uh oh!

@lovasoa lovasoa left a comment

Choose a reason for hiding this comment

Uh oh!

@lovasoa lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

@lovasoa lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

@lovasoa lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

@lovasoa lovasoa Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 19, 2026

Choose a reason for hiding this comment

Summary

Uh oh!

chatgpt-codex-connector Bot commented Mar 19, 2026

Uh oh!

chatgpt-codex-connector Bot commented Mar 19, 2026

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant