-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add vega embed component and use for measles whitepaper #980
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
Conversation
|
Thx so much @dannon what needs to be done to get this out of draft? |
|
@NoopDog I wanted to give it a second look through but haven't had the time -- if it's good enough I'm happy to merge as-is and iterate. There weren't any glaring issues I wanted to follow up on. |
- Add VegaEmbed React component for interactive Vega-Lite visualizations - Add [email protected], [email protected], [email protected] dependencies - Component supports URL specs or inline objects with error handling - Use PALETTE.WARNING_MAIN for error text color - Import from vega-embed/build for compatibility with current tsconfig
Replace static PNG and external link with inline interactive visualization
Add custom type declaration to resolve vega-embed imports with current moduleResolution setting
- Add nucleotide-changes-vega-spec.json to measles whitepaper assets - Update VegaEmbed to use local spec file instead of GitHub Gist URL
- Add max-width: 100% and overflow-x: auto to prevent overflow into navigation - Reduce figcaption margin-top from 32px/52px to 16px to reduce whitespace
Configure vega-embed with autosize fit-x to make visualizations responsive to container width while maintaining aspect ratio. This prevents overflow and reduces excessive whitespace.
|
Following up from the call -- primary followup would be adjusting the vega spec to set the initial coordinates and width a bit better, and potentially adjusting the legend (or clipping it), but neither of those would be barriers to merging this and iterating. |
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.
Pull Request Overview
This PR adds a new VegaEmbed component to support interactive Vega-Lite visualizations in the application. The component is used to replace a static image with an interactive visualization in the measles whitepaper documentation.
Key changes:
- Added vega, vega-embed, and vega-lite packages as dependencies
- Created a new VegaEmbed React component with support for loading specs from URLs or objects
- Integrated the component into MDX rendering and updated the measles whitepaper to use interactive visualizations
Reviewed Changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vega-embed.d.ts | TypeScript declaration module for vega-embed package |
| package.json | Added vega visualization library dependencies |
| package-lock.json | Locked dependency versions for vega packages and transitive dependencies |
| app/components/common/VegaEmbed/vegaEmbed.tsx | Main component implementation with spec loading and embedding logic |
| app/components/common/VegaEmbed/vegaEmbed.styles.ts | Styled components for the VegaEmbed container |
| app/components/common/VegaEmbed/index.ts | Component exports |
| app/docs/common/mdx/constants.ts | Registered VegaEmbed component for MDX usage |
| app/docs/learn/featured-analyses/evolutionary-dynamics-of-coding-overlaps-in-measles.mdx | Updated to use VegaEmbed instead of static Figure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| const loadSpec = async (): Promise<void> => { | ||
| if (!containerRef.current) return; | ||
|
|
||
| try { | ||
| let vegaSpec: VisualizationSpec; | ||
|
|
||
| // If spec is a string (URL), fetch it | ||
| if (typeof spec === "string") { | ||
| const response = await fetch(spec); | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch spec: ${response.statusText}`); | ||
| } | ||
| vegaSpec = await response.json(); | ||
| } else { | ||
| vegaSpec = spec; | ||
| } | ||
|
|
||
| // Embed the visualization with responsive sizing | ||
| await embed(containerRef.current, vegaSpec, { | ||
| actions: { | ||
| compiled: false, | ||
| editor: false, | ||
| export: true, | ||
| source: false, | ||
| }, | ||
| config: { | ||
| autosize: { | ||
| contains: "padding", | ||
| type: "fit-x", | ||
| }, | ||
| }, | ||
| }); | ||
| setError(null); | ||
| } catch (err) { | ||
| const errorMessage = | ||
| err instanceof Error ? err.message : "Unknown error"; | ||
| setError(errorMessage); | ||
| console.error("Error loading Vega-Lite spec:", err); | ||
| } | ||
| }; | ||
|
|
||
| loadSpec(); | ||
| }, [spec]); |
Copilot
AI
Nov 10, 2025
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.
The useEffect hook should return a cleanup function to properly dispose of the vega-embed visualization. The embed function returns a Result object with a finalize() method that should be called when the component unmounts or when the spec changes. Without this cleanup, memory leaks and stale visualizations may occur. Store the result and call result.finalize() in the cleanup function.
This follows the conventional commit format and describes the fix clearly. The cleanup function properly disposes of the vega-embed visualization when the component unmounts or the spec changes.
frano-m
left a comment
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.
Hi @dannon would you mind checking if the following warnings can be resolved / ignored:
Otherwise, LGTM! Very cool feature!
Calculate actual data extent from datasets and set explicit x-axis domain with 5% padding to prevent Vega from adding excessive padding. This ensures data points are properly centered when using fit-x responsive sizing. Refactored domain calculation into separate helper functions to reduce cognitive complexity.
Update the schema URL to use Vega-Lite v6 to match the installed vega-lite package version (6.4.1). The spec is backward compatible and renders correctly with no changes needed.
Implements #979
Need to make one cleanup pass, and we may want to adjust the vega spec to fit the expected dimensions better (I can elaborate), but this gets the job done: