From ad4dc514358ba2cfc3843dbec4bb451bb3702ca2 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Mon, 5 Feb 2024 13:36:24 +1300 Subject: [PATCH 1/5] Remove unused redux state and actions --- src/actions/types.js | 2 -- src/components/info/info.js | 2 +- src/components/tree/reactD3Interface/change.js | 5 ----- src/middleware/changeURL.js | 1 - src/reducers/controls.ts | 12 ------------ src/reducers/tree.js | 2 -- src/reducers/treeToo.js | 1 - 7 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/actions/types.js b/src/actions/types.js index 1b2f0cd41..0380fbd84 100644 --- a/src/actions/types.js +++ b/src/actions/types.js @@ -3,8 +3,6 @@ export const NEW_COLORS = "NEW_COLORS"; export const LOAD_FREQUENCIES = "LOAD_FREQUENCIES"; export const FREQUENCY_MATRIX = "FREQUENCY_MATRIX"; export const BROWSER_DIMENSIONS = "BROWSER_DIMENSIONS"; -export const NODE_MOUSEENTER = "NODE_MOUSEENTER"; -export const NODE_MOUSELEAVE = "NODE_MOUSELEAVE"; export const SEARCH_INPUT_CHANGE = "SEARCH_INPUT_CHANGE"; export const CHANGE_LAYOUT = "CHANGE_LAYOUT"; export const CHANGE_BRANCH_LABEL = "CHANGE_BRANCH_LABEL"; diff --git a/src/components/info/info.js b/src/components/info/info.js index 554c8a2ee..7734df881 100644 --- a/src/components/info/info.js +++ b/src/components/info/info.js @@ -35,7 +35,7 @@ class Info extends React.Component { if (!this.props.metadata || !this.props.nodes || !this.props.visibility) return null; const styles = computeStyles(this.props.width, this.props.browserWidth); const animating = this.props.animationPlayPauseButton === "Pause"; - const showExtended = !animating && !this.props.selectedStrain; + const showExtended = !animating; return (
diff --git a/src/components/tree/reactD3Interface/change.js b/src/components/tree/reactD3Interface/change.js index 4ff6f57ec..9e759e5af 100644 --- a/src/components/tree/reactD3Interface/change.js +++ b/src/components/tree/reactD3Interface/change.js @@ -11,11 +11,6 @@ export const changePhyloTreeViaPropsComparison = (mainTree, phylotree, oldProps, Note that updating properties itself won't trigger any visual changes */ phylotree.dateRange = [newProps.dateMinNumeric, newProps.dateMaxNumeric]; - /* catch selectedStrain disappearance separately to visibility and remove modal */ - if (oldTreeRedux.selectedStrain && !newTreeRedux.selectedStrain) { - /* TODO change back the tip radius */ - newState.selectedNode = {}; - } /* colorBy change? */ if (!!newTreeRedux.nodeColorsVersion && (oldTreeRedux.nodeColorsVersion !== newTreeRedux.nodeColorsVersion || diff --git a/src/middleware/changeURL.js b/src/middleware/changeURL.js index 7e08befa9..cd44259cc 100644 --- a/src/middleware/changeURL.js +++ b/src/middleware/changeURL.js @@ -185,7 +185,6 @@ export const changeURLMiddleware = (store) => (next) => (action) => { break; } case types.UPDATE_VISIBILITY_AND_BRANCH_THICKNESS: { - // query.s = action.selectedStrain ? action.selectedStrain : undefined; query.label = action.cladeName ? action.cladeName : undefined; break; } diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index c78a82e14..7b3a95b6a 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -53,10 +53,6 @@ export const getDefaultControlsState = () => { defaults, available: undefined, canTogglePanelLayout: true, - selectedNode: null, - region: null, - search: null, - strain: null, temporalConfidence: { exists: false, display: false, on: false }, layout: defaults.layout, scatterVariables: {}, @@ -120,14 +116,6 @@ const Controls = (state: ControlsState = getDefaultControlsState(), action): Con return action.controls; case types.SET_AVAILABLE: return Object.assign({}, state, { available: action.data }); - case types.NODE_MOUSEENTER: - return Object.assign({}, state, { - selectedNode: action.data - }); - case types.NODE_MOUSELEAVE: - return Object.assign({}, state, { - selectedNode: null - }); case types.CHANGE_EXPLODE_ATTR: return Object.assign({}, state, { explodeAttr: action.explodeAttr, diff --git a/src/reducers/tree.js b/src/reducers/tree.js index cd9b3a8a5..9ce411cf8 100644 --- a/src/reducers/tree.js +++ b/src/reducers/tree.js @@ -24,7 +24,6 @@ export const getDefaultTreeState = () => { totalStateCounts: {}, observedMutations: {}, availableBranchLabels: [], - selectedStrain: undefined, selectedClade: undefined }; }; @@ -51,7 +50,6 @@ const Tree = (state = getDefaultTreeState(), action) => { idxOfFilteredRoot: action.idxOfFilteredRoot, cladeName: action.cladeName, selectedClade: action.cladeName, - selectedStrain: action.selectedStrain }; return Object.assign({}, state, newStates); } diff --git a/src/reducers/treeToo.js b/src/reducers/treeToo.js index 6b195ce06..e5c67a46b 100644 --- a/src/reducers/treeToo.js +++ b/src/reducers/treeToo.js @@ -45,7 +45,6 @@ const treeToo = (state = getDefaultTreeState(), action) => { branchThicknessVersion: action.branchThicknessVersionToo, idxOfInViewRootNode: action.idxOfInViewRootNodeToo, idxOfFilteredRoot: action.idxOfFilteredRootToo, - selectedStrain: action.selectedStrain }); } return state; From f7e944dd9eee3f19c34d3611f8de916ad2465924 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Mon, 5 Feb 2024 15:01:44 +1300 Subject: [PATCH 2/5] Lift node-selected modal to redux state Lifting this from tree state to redux state paves the way to fix inconsistent behaviour as well as allowing the information to be displayed in a modal outside of the Tree component in the future. We no longer update the (tip circle) radius within `clearSelectedNode` as testing (Firefox v121, Chrome v121) shows the `onTipLeave` function is enough to achieve the desired radius update. --- src/actions/types.js | 3 ++ src/components/tree/index.js | 1 + src/components/tree/infoPanels/click.js | 36 ++++++++++--------- .../tree/reactD3Interface/callbacks.js | 32 +++++------------ src/components/tree/tree.js | 10 ++++-- src/reducers/controls.ts | 8 +++++ 6 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/actions/types.js b/src/actions/types.js index 0380fbd84..edf3cd5bb 100644 --- a/src/actions/types.js +++ b/src/actions/types.js @@ -61,3 +61,6 @@ export const APPLY_MEASUREMENTS_FILTER = "APPLY_MEASUREMENTS_FILTER"; export const UPDATE_MEASUREMENTS_ERROR = "UPDATE_MEASUREMENTS_ERROR"; export const TOGGLE_SHOW_ALL_BRANCH_LABELS = "TOGGLE_SHOW_ALL_BRANCH_LABELS"; export const TOGGLE_MOBILE_DISPLAY = "TOGGLE_MOBILE_DISPLAY"; +export const SELECT_NODE = "SELECT_NODE"; +export const DESELECT_NODE = "DESELECT_NODE"; + diff --git a/src/components/tree/index.js b/src/components/tree/index.js index dd34a5a58..c9bc68c38 100644 --- a/src/components/tree/index.js +++ b/src/components/tree/index.js @@ -4,6 +4,7 @@ import UnconnectedTree from "./tree"; const Tree = connect((state) => ({ tree: state.tree, treeToo: state.treeToo, + selectedNode: state.controls.selectedNode, dateMinNumeric: state.controls.dateMinNumeric, dateMaxNumeric: state.controls.dateMaxNumeric, quickdraw: state.controls.quickdraw, diff --git a/src/components/tree/infoPanels/click.js b/src/components/tree/infoPanels/click.js index 27c53a281..eefb5148a 100644 --- a/src/components/tree/infoPanels/click.js +++ b/src/components/tree/infoPanels/click.js @@ -233,43 +233,45 @@ const Trait = ({node, trait, colorings, isTerminal}) => { /** * A React component to display information about a tree tip in a modal-overlay style * @param {Object} props - * @param {Object} props.tip tip node selected - * @param {function} props.goAwayCallback - * @param {object} props.colorings + * @param {Object} props.selectedNode + * @param {Object[]} props.nodes + * @param {function} props.clearSelectedNode + * @param {Object} props.colorings + * @param {Object} props.observedMutations + * @param {function} props.geneSortFn + * @param {function} props.t */ -const NodeClickedPanel = ({selectedNode, clearSelectedNode, colorings, observedMutations, geneSortFn, t}) => { - if (selectedNode.event!=="click") {return null;} +const NodeClickedPanel = ({selectedNode, nodes, clearSelectedNode, colorings, observedMutations, geneSortFn, t}) => { + if (!selectedNode) return null; + const node = nodes[selectedNode.idx]; const panelStyle = { ...infoPanelStyles.panel}; panelStyle.maxHeight = "70%"; - const node = selectedNode.node.n; - const isTip = selectedNode.type === "tip"; - const isTerminal = node.fullTipCount===1; - - const title = isTip ? + const isTerminal = !node.hasChildren; + const title = isTerminal ? node.name : isTerminal ? `Branch leading to ${node.name}` : "Internal branch"; return ( -
clearSelectedNode(selectedNode)}> +
clearSelectedNode(selectedNode, isTerminal)}>
stopProp(e)}> {title} - {!isTip && item(t("Number of terminal tips"), node.fullTipCount)} - {isTip && } + {!isTerminal && item(t("Number of terminal tips"), node.fullTipCount)} + {isTerminal && } - {!isTip && item("Node name", node.name)} - {isTip && } + {!isTerminal && item("Node name", node.name)} + {isTerminal && } {getTraitsToDisplay(node).map((trait) => ( ))} - {isTip && } + {isTerminal && } {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 396260754..7e68513b7 100644 --- a/src/components/tree/reactD3Interface/callbacks.js +++ b/src/components/tree/reactD3Interface/callbacks.js @@ -2,6 +2,7 @@ import { updateVisibleTipsAndBranchThicknesses, applyFilter } from "../../../act import { NODE_VISIBLE, strainSymbol } from "../../../util/globals"; import { getDomId, getParentBeyondPolytomy, getIdxOfInViewRootNode } from "../phyloTree/helpers"; import { branchStrokeForHover, branchStrokeForLeave } from "../phyloTree/renderers"; +import { SELECT_NODE, DESELECT_NODE } from "../../../actions/types"; /* Callbacks used by the tips / branches when hovered / selected */ @@ -24,13 +25,7 @@ export const onTipHover = function onTipHover(d) { export const onTipClick = function onTipClick(d) { if (d.visibility !== NODE_VISIBLE) return; if (this.props.narrativeMode) return; - this.setState({ - selectedNode: { - node: d, - type: "tip", - event: "click" - } - }); + this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx}); this.props.dispatch(applyFilter("add", strainSymbol, [d.n.name])); }; @@ -69,13 +64,8 @@ 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) { - this.setState({ - selectedNode: { - node: d, - type: "branch", - event: "click" - } - }); + // no need to dispatch a filter action + this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx}) return; } @@ -141,15 +131,9 @@ export const onTipLeave = function onTipLeave(d) { }; /* clearSelectedNode when clicking to remove the node-selected modal */ -export const clearSelectedNode = function clearSelectedNode(selectedNode) { - const phylotree = selectedNode.node.that.params.orientation[0] === 1 ? - this.state.tree : - this.state.treeToo; - phylotree.svg.select("#"+getDomId("tip", selectedNode.node.n.name)) - .attr("r", (dd) => dd["r"]); - this.setState({selectedNode: {}}); - if (selectedNode.type==="tip") { - /* restore the tip visibility! */ - this.props.dispatch(applyFilter("inactivate", strainSymbol, [selectedNode.node.n.name])); +export const clearSelectedNode = function clearSelectedNode(selectedNode, isTerminal) { + if (isTerminal) { + this.props.dispatch(applyFilter("inactivate", strainSymbol, [selectedNode.name])); } + this.props.dispatch({type: DESELECT_NODE}); }; diff --git a/src/components/tree/tree.js b/src/components/tree/tree.js index 5018b18f1..0f188957f 100644 --- a/src/components/tree/tree.js +++ b/src/components/tree/tree.js @@ -43,8 +43,11 @@ 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.state.selectedNode?.node) { - this.clearSelectedNode(this.state.selectedNode); + if (event.key==="Escape" && this.props.selectedNode) { + this.clearSelectedNode( + this.props.selectedNode, + !this.props.tree.nodes[this.props.selectedNode.idx].hasChildren + ); } }; } @@ -208,7 +211,8 @@ class Tree extends React.Component { /> { explodeAttr: undefined, selectedBranchLabel: "none", showAllBranchLabels: false, + selectedNode: null, canRenderBranchLabels: true, analysisSlider: false, geoResolution: defaults.geoResolution, @@ -232,6 +233,13 @@ const Controls = (state: ControlsState = getDefaultControlsState(), action): Con return Object.assign({}, state, { geoResolution: action.data }); + + case types.SELECT_NODE: { + return {...state, selectedNode: {name: action.name, idx: action.idx}}; + } + case types.DESELECT_NODE: { + return {...state, selectedNode: null} + } case types.APPLY_FILTER: { // values arrive as array const filters = Object.assign({}, state.filters, {}); From 86b85273e4f5df94e4b6191efb2781bb614f032c Mon Sep 17 00:00:00 2001 From: james hadfield Date: Mon, 5 Feb 2024 15:22:30 +1300 Subject: [PATCH 3/5] Rename hovered node state With the previous commit creating "selectedNode" as part of redux state, the tree state variable "selectedNode" is only used for hover events, so rename it to convey this. --- src/components/tree/infoPanels/hover.js | 7 +++--- .../tree/reactD3Interface/callbacks.js | 24 ++++--------------- src/components/tree/tree.js | 4 ++-- 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/components/tree/infoPanels/hover.js b/src/components/tree/infoPanels/hover.js index cbbd7772d..393c36bbc 100644 --- a/src/components/tree/infoPanels/hover.js +++ b/src/components/tree/infoPanels/hover.js @@ -401,13 +401,12 @@ const HoverInfoPanel = ({ observedMutations, t }) => { - if (selectedNode.event !== "hover") return null; - const node = selectedNode.node.n; + if (!selectedNode) return null + const node = selectedNode.n; const idxOfInViewRootNode = getIdxOfInViewRootNode(node); - return ( - {selectedNode.type === "tip" ? ( + {node.hasChildren===false ? ( <> diff --git a/src/components/tree/reactD3Interface/callbacks.js b/src/components/tree/reactD3Interface/callbacks.js index 7e68513b7..1c327aadc 100644 --- a/src/components/tree/reactD3Interface/callbacks.js +++ b/src/components/tree/reactD3Interface/callbacks.js @@ -13,13 +13,7 @@ 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({ - selectedNode: { - node: d, - type: "tip", - event: "hover" - } - }); + this.setState({hoveredNode: d}); }; export const onTipClick = function onTipClick(d) { @@ -49,13 +43,7 @@ export const onBranchHover = function onBranchHover(d) { } /* Set the hovered state so that an info box can be displayed */ - this.setState({ - selectedNode: { - node: d, - type: "branch", - event: "hover" - } - }); + this.setState({hoveredNode: d}); }; export const onBranchClick = function onBranchClick(d) { @@ -104,7 +92,6 @@ export const onBranchClick = function onBranchClick(d) { /* onBranchLeave called when mouse-off, i.e. anti-hover */ export const onBranchLeave = function onBranchLeave(d) { - if (this.state.selectedNode.event!=="hover") return; /* Reset the stroke back to what it was before */ branchStrokeForLeave(d); @@ -115,19 +102,18 @@ export const onBranchLeave = function onBranchLeave(d) { tree.removeConfidence(); } /* Set selectedNode state to an empty object, which will remove the info box */ - this.setState({selectedNode: {}}); + this.setState({hoveredNode: null}); }; export const onTipLeave = function onTipLeave(d) { - if (this.state.selectedNode.event!=="hover") return; const phylotree = d.that.params.orientation[0] === 1 ? this.state.tree : this.state.treeToo; - if (this.state.selectedNode) { + if (this.state.hoveredNode) { phylotree.svg.select("#"+getDomId("tip", d.n.name)) .attr("r", (dd) => dd["r"]); } - this.setState({selectedNode: {}}); + this.setState({hoveredNode: null}); }; /* clearSelectedNode when clicking to remove the node-selected modal */ diff --git a/src/components/tree/tree.js b/src/components/tree/tree.js index 0f188957f..2020657dd 100644 --- a/src/components/tree/tree.js +++ b/src/components/tree/tree.js @@ -29,7 +29,7 @@ class Tree extends React.Component { }; this.tangleRef = undefined; this.state = { - selectedNode: {}, + hoveredNode: null, tree: null, treeToo: null }; @@ -199,7 +199,7 @@ class Tree extends React.Component { Date: Mon, 5 Feb 2024 15:53:42 +1300 Subject: [PATCH 4/5] Remove node modal on filter removal Clicking a tip brings up a modal and activates the corresponding strain filter. Inactivating or removing this filter now also clears the modal. Closes #1243 --- src/reducers/controls.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index b36b8c7ed..f25dc2866 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -248,8 +248,20 @@ const Controls = (state: ControlsState = getDefaultControlsState(), action): Con } else { // remove if no active+inactive filters delete filters[action.trait] } + + /* In the situation where a node-selected modal is active + we have + removed or inactivated the corresponding filter, then we want to remove + the modal */ + let selectedNode = state.selectedNode + if (selectedNode) { + const filterInfo = filters?.[strainSymbol]?.find((f)=>f.value===selectedNode.name); + if (!filterInfo || !filterInfo.active) { + selectedNode = null; + } + } return Object.assign({}, state, { - filters + filters, + selectedNode, }); } case types.TOGGLE_TEMPORAL_CONF: From 4dcc7d6aa667d06fbf58408f2995d98533dbde8d Mon Sep 17 00:00:00 2001 From: james hadfield Date: Mon, 5 Feb 2024 16:43:20 +1300 Subject: [PATCH 5/5] Change how closing a tip modal modifies filters Clicking on a tip activates a modal and activates the corresponding strain filter. Previously clearing the modal would always inactivate the filter. In the absence of any filtering this behaviour is not bad, but when already filtering to a set of strains then it's counter-intuitive because we inactivate a filter that was active. The new behaviour restores the filtering state of the strain before the modal was opened. Specifically, * If the tip was already filtered, opening and closing the modal doesn't change the filters. * If the tip was filtered but inactive, opening the modal activates it and closing the modal returns it to the inactive state. * If the tip wasn't filtered (active or inactive) then opening the modal makes it an active filter and closing the modal removes the filter entirely. Closes #1701 --- src/components/tree/reactD3Interface/callbacks.js | 12 +++++++++++- src/reducers/controls.ts | 7 +++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/components/tree/reactD3Interface/callbacks.js b/src/components/tree/reactD3Interface/callbacks.js index 1c327aadc..dbd13f7d0 100644 --- a/src/components/tree/reactD3Interface/callbacks.js +++ b/src/components/tree/reactD3Interface/callbacks.js @@ -19,6 +19,9 @@ export const onTipHover = function onTipHover(d) { export const onTipClick = function onTipClick(d) { if (d.visibility !== NODE_VISIBLE) return; if (this.props.narrativeMode) return; + /* 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(applyFilter("add", strainSymbol, [d.n.name])); }; @@ -119,7 +122,14 @@ export const onTipLeave = function onTipLeave(d) { /* clearSelectedNode when clicking to remove the node-selected modal */ export const clearSelectedNode = function clearSelectedNode(selectedNode, isTerminal) { if (isTerminal) { - this.props.dispatch(applyFilter("inactivate", strainSymbol, [selectedNode.name])); + /* perform the filtering action (if necessary) that will restore the + filtering state of the node prior to the selection */ + if (!selectedNode.existingFilterState) { + this.props.dispatch(applyFilter("remove", strainSymbol, [selectedNode.name])); + } else if (selectedNode.existingFilterState==='inactive') { + this.props.dispatch(applyFilter("inactivate", strainSymbol, [selectedNode.name])); + } + /* else the filter was already active, so leave it unchanged */ } this.props.dispatch({type: DESELECT_NODE}); }; diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index f25dc2866..503c8ded5 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -234,8 +234,11 @@ const Controls = (state: ControlsState = getDefaultControlsState(), action): Con geoResolution: action.data }); - case types.SELECT_NODE: { - return {...state, selectedNode: {name: action.name, idx: action.idx}}; + case types.SELECT_NODE: { + 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}}; } case types.DESELECT_NODE: { return {...state, selectedNode: null}