Skip to content

Commit

Permalink
[tangletrees] fix node-click modal for RHS tree
Browse files Browse the repository at this point in the history
This bug arrived with recent work which lifted the selected node state
out of component state and into redux¹, where I forgot to consider
tangletrees so that we always ended up retrieving the node (via array
index) from the LHS tree.

Closes <#1770>

¹ <#1749>
  • Loading branch information
jameshadfield committed Jun 6, 2024
1 parent bcf0b9f commit d59dbd5
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 10 deletions.
12 changes: 9 additions & 3 deletions src/components/tree/infoPanels/click.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { numericToCalendar } from "../../../util/dateHelpers";
import { getTraitFromNode, getFullAuthorInfoFromNode, getVaccineFromNode,
getAccessionFromNode, getUrlFromNode } from "../../../util/treeMiscHelpers";
import { MutationTable } from "./MutationTable";
import { lhsTreeId} from "../tree";

export const styles = {
container: {
Expand Down Expand Up @@ -234,16 +235,21 @@ 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.selectedNode
* @param {Object[]} props.nodes
* @param {Object[]} props.nodesLhsTree
* @param {Object[]|undefined} props.nodesRhsTree
* @param {function} props.clearSelectedNode
* @param {Object} props.colorings
* @param {Object} props.observedMutations
* @param {function} props.geneSortFn
* @param {function} props.t
*/
const NodeClickedPanel = ({selectedNode, nodes, clearSelectedNode, colorings, observedMutations, geneSortFn, t}) => {
const NodeClickedPanel = ({selectedNode, nodesLhsTree, nodesRhsTree, clearSelectedNode, colorings, observedMutations, geneSortFn, t}) => {
if (!selectedNode) return null;
const node = nodes[selectedNode.idx];
const node = (selectedNode.treeId===lhsTreeId ? nodesLhsTree : nodesRhsTree)?.[selectedNode.idx];
if (!node) {
console.error('Internal error retrieving selected node');
return null;
}
const panelStyle = { ...infoPanelStyles.panel};
panelStyle.maxHeight = "70%";
const isTerminal = !node.hasChildren;
Expand Down
4 changes: 2 additions & 2 deletions src/components/tree/reactD3Interface/callbacks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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, isBranch: false});
this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx, isBranch: false, treeId: d.that.id});
this.props.dispatch(applyFilter("add", strainSymbol, [d.n.name]));
};

Expand Down Expand Up @@ -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, isBranch: true})
this.props.dispatch({type: SELECT_NODE, name: d.n.name, idx: d.n.arrayIdx, isBranch: true, treeId: d.that.id})
return;
}

Expand Down
9 changes: 6 additions & 3 deletions src/components/tree/tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { untangleTreeToo } from "./tangle/untangling";
import { sortByGeneOrder } from "../../util/treeMiscHelpers";

export const spaceBetweenTrees = 100;
export const lhsTreeId = "LEFT";
const rhsTreeId = "RIGHT";

class Tree extends React.Component {
constructor(props) {
Expand Down Expand Up @@ -51,7 +53,7 @@ class Tree extends React.Component {
setUpAndRenderTreeToo(props, newState) {
/* this.setState(newState) will be run sometime after this returns */
/* modifies newState in place */
newState.treeToo = new PhyloTree(props.treeToo.nodes, "RIGHT", props.treeToo.idxOfInViewRootNode);
newState.treeToo = new PhyloTree(props.treeToo.nodes, rhsTreeId, props.treeToo.idxOfInViewRootNode);
if (attemptUntangle) {
untangleTreeToo(newState.tree, newState.treeToo);
}
Expand All @@ -61,7 +63,7 @@ class Tree extends React.Component {
document.addEventListener('keyup', this.handlekeydownEvent);
if (this.props.tree.loaded) {
const newState = {};
newState.tree = new PhyloTree(this.props.tree.nodes, "LEFT", this.props.tree.idxOfInViewRootNode);
newState.tree = new PhyloTree(this.props.tree.nodes, lhsTreeId, this.props.tree.idxOfInViewRootNode);
renderTree(this, true, newState.tree, this.props);
if (this.props.showTreeToo) {
this.setUpAndRenderTreeToo(this.props, newState); /* modifies newState in place */
Expand Down Expand Up @@ -209,7 +211,8 @@ class Tree extends React.Component {
<NodeClickedPanel
clearSelectedNode={this.clearSelectedNode}
selectedNode={this.props.selectedNode}
nodes={this.props.tree.nodes}
nodesLhsTree={this.props.tree.nodes}
nodesRhsTree={this.props.treeToo?.nodes}
observedMutations={this.props.tree.observedMutations}
colorings={this.props.colorings}
geneSortFn={this.state.geneSortFn}
Expand Down
9 changes: 7 additions & 2 deletions src/reducers/controls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,16 @@ const Controls = (state: ControlsState = getDefaultControlsState(), action): Con
geoResolution: action.data
});

case types.SELECT_NODE: {
case types.SELECT_NODE: {
/**
* We don't store a (reference to) the node itself as that breaks redux's immutability checking,
* instead we store the information needed to access it from the nodes array(s)
*/
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, isBranch: action.isBranch}};
const selectedNode = {name: action.name, idx: action.idx, existingFilterState, isBranch: action.isBranch, treeId: action.treeId};
return {...state, selectedNode};
}
case types.DESELECT_NODE: {
return {...state, selectedNode: null}
Expand Down

0 comments on commit d59dbd5

Please sign in to comment.