From 9f3fddc4aba52996b095e8c6e9e2fb00ed2bd896 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Wed, 21 Feb 2024 11:32:05 +1300 Subject: [PATCH 1/4] Restore terminal branch hover behaviour Hovering on terminal branches incorrectly displayed tip-specific information rather than branch-specific. This was first noticed in as the mutations are summarised differently for branches vs nodes. This bug was introduced in 86b85273e4f5df94e4b6191efb2781bb614f032c and this commit reverts some of the changes introduced there. Closes the original issue described in , but comments in that issue also identified a separate bug related to the branch-click modal. --- src/components/tree/infoPanels/hover.js | 5 +++-- src/components/tree/reactD3Interface/callbacks.js | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/components/tree/infoPanels/hover.js b/src/components/tree/infoPanels/hover.js index 393c36bbc..1812bea7c 100644 --- a/src/components/tree/infoPanels/hover.js +++ b/src/components/tree/infoPanels/hover.js @@ -402,11 +402,12 @@ const HoverInfoPanel = ({ t }) => { if (!selectedNode) return null - const node = selectedNode.n; + const node = selectedNode.node.n; // want the redux node, not the phylo node const idxOfInViewRootNode = getIdxOfInViewRootNode(node); + return ( - {node.hasChildren===false ? ( + {selectedNode.isBranch===false ? ( <> diff --git a/src/components/tree/reactD3Interface/callbacks.js b/src/components/tree/reactD3Interface/callbacks.js index dbd13f7d0..00d51fbec 100644 --- a/src/components/tree/reactD3Interface/callbacks.js +++ b/src/components/tree/reactD3Interface/callbacks.js @@ -13,7 +13,9 @@ export const onTipHover = function onTipHover(d) { this.state.treeToo; phylotree.svg.select("#"+getDomId("tip", d.n.name)) .attr("r", (e) => e["r"] + 4); - this.setState({hoveredNode: d}); + this.setState({ + hoveredNode: {node: d, isBranch: false} + }); }; export const onTipClick = function onTipClick(d) { @@ -46,7 +48,9 @@ export const onBranchHover = function onBranchHover(d) { } /* Set the hovered state so that an info box can be displayed */ - this.setState({hoveredNode: d}); + this.setState({ + hoveredNode: {node: d, isBranch: true} + }); }; export const onBranchClick = function onBranchClick(d) { From 4d70916238355fc84f99219865de39c89868a949 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Wed, 21 Feb 2024 12:05:14 +1300 Subject: [PATCH 2/4] Restore terminal branch click behaviour Clicking on terminal branches incorrectly behaved as if we had clicked on the tip (the circle); the intended behaviour is for internal + terminal branches to summarise the information the same way. This was noticed in an issue comment The bug was introduced in f7e944dd9eee3f19c34d3611f8de916ad2465924 which removed the ability to detect if a click occurred on a terminal node (tip) or terminal branch. --- src/components/tree/infoPanels/click.js | 16 +++++++++------- .../tree/reactD3Interface/callbacks.js | 4 ++-- src/reducers/controls.ts | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/components/tree/infoPanels/click.js b/src/components/tree/infoPanels/click.js index eefb5148a..b56c82f23 100644 --- a/src/components/tree/infoPanels/click.js +++ b/src/components/tree/infoPanels/click.js @@ -247,7 +247,9 @@ const NodeClickedPanel = ({selectedNode, nodes, clearSelectedNode, colorings, ob const panelStyle = { ...infoPanelStyles.panel}; panelStyle.maxHeight = "70%"; const isTerminal = !node.hasChildren; - const title = isTerminal ? + const isTip = !selectedNode.isBranch; + + const title = isTip ? node.name : isTerminal ? `Branch leading to ${node.name}` : @@ -259,19 +261,19 @@ const NodeClickedPanel = ({selectedNode, nodes, clearSelectedNode, colorings, ob {title} - {!isTerminal && item(t("Number of terminal tips"), node.fullTipCount)} - {isTerminal && } + {!isTip && item(t("Number of terminal tips"), node.fullTipCount)} + {isTip && } - {!isTerminal && item("Node name", node.name)} - {isTerminal && } + {!isTip && item("Node name", node.name)} + {isTip && } {getTraitsToDisplay(node).map((trait) => ( ))} - {isTerminal && } + {isTip && } {item("", "")}
- +

{t("Click outside this box to go back to the tree")}

diff --git a/src/components/tree/reactD3Interface/callbacks.js b/src/components/tree/reactD3Interface/callbacks.js index 00d51fbec..3ef60c940 100644 --- a/src/components/tree/reactD3Interface/callbacks.js +++ b/src/components/tree/reactD3Interface/callbacks.js @@ -24,7 +24,7 @@ export const onTipClick = function onTipClick(d) { /* The order of these two dispatches is important: the reducer handling `SELECT_NODE` must have access to the filtering state _prior_ to these filters being applied */ - this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx}); + this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx, isBranch: false}); this.props.dispatch(applyFilter("add", strainSymbol, [d.n.name])); }; @@ -60,7 +60,7 @@ export const onBranchClick = function onBranchClick(d) { /* if a branch was clicked while holding the shift key, we instead display a node-clicked modal */ if (window.event.shiftKey) { // no need to dispatch a filter action - this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx}) + this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx, isBranch: true}) return; } diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index 503c8ded5..ca4024447 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -238,7 +238,7 @@ const Controls = (state: ControlsState = getDefaultControlsState(), action): Con const existingFilterInfo = (state.filters?.[strainSymbol]||[]).find((info) => info.value===action.name); const existingFilterState = existingFilterInfo === undefined ? null : existingFilterInfo.active ? 'active' : 'inactive'; - return {...state, selectedNode: {name: action.name, idx: action.idx, existingFilterState}}; + return {...state, selectedNode: {name: action.name, idx: action.idx, existingFilterState, isBranch: action.isBranch}}; } case types.DESELECT_NODE: { return {...state, selectedNode: null} From 47e4e6642c909cbcf4a45cadd30ed1a8289cc5df Mon Sep 17 00:00:00 2001 From: james hadfield Date: Wed, 21 Feb 2024 12:44:55 +1300 Subject: [PATCH 3/4] Fix un-initialised filter console error With the changes in the previous commit we no longer need the `isTerminal` argument, as we use the (more accurate) `isBranch` property. This removes the console errors "trying to remove values from an un-initialised filter!" when shift-clicking on a branch then removing the info modal. The bug was introduced by #1749. --- src/components/tree/infoPanels/click.js | 2 +- src/components/tree/reactD3Interface/callbacks.js | 4 ++-- src/components/tree/tree.js | 5 +---- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/components/tree/infoPanels/click.js b/src/components/tree/infoPanels/click.js index b56c82f23..dc21c7281 100644 --- a/src/components/tree/infoPanels/click.js +++ b/src/components/tree/infoPanels/click.js @@ -256,7 +256,7 @@ const NodeClickedPanel = ({selectedNode, nodes, clearSelectedNode, colorings, ob "Internal branch"; return ( -
clearSelectedNode(selectedNode, isTerminal)}> +
clearSelectedNode(selectedNode)}>
stopProp(e)}> {title} diff --git a/src/components/tree/reactD3Interface/callbacks.js b/src/components/tree/reactD3Interface/callbacks.js index 3ef60c940..139bfdadc 100644 --- a/src/components/tree/reactD3Interface/callbacks.js +++ b/src/components/tree/reactD3Interface/callbacks.js @@ -124,8 +124,8 @@ export const onTipLeave = function onTipLeave(d) { }; /* clearSelectedNode when clicking to remove the node-selected modal */ -export const clearSelectedNode = function clearSelectedNode(selectedNode, isTerminal) { - if (isTerminal) { +export const clearSelectedNode = function clearSelectedNode(selectedNode) { + if (!selectedNode.isBranch) { /* perform the filtering action (if necessary) that will restore the filtering state of the node prior to the selection */ if (!selectedNode.existingFilterState) { diff --git a/src/components/tree/tree.js b/src/components/tree/tree.js index 2020657dd..f91040fa4 100644 --- a/src/components/tree/tree.js +++ b/src/components/tree/tree.js @@ -44,10 +44,7 @@ class Tree extends React.Component { /* pressing the escape key should dismiss an info modal (if one exists) */ this.handlekeydownEvent = (event) => { if (event.key==="Escape" && this.props.selectedNode) { - this.clearSelectedNode( - this.props.selectedNode, - !this.props.tree.nodes[this.props.selectedNode.idx].hasChildren - ); + this.clearSelectedNode(this.props.selectedNode); } }; } From e6c461c1bcbe65ab7e6e1e6bbfaea2e62c2e611c Mon Sep 17 00:00:00 2001 From: james hadfield Date: Wed, 21 Feb 2024 12:24:10 +1300 Subject: [PATCH 4/4] changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16ac87231..5263c8af5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ # Changelog -## version 2.52.0 - 2024/02/09 - +* Bugfix: Restore the intended behaviour when hovering or clicking on terminal branches. ([#1753](https://github.com/nextstrain/auspice/pull/1753)) +## version 2.52.0 - 2024/02/09 * Sidebar filtering now contains all non-continuous metadata defined across the tree (i.e. all data within `node.node_attrs`). The traits listed in `meta.filters` are now only used to determine which filters to list in the footer of the page. ([#1743](https://github.com/nextstrain/auspice/pull/1743)) * The interaction between strain-selected modals and the corresponding strain-filter has been improved. We now preserve the strain filter state present before the node was clicked. ([#1749](https://github.com/nextstrain/auspice/issues/1749))