Skip to content

refactor: share viz footer assets across R and Python#227

Merged
cpsievert merged 10 commits intofeat/ggsql-integration-r-v2from
feat/shared-viz-assets
May 1, 2026
Merged

refactor: share viz footer assets across R and Python#227
cpsievert merged 10 commits intofeat/ggsql-integration-r-v2from
feat/shared-viz-assets

Conversation

@cpsievert
Copy link
Copy Markdown
Contributor

@cpsievert cpsievert commented May 1, 2026

Stacked on #224.

Summary

Centralize the visualization footer assets under js/ and generate the package-local artifacts for both runtimes from a single shared source.

This branch now does four concrete things:

  • adds a minimal shared web asset toolchain under js/ and copies generated viz.js / viz.css into pkg-r and pkg-py
  • extracts the common footer behavior into viz-core.ts and makes web-build atomic while making web-check fail on stale generated assets
  • unifies the runtime contract so both R and Python write the final DOM id into the footer and both export through Vega Embed's action links
  • trims diff and runtime noise by preserving the original CSS ordering and simplifying footer click handling to a single action lookup plus tracked open-menu state

The browser-side result is that the shared logic now owns:

  • query toggle behavior
  • save menu open/close behavior
  • outside-click closing
  • PNG/SVG export through a shared Vega action-link path

The runtime-specific surface is much smaller than where this branch started.

Verification

  • make web-build
  • make web-check
  • Rscript -e "devtools::test('pkg-r', filter = '^(viz-tool|querychat-viz)$', reporter = 'summary')"
  • uv run pytest pkg-py/tests/test_viz_footer.py -v
  • uv run pytest pkg-py/tests/playwright/test_11_viz_footer.py -k 'click_save_opens_menu or click_outside_closes_menu or toggle_save_menu' -v
  • uv run pytest pkg-py/tests/playwright/test_11_viz_footer.py -k 'save_as_png_downloads_png or save_as_svg_downloads_svg' -v
  • Live R module repro via Playwright confirmed namespaced-id save menu behavior and successful Repro Chart.png / Repro Chart.svg downloads

@cpsievert cpsievert force-pushed the feat/shared-viz-assets branch from 1192362 to 6beedc1 Compare May 1, 2026 15:39
@cpsievert cpsievert requested a review from Copilot May 1, 2026 16:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Centralizes the visualization footer JS/CSS into a shared js/ source, then generates package-local artifacts for both the R and Python runtimes. This reduces divergence between implementations and adds stronger guarantees that committed web assets match the sources.

Changes:

  • Introduce shared js/src/viz-core.ts + thin runtime entrypoints and generate viz.js/viz.css into pkg-r and pkg-py.
  • Update R/Python footer markup to use data-querychat-action and resolve Shiny DOM widget IDs for export behavior.
  • Add web-build/web-check targets and Playwright coverage for “Save as PNG/SVG” downloads in Python.

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg-r/inst/htmldep/viz.js Replace inline footer logic with generated bundle sourced from shared viz-core.
pkg-r/inst/htmldep/viz.css Add generated-file header; CSS now sourced from shared js/src/viz.css.
pkg-r/R/querychat_viz.R Add data-querychat-action attributes and ensure DOM widget ID is passed for exports.
pkg-py/src/querychat/static/js/viz.js Replace runtime-specific implementation with generated bundle sourced from shared viz-core.
pkg-py/src/querychat/static/css/viz.css Add generated-file header; keep in sync with shared CSS source.
pkg-py/src/querychat/_viz_tools.py Resolve Shiny DOM widget IDs via resolve_id; add data-querychat-action attributes in footer markup.
pkg-py/tests/test_viz_footer.py Add unit tests asserting resolved DOM widget IDs are used in rendered footer markup.
pkg-py/tests/playwright/test_11_viz_footer.py Add real download assertions for PNG/SVG export via the Save menu.
js/src/viz-core.ts New shared footer event delegation + export adapter interface.
js/src/viz-r.ts New R entrypoint wiring shared core + Vega action adapter.
js/src/viz-py.ts New Python entrypoint wiring shared core + Vega action adapter.
js/src/viz.css New shared CSS source for viz container/footer styling.
js/build.mjs New build pipeline to stage and copy generated assets into R/Python package locations.
js/check.mjs New “stale output” check to fail when committed generated assets diverge from sources.
js/package.json New private JS workspace for shared asset build/check scripts.
js/package-lock.json Lockfile for shared JS tooling (esbuild, typescript).
js/tsconfig.json New TS config for typechecking shared sources (no emit).
Makefile Add web-setup, web-build, web-check targets for shared asset workflows.
.gitignore Ignore js/node_modules/.
Files not reviewed (1)
  • js/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cpsievert cpsievert marked this pull request as ready for review May 1, 2026 16:22
@cpsievert cpsievert merged commit 1dc8701 into feat/ggsql-integration-r-v2 May 1, 2026
8 of 27 checks passed
@cpsievert cpsievert deleted the feat/shared-viz-assets branch May 1, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants