Skip to content

Conversation

@dannon
Copy link
Member

@dannon dannon commented Nov 5, 2025

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:

image

@dannon dannon changed the title Vega embed feat: add vega embed component and use for measles whitepaper Nov 5, 2025
@NoopDog
Copy link
Collaborator

NoopDog commented Nov 10, 2025

Thx so much @dannon what needs to be done to get this out of draft?

@dannon
Copy link
Member Author

dannon commented Nov 10, 2025

@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.

@dannon dannon marked this pull request as ready for review November 10, 2025 17:28
@github-actions github-actions bot added the feat label Nov 10, 2025
- 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.
@dannon
Copy link
Member Author

dannon commented Nov 10, 2025

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.

@NoopDog NoopDog requested review from Copilot and frano-m November 10, 2025 21:47
Copilot finished reviewing on behalf of NoopDog November 10, 2025 21:48
Copy link
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

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.

Comment on lines 24 to 67
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]);
Copy link

Copilot AI Nov 10, 2025

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.

Copilot uses AI. Check for mistakes.
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 frano-m changed the title feat: add vega embed component and use for measles whitepaper feat: add vega embed component and use for measles whitepaper (#979) Nov 11, 2025
Copy link
Contributor

@frano-m frano-m left a 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:

Image

Otherwise, LGTM! Very cool feature!

@frano-m frano-m changed the title feat: add vega embed component and use for measles whitepaper (#979) feat: add vega embed component and use for measles whitepaper Nov 11, 2025
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.
@dannon dannon merged commit f87f7e3 into galaxyproject:main Nov 13, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants