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

[tree] button to go back to previous zoom #1403

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jameshadfield
Copy link
Member

The ability to return to the previous zoom level is useful when
exploring trees in detail, especially when combined with the
magnifying glass (which zooms out).

A hover info box has been added to convey the functionality of this
icon.

The implementation involving multiple trees is conceptually more
complicated, as I don't want us to return to the previous zoom state
in each tree. (This is similar to the magnifiying glass icon, which
zooms out in both trees.) The best solution is probably tree-specific
UI. For now, this functionality is only surfaced if we are viewing a
single tree.

The ability to return to the previous zoom level is useful when
exploring trees in detail, especially when combined with the
magnifying glass (which zooms out).

A hover info box has been added to convey the functionality of this
icon.

The implementation involving multiple trees is conceptually more
complicated, as I don't want us to return to the previous zoom state
in each tree. (This is similar to the magnifiying glass icon, which
zooms out in both trees.) The best solution is probably tree-specific
UI. For now, this functionality is only surfaced if we are viewing a
single tree.
@jameshadfield jameshadfield temporarily deployed to auspice-go-back-tree-c97olev7b September 21, 2021 04:31 Inactive
@trvrb
Copy link
Member

trvrb commented Sep 21, 2021

Hmm... very interesting idea. However, I have to admit, I'd prefer it if the browser back button could just be a general purpose UI to go back to previous visualization state (whether it's tree zoom or tree layout or whatever). Although, given that tree zoom is not usually encoded in URL, I see how this is difficult.

I do think that the previous idea of tagging branches in a dataset specific fashion (#1036) could interact well here. However, in the interim I'd support this small "go back" icon.

Also, the hover tooltip is really helpful. It would be nice if this existed for the other top-right panel buttons.

@corneliusroemer
Copy link
Member

This is really cool! Love it.

One thing I'm not sure about: should zoom out via lens count as a step that is reversed by the back button? Since it's easy to undo zoom out by just clicking into the branch and it feels that zooming out goes in the same direction as the back button I'd potentially prefer go back to the state where I was before I clicked into a specific branch. Does that make sense? It's a small detail, not very important but maybe others have the same thought.

Also: since it's sometimes quite hard to find the right branch to click into, would it make sense to add a forward button that goes back in to where we started off before clicking the back button? Again, less important than back button but maybe something to think about.

Copy link
Contributor

@joverlee521 joverlee521 left a comment

Choose a reason for hiding this comment

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

This is great! I agree that this function seems like it should work separately for each tree if there are two trees. I think the biggest question there is where those buttons should go on the UI 🙃

I could also see a use for the suggested "forward" button, but maybe we can come back to that when zooming is added to the URL.

If I'm not misunderstanding the history stack, I think I found a small bug. See comment below.

@@ -111,6 +124,15 @@ export const updateVisibleTipsAndBranchThicknesses = (
visibilityToo: dispatchObj.visibilityToo
});

/* update the history stack for zoom levels (if we are zooming) */
if (tree.idxOfFilteredRoot !== dispatchObj.idxOfInViewRootNode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this line be

if (tree.idxOfInViewRootNode !== dispatchObj.idxOfInViewRootNode) {

Currently the revert button doesn't always work after clicking the "zoom to selected" button and I believe this is the cause.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! It should. Thanks :)

@corneliusroemer
Copy link
Member

Any progress here @jameshadfield? Would be cool to have in production :)

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.

[Feature request] Allow browser back button to reverse state changes
4 participants