-
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
Improve visibility of filtered samples under various conditions #1368
Comments
Picking this up after prioritizing in https://github.com/nextstrain/private/issues/129 and chatting with @jameshadfield the other day. First setting some terminology:
With these definitions, #1373 is about focus, not zoom. I've updated that PR with a separate "focus" button to distinguish the two. This issue as described is more about improving zoom, not focus. So I don't think it should be closed by that PR. Changing focus on the y-axis (of the rectangular tree view) is trivial because there is no labeled meaning to the y scale. So adding more gap between the focal tips and shrinking the gap between non-focal tips is trivial. This is what #1373 does currently. Changing focus on the x-axis is less trivial because the x scale is currently a linear representation of time or divergence. It may be possible, but more thought would have to be put into how to make this user-friendly. Some open questions:
|
The zoom/focus split reflects the technical implementation details, but I'm not sure the distinction is totally obvious to the user. It feels like the zoom/focus split should be completely behind the scenes. If the MRCA is not the root, then zoom + focus. If the MRCA already to the root, then just focus.
Yes, I think it's still helpful with large trees. |
Yes. Using your terminology I'm not sure that x focus will be a good idea, but I think there's a good zoom implementation available. |
Thanks for the answers! I have a better understanding now after fixing up the focus implementation in #1373 – adding my own answers:
Yes, at least for now. The focus implementation in #1373 has notable differences from zoom:
We could consider adding an "auto-zoom" toggle in the sidebar. If implemented, it might make sense to merge the two toggles.
Yes, and let's not plan to pursue x focus. The next step in this direction is to implement x zoom. |
Context
The implementation of a "Zoom to Selected" button on the tree has been really useful. I'll often filter to say clade 21A and then be able to just click to zoom in properly.
However, there are a number of edge cases where "Zoom to Selected" isn't very helpful. For a simple example, if we filter to early samples (https://nextstrain.org/ncov/global?dmax=2020-02-01) there's no way to zoom into these (the button is deactivated). Or if we filter to clade 19A viruses (https://nextstrain.org/ncov/global?f_clade_membership=19A) there's also no way to zoom in.
Description
We could at some point contemplate a fancier "accordion" view which would help to emphasize situations where multiple blocks of tips are selected, but before this we should get basic bounding box working (the accordion view would require updating node y-values and will necessarily be more involved).
Instead, I'd recommend when clicking "Zoom to Selected" to calculate the maximum x and y-values and the minimum x and y-value out of all visible nodes. Use these values to define the bounding box to zoom to.
cc @joverlee521 who implemented the initial version of zoom to selected in #1257. This PR contains discussion of the implementation challenge of this bounding box design.
This implementation would also resolve #1360.
Related issues
The text was updated successfully, but these errors were encountered: