Skip to content
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

Add ability to zoom and pan plots when zoomed #4629

Merged
merged 1 commit into from
Sep 7, 2023

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Sep 7, 2023

Closes #4530
Related to iterative/dvc-render#7

This PR adds the ability to zoom (using the mouse wheel) and pan plots when they are focused. I have limited the change to only focused because it mitigates the unexpected behaviour of the plot being zoomed when the user is trying to scroll between plots.

Types covered by this change?:

  1. Scatter plots
  2. Smooth plots
  3. Simple plots
  4. Horizontal bar plots
  5. Custom plots

What's not covered:

  • Confusion matrices (and probably other multiview plots)

Demo

Screen.Recording.2023-09-07.at.11.49.54.am.mov

Note: has the potential to break custom templates.

@mattseddon mattseddon added the product PR that affects product label Sep 7, 2023
@mattseddon mattseddon self-assigned this Sep 7, 2023
@mattseddon mattseddon marked this pull request as ready for review September 7, 2023 01:23
const hasSmoothing =
isTemplatePlot &&
(props.spec as { params?: { name: string }[] }).params?.[0]?.name ===
'smooth'
Copy link
Member Author

Choose a reason for hiding this comment

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

[C] This is pretty brittle but will do for now. Will discuss at the next cross-team whether or not we want to push this back in to dvc-render.

Copy link
Member Author

Choose a reason for hiding this comment

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

worst case scenario - if the spec changes then makePlotZoomOnWheel will fall through to the default. The zoom/pan behaviour won't work but the template will still render.

Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

Great work!

@shcheklein
Copy link
Member

Looks great! Let go for it (and can we add it to Studio?)

QQ - are there other ways to control it? A square box selection? (can be then easier to enable on the main dashboard?)

@mattseddon
Copy link
Member Author

Looks great! Let go for it (and can we add it to Studio?)

Do you want me to add this to Studio?

QQ - are there other ways to control it? A square box selection? (can be then easier to enable on the main dashboard?)

I have been through the docs and this seems like the only option without trying to directly access d3 (like tensorboard does) so there is not a simple way to "do the same thing as tensorboard". Brush zoom looks like this or this (note v4) as usual there is an open issue vega/vega-lite#4742.

@codeclimate
Copy link

codeclimate bot commented Sep 7, 2023

Code Climate has analyzed commit 8c33835 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 95.0% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit fdf3ae7 into main Sep 7, 2023
3 checks passed
@mattseddon mattseddon deleted the add-plot-zoom-and-pan branch September 7, 2023 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plots: Ability to zoom and maybe pan
3 participants