auspice
auspice copied to clipboard
Deselecting a single strain doesn't remove the info panel overlay
Current Behavior
With new filters behavior (implemented in #1200) clicking on a single tip in the tree adds this single tip as a "filter" and covers tree with gray info panel overlay, like so:
![tip-selected](https://user-images.githubusercontent.com/1176109/101833280-98ca7f80-3aed-11eb-99d0-ab1124d6d9dd.png)
This behavior is as intended. Once in this state you can click outside the overlay panel and both the overlay panel and the strain filter will be dismissed. This is also as intended.
However, if you click on the trash can icon to remove the filter you're left with the full tree displayed but the overlay panel remains. It takes a second click outside the panel to dismiss it.
![dismissed](https://user-images.githubusercontent.com/1176109/101833511-f6f76280-3aed-11eb-83e9-478c5e18e6f7.png)
Expected behavior
Instead of this behavior, clicking the trash can icon should remove both strain filter and the overlay panel.
@jameshadfield I started looking into this. I'm a bit confused about the different trees.
This is what I've gathered:
- Clicking outside the overlay panel restores the view by removing the modal (L149) and inactivating the filter (L152): https://github.com/nextstrain/auspice/blob/d8b3d2ae700978a8ef6ebb817b52e994936b1a09/src/components/tree/reactD3Interface/callbacks.js#L149-L153
- Clicking the trash can icon only removes the filter using the same
applyFilter
function. It doesn't resetselectedNode
which is the reason for this bug. https://github.com/nextstrain/auspice/blob/d8b3d2ae700978a8ef6ebb817b52e994936b1a09/src/components/info/filtersSummary.js#L54
And respectively,
- The external click, through a callback binding, removes the modal via
selectedNode
on thisTree
class: https://github.com/nextstrain/auspice/blob/d8b3d2ae700978a8ef6ebb817b52e994936b1a09/src/components/tree/tree.js#L23 - The icon click is connected to this
Tree
reducer: https://github.com/nextstrain/auspice/blob/d8b3d2ae700978a8ef6ebb817b52e994936b1a09/src/reducers/tree.js#L32
I can't figure out how to get to the Tree
class from the Tree
reducer. Are they even related?
Notes after a call between Jover / Victor / Jennifer earlier this week
-
The piece of redux state
tree.selectedStrain
used to encode the selected strain (!) and we had code within the Tree component to watch for changes here and update the modal appropriately. Despite references to this redux state remaining within middleware, components and reducers, this is no longer set by any action (i.e. it is alwaysundefined
). -
If one filters to a strain via the sidebar filtering UI, then the modal shouldn't pop up (no change from current behavior). I think it makes sense in the current design to only show the modal when clicking on the tree tip. Actions which should remove the modal: clicking outside it, clicking inactivate / remove on the filter badge. (Other ideas welcomed here.)
-
Point 2 implies that we wish to store a piece of redux state representing "strain modal showing" or similar and that we can't use filter state as it currently stands to determine whether a modal should be shown. (One could introduce a more complicated structure into the strain filter state, but I think the concepts are separate enough to warrant different pieces of state.) We could reuse
reduxState.tree.selectedStrain
if we want, but we don't have to. -
Currently all information about which parts of the tree are hovered / clicked, and therefore if a modal is showing, is represented by the Tree component's
state.selectedNode
. This is an object with three properties:node {obj}, type {str}, event {str}
. To elaborate, the<NodeClickedPanel>
is always part of the render cycle of<Tree>
, but in most cases simply returnsnull
(and therefore no DOM elements are drawn) ifstate.selectedNode.event!=="click"
. -
We could replace the tip-click events currently encoded by (4) with actions to change redux state (3), with the redux state being the one source of truth for which strain is showing in a modal. Alternatively we could use a redux boolean indicating whether to show a tip modal or not, but this raises issues of keeping redux state and tree component state in sync.
-
As a bonus, the deselecting of a tip modal by clicking outside the tree would ideally not affect the filter state. This should be easy no matter which decision (5) we choose.
-
One day we may choose a different UI to a modal (separate to this issue). Having the selected modal strain in redux will help with this work.