-
Notifications
You must be signed in to change notification settings - Fork 162
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
base: master
Are you sure you want to change the base?
Conversation
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.
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. |
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. |
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.
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) { |
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.
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.
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.
Yes! It should. Thanks :)
Any progress here @jameshadfield? Would be cool to have in production :) |
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.