-
Notifications
You must be signed in to change notification settings - Fork 1
Enable Chart.js zoom plugin and drag-zoom on completeness/contamination chart #69
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
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 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.
app/templates/isolate_stats.html
Outdated
| const chartPlugins = zoomPlugin ? [zoomPlugin] : []; | ||
| new Chart(context, { | ||
| type: 'scatter', | ||
| data: { datasets }, | ||
| plugins: chartPlugins, |
Copilot
AI
Jan 16, 2026
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 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.
| const chartPlugins = zoomPlugin ? [zoomPlugin] : []; | |
| new Chart(context, { | |
| type: 'scatter', | |
| data: { datasets }, | |
| plugins: chartPlugins, | |
| new Chart(context, { | |
| type: 'scatter', | |
| data: { datasets }, |
app/templates/isolate_stats.html
Outdated
| window.ChartZoom || | ||
| window.ChartZoom?.default || |
Copilot
AI
Jan 16, 2026
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 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.
| window.ChartZoom || | |
| window.ChartZoom?.default || | |
| window.ChartZoom?.default || | |
| window.ChartZoom || |
Motivation
Description
app/templates/isolate_stats.htmlto look for the zoom plugin across multiple global names, callChart.register(zoomPlugin)when found, and attach the plugin to the scatter chart via thepluginsarray while enablingdragzoom.Testing
black .which completed successfully and left files unchanged where applicable.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