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

Skyline: follow replicate in graphs #2766

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

meowcat
Copy link

@meowcat meowcat commented Nov 1, 2023

This PR adds functionality that forces the currently selected replicate into X axis range on the SummaryReplicateGraphPanes. Previously, a user could select a replicate (by ctrl-left/right or by selecting from results grid/dropdown) that is outside the current zoom range on Peak Area, Retention Time, or Mass Errors. Now, the X axis will adapt to include the selected replicate, but keeping the range (i.e., if it went from 6-10 and replicate 4 is newly selected, it will adapt to 4-8.)

It works with GroupBy Replicate/Sampletype/Conc and with Order Document/Acquired Time, though only the basic case Replicate/Document has a testcase currently. It works independently from (ie. with or without) Sync Zooming. If desired, I can make it work on Sync Zooming only.

The test currently reuses the data from MultiSelectPeakAreaGraphTest.

For my original feature request/description, see https://skyline.ms/announcements/home/support/thread.view?rowId=62047

@meowcat meowcat changed the title Skyline/work/20231013 follow replicate in graphs Skyline: follow replicate in graphs Nov 1, 2023
@meowcat
Copy link
Author

meowcat commented Nov 1, 2023

I am noticing that the executing logic https://github.com/ProteoWizard/pwiz/pull/2766/files#diff-c05006f567abb8ecca0cf14c9e7ab3abf42b632da08ecf93ef861dcd3848c016R4048-R4058 could be moved to SummaryReplicateGraphPane, and then SelectedIndex wouldn't need to be made public.

@nickshulman
Copy link
Contributor

I wonder if you should add a method to the class "GraphPane" (in the "ZedGraph" project) called "ScrollIntoView which takes a Rectangle (or maybe an x-coordinate range) as a parameter.
When the user zooms in on or scrolls a graph using the mouse, they are able to undo that operation by right-clicking on the graph and choosing "Unzoom". I am not sure exactly how this auto-scrolling because the selected replicate changed should interact with the unzoom feature, but it is something to think about.

@meowcat
Copy link
Author

meowcat commented Nov 2, 2023

Thanks for the initial feedback,

When the user zooms in on or scrolls a graph using the mouse, they are able to undo that operation by right-clicking on the graph and choosing "Unzoom".

How does "scrolling a graph with the mouse" work? As far as I have managed, in Skyline 23.1 I can only

  1. Zoom in or out of X, Y axes indivdually, by using the scroll wheel while pointing on an axis
  2. Zoom in or out of X and Y simultaneously, by using the scroll wheel while pointing on the graph canvas
    (Note that when zooming using the mouse wheel, only the last tick of wheel scrolling is undone by "un-zoom")
  3. Zoom to a selected rectangle.

Is there any other mouse action that will scroll the axes?

Another behaviour I discovered now in Skyline 23.1:
When being in "zoomed in" state, selecting a different transition will reset the zoom to full range. "Un-zoom" will actually restore the zoom setting from the previous transition, on both X and Y axis, so it will set the zoom to a retention time or peak intensity range irrelevant to this new transition. Even "undo all zoom/pan" does not reset to full range, it is not yet clear to me what reference state it goes back to.

In that light I would argue that un-zoom is not very meaningful for such cases... (I would even argue that selecting a different transition should only reset the Y axis, not the X axis; and that "undo all zoom/pan" should always reset to the full X range and a meaningful Y range for the selected data. I could potentially look into that.)

I wonder if you should add a method to the class "GraphPane" (in the "ZedGraph" project) called "ScrollIntoView which takes a Rectangle (or maybe an x-coordinate range) as a parameter.

Hm, there are different places in the code that already interact with XAxis and YAxis (I'm sure I haven't found all of them yet); would your goal be to unify behaviour in all of them?

@nickshulman
Copy link
Contributor

You can scroll the graph by holding down the Ctrl key and dragging the mouse across the graph.
I just noticed that if you scroll the graph in that way, a new menu item appears on the graph "Undo Pan".
I believe that "Undo all zoom/pan" takes you back to the way the graph was before the user started zooming and panning using the mouse. That is, the axis ranges end up being reset back to what Skyline set them to programmatically. However, I might be wrong about that.
The class which is responsible for remembering what the zoom state was is called "ZoomStateStack" in the ZedGraph project.

@meowcat
Copy link
Author

meowcat commented Nov 2, 2023

You can scroll the graph by holding down the Ctrl key and dragging the mouse across the graph.

Something new every day! I thought I tried that.

Hm, one option could be

  • to add a Pan entry to the pane.ZoomStack when panning due to replicate switching
  • to call this feature "Auto-Pan" and make it toggleable in the context menu, just like "Synchronize Zooming".

@meowcat
Copy link
Author

meowcat commented Nov 15, 2023

Hi,
should I continue working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants