Skip to content

Conversation

@Ulthran
Copy link
Contributor

@Ulthran Ulthran commented Jan 16, 2026

Motivation

  • Make the completeness vs contamination scatter plot respond to zoom/pan interactions by reliably registering the Chart.js zoom plugin and enabling drag-zoom when available.

Description

  • Update app/templates/isolate_stats.html to look for the zoom plugin across multiple global names, call Chart.register(zoomPlugin) when found, and attach the plugin to the scatter chart via the plugins array while enabling drag zoom.

Testing

  • Ran black . which completed successfully and left files unchanged where applicable.
  • Ran pytest tests/ which executed and produced 3 passing and 6 failing tests, with failures caused by missing SQLite tables in the test environment (OperationalError: no such table).

Codex Task

Copilot AI review requested due to automatic review settings January 16, 2026 20:29
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 enables zoom and pan functionality on the completeness vs contamination scatter plot by integrating the Chart.js zoom plugin. The changes detect the zoom plugin from multiple global variable names, register it with Chart.js, and configure drag-to-zoom along with the existing wheel and pinch zoom options.

Changes:

  • Added multi-name fallback detection for the Chart.js zoom plugin to handle different module export patterns
  • Enabled drag-to-zoom functionality in addition to existing wheel and pinch zoom options
  • Attempted to pass zoom plugin reference to the scatter chart constructor (though this is redundant with global registration)

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

Comment on lines 303 to 307
const chartPlugins = zoomPlugin ? [zoomPlugin] : [];
new Chart(context, {
type: 'scatter',
data: { datasets },
plugins: chartPlugins,
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The plugins array should not be used to pass the zoom plugin to the Chart constructor. Chart.js plugins are registered globally via Chart.register() (which is already done on line 97), not through the chart instance's plugins array. The plugins property in the chart constructor is intended for inline plugin definitions or plugin instances with specific configurations, not for registered plugins. Remove lines 303 and 307 as the zoom plugin will be automatically available to all charts after registration.

Suggested change
const chartPlugins = zoomPlugin ? [zoomPlugin] : [];
new Chart(context, {
type: 'scatter',
data: { datasets },
plugins: chartPlugins,
new Chart(context, {
type: 'scatter',
data: { datasets },

Copilot uses AI. Check for mistakes.
Comment on lines 91 to 92
window.ChartZoom ||
window.ChartZoom?.default ||
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The plugin detection order is incorrect. If window.ChartZoom is an object containing a default property with the actual plugin, the current code will return the object itself (line 91) instead of the plugin (window.ChartZoom.default on line 92). The order should check for window.ChartZoom?.default before window.ChartZoom to properly handle module exports where the plugin is in a default property.

Suggested change
window.ChartZoom ||
window.ChartZoom?.default ||
window.ChartZoom?.default ||
window.ChartZoom ||

Copilot uses AI. Check for mistakes.
@Ulthran Ulthran merged commit 890a93a into main Jan 16, 2026
1 of 5 checks passed
@Ulthran Ulthran deleted the codex/fix-zoom-feature-on-completeness-plot branch January 16, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants