From 3f3d5dd0d12e47c7d0efc5bd1101e9055c402e20 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Tue, 21 Jan 2020 17:31:56 +0100 Subject: [PATCH 01/21] Show Visible-Icon for all ranges in tree that contain visible canvases That includes ranges with visible canvases in their members / canvases / items as well as their parent ranges. --- src/components/SidebarIndexTableOfContents.js | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index 7eba105f4e..c408417fda 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -3,10 +3,22 @@ import PropTypes from 'prop-types'; import TreeView from '@material-ui/lab/TreeView'; import ExpandMoreIcon from '@material-ui/icons/ExpandMore'; import ChevronRightIcon from '@material-ui/icons/ChevronRight'; +import VisibilityIcon from '@material-ui/icons/Visibility'; import TreeItem from '@material-ui/lab/TreeItem'; /** */ export class SidebarIndexTableOfContents extends Component { + /** */ + getAllSubTreeCanvasIds(node) { + const canvasIds = node.data.getCanvasIds() || []; + if (node.nodes) { + canvasIds.push( + ...node.nodes.reduce((acc, n) => acc.concat(...this.getAllSubTreeCanvasIds(n)), []), + ); + } + return canvasIds; + } + /** */ selectTreeItem(node) { const { setCanvas, windowId } = this.props; @@ -19,16 +31,24 @@ export class SidebarIndexTableOfContents extends Component { } /** */ - buildTreeItems(nodes) { + buildTreeItems(nodes, canvasIds) { return ( nodes.map(node => ( + {canvasIds.reduce( + (acc, canvasId) => acc || (this.getAllSubTreeCanvasIds(node).indexOf(canvasId) !== -1), + false, + ) && } + {node.label} + + )} onClick={() => this.selectTreeItem(node)} > - {node.nodes.length > 0 ? this.buildTreeItems(node.nodes) : null} + {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds) : null} )) ); @@ -37,21 +57,24 @@ export class SidebarIndexTableOfContents extends Component { /** */ render() { const { - classes, treeStructure, + canvases, classes, treeStructure, } = this.props; if (!treeStructure) { return <>; } + const canvasIds = canvases.map(canvas => canvas.id); + return ( <> } defaultExpandIcon={} + defaultEndIcon={<>} > - {this.buildTreeItems(treeStructure.nodes)} + {this.buildTreeItems(treeStructure.nodes, canvasIds)} ); @@ -59,6 +82,7 @@ export class SidebarIndexTableOfContents extends Component { } SidebarIndexTableOfContents.propTypes = { + canvases: PropTypes.arrayOf(PropTypes.object).isRequired, classes: PropTypes.objectOf(PropTypes.string).isRequired, setCanvas: PropTypes.func.isRequired, treeStructure: PropTypes.objectOf().isRequired, From 167e107f26358df95e05aa089d8ca553d9a584e5 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Wed, 5 Feb 2020 09:55:48 +0100 Subject: [PATCH 02/21] Add selector for ids of visible ranges --- src/components/SidebarIndexTableOfContents.js | 15 ++++--- src/containers/SidebarIndexTableOfContents.js | 2 + src/state/selectors/index.js | 1 + src/state/selectors/ranges.js | 39 +++++++++++++++++++ 4 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 src/state/selectors/ranges.js diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index c408417fda..37405e4ae6 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -31,7 +31,7 @@ export class SidebarIndexTableOfContents extends Component { } /** */ - buildTreeItems(nodes, canvasIds) { + buildTreeItems(nodes, canvasIds, visibleRangeIds) { return ( nodes.map(node => ( - {canvasIds.reduce( + {/* {canvasIds.reduce( (acc, canvasId) => acc || (this.getAllSubTreeCanvasIds(node).indexOf(canvasId) !== -1), false, - ) && } + ) && } */} + { visibleRangeIds.indexOf(node.id) !== -1 && } {node.label} )} onClick={() => this.selectTreeItem(node)} > - {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds) : null} + {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleRangeIds) : null} )) ); @@ -57,9 +58,10 @@ export class SidebarIndexTableOfContents extends Component { /** */ render() { const { - canvases, classes, treeStructure, + canvases, classes, treeStructure, visibleRanges, } = this.props; + console.log(visibleRanges); if (!treeStructure) { return <>; } @@ -74,7 +76,7 @@ export class SidebarIndexTableOfContents extends Component { defaultExpandIcon={} defaultEndIcon={<>} > - {this.buildTreeItems(treeStructure.nodes, canvasIds)} + {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRanges)} ); @@ -86,5 +88,6 @@ SidebarIndexTableOfContents.propTypes = { classes: PropTypes.objectOf(PropTypes.string).isRequired, setCanvas: PropTypes.func.isRequired, treeStructure: PropTypes.objectOf().isRequired, + visibleRanges: PropTypes.arrayOf(PropTypes.string).isRequired, windowId: PropTypes.string.isRequired, }; diff --git a/src/containers/SidebarIndexTableOfContents.js b/src/containers/SidebarIndexTableOfContents.js index dc65dd848e..4b22e5d937 100644 --- a/src/containers/SidebarIndexTableOfContents.js +++ b/src/containers/SidebarIndexTableOfContents.js @@ -8,6 +8,7 @@ import { getManifestoInstance, getManifestTreeStructure, getVisibleCanvases, + getVisibleRangeIds, } from '../state/selectors'; import * as actions from '../state/actions'; @@ -19,6 +20,7 @@ const mapStateToProps = (state, { id, windowId }) => ({ canvases: getVisibleCanvases(state, { windowId }), manifesto: getManifestoInstance(state, { windowId }), treeStructure: getManifestTreeStructure(state, { windowId }), + visibleRangeIds: getVisibleRangeIds(state, { windowId }), }); /** diff --git a/src/state/selectors/index.js b/src/state/selectors/index.js index 8e1d1ed1dd..4c3033b5e6 100644 --- a/src/state/selectors/index.js +++ b/src/state/selectors/index.js @@ -6,3 +6,4 @@ export * from './manifests'; export * from './windows'; export * from './workspace'; export * from './searches'; +export * from './ranges'; diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js new file mode 100644 index 0000000000..aa7b4865ce --- /dev/null +++ b/src/state/selectors/ranges.js @@ -0,0 +1,39 @@ +import { createSelector } from 'reselect'; +import { getManifestTreeStructure } from './manifests'; +import { getVisibleCanvases } from './canvases'; + +/** */ +function getVisibleRangeIdsInSubTree(nodes, canvasIds) { + return nodes.reduce((rangeIds, node) => { + const currentRangeIds = []; + const nodeContainsVisibleCanvas = canvasIds.reduce( + (acc, canvasId) => acc || node.data.getCanvasIds().indexOf(canvasId) !== -1, + false, + ); + if (node.nodes.length > 0) { + const subTreeVisibleRangeIds = node.nodes.length > 0 + ? getVisibleRangeIdsInSubTree(node.nodes, canvasIds) : []; + currentRangeIds.push(...subTreeVisibleRangeIds); + } + if (currentRangeIds.length > 0 || nodeContainsVisibleCanvas) { + currentRangeIds.push(node.id); + } + rangeIds.push(...currentRangeIds); + return rangeIds; + }, []); +} + +/** */ +export const getVisibleRangeIds = createSelector( + [ + getManifestTreeStructure, + getVisibleCanvases, + ], + (tree, canvases) => { + if (!canvases) { + return []; + } + const canvasIds = canvases.map(canvas => canvas.id); + return getVisibleRangeIdsInSubTree(tree.nodes, canvasIds); + }, +); From 23613a1b9204e41d1dfa5c0f2498b27ccc78e847 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Thu, 6 Feb 2020 16:52:41 +0100 Subject: [PATCH 03/21] Track manually opened TOC node ids in store --- src/components/SidebarIndexTableOfContents.js | 30 ++++++------------- src/containers/SidebarIndexTableOfContents.js | 7 +++-- src/state/actions/companionWindow.js | 21 +++++++++++++ src/state/selectors/ranges.js | 23 +++++++++++--- 4 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index 37405e4ae6..fab58a7f0d 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -8,20 +8,10 @@ import TreeItem from '@material-ui/lab/TreeItem'; /** */ export class SidebarIndexTableOfContents extends Component { - /** */ - getAllSubTreeCanvasIds(node) { - const canvasIds = node.data.getCanvasIds() || []; - if (node.nodes) { - canvasIds.push( - ...node.nodes.reduce((acc, n) => acc.concat(...this.getAllSubTreeCanvasIds(n)), []), - ); - } - return canvasIds; - } - /** */ selectTreeItem(node) { - const { setCanvas, windowId } = this.props; + const { setCanvas, toggleRangeNode, windowId } = this.props; + toggleRangeNode(node.id); // Do not select if there are child nodes if (node.nodes.length > 0) { return; @@ -39,11 +29,7 @@ export class SidebarIndexTableOfContents extends Component { nodeId={node.id} label={( <> - {/* {canvasIds.reduce( - (acc, canvasId) => acc || (this.getAllSubTreeCanvasIds(node).indexOf(canvasId) !== -1), - false, - ) && } */} - { visibleRangeIds.indexOf(node.id) !== -1 && } + {visibleRangeIds.indexOf(node.id) !== -1 && } {node.label} )} @@ -58,10 +44,9 @@ export class SidebarIndexTableOfContents extends Component { /** */ render() { const { - canvases, classes, treeStructure, visibleRanges, + canvases, classes, treeStructure, visibleRangeIds, expandedRangeIds, } = this.props; - console.log(visibleRanges); if (!treeStructure) { return <>; } @@ -75,8 +60,9 @@ export class SidebarIndexTableOfContents extends Component { defaultCollapseIcon={} defaultExpandIcon={} defaultEndIcon={<>} + expanded={expandedRangeIds} > - {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRanges)} + {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRangeIds)} ); @@ -86,8 +72,10 @@ export class SidebarIndexTableOfContents extends Component { SidebarIndexTableOfContents.propTypes = { canvases: PropTypes.arrayOf(PropTypes.object).isRequired, classes: PropTypes.objectOf(PropTypes.string).isRequired, + expandedRangeIds: PropTypes.arrayOf(PropTypes.string).isRequired, setCanvas: PropTypes.func.isRequired, + toggleRangeNode: PropTypes.func.isRequired, treeStructure: PropTypes.objectOf().isRequired, - visibleRanges: PropTypes.arrayOf(PropTypes.string).isRequired, + visibleRangeIds: PropTypes.arrayOf(PropTypes.string).isRequired, windowId: PropTypes.string.isRequired, }; diff --git a/src/containers/SidebarIndexTableOfContents.js b/src/containers/SidebarIndexTableOfContents.js index 4b22e5d937..1b3e47de70 100644 --- a/src/containers/SidebarIndexTableOfContents.js +++ b/src/containers/SidebarIndexTableOfContents.js @@ -8,7 +8,8 @@ import { getManifestoInstance, getManifestTreeStructure, getVisibleCanvases, - getVisibleRangeIds, + getVisibleNodeIds, + getExpandedNodeIds, } from '../state/selectors'; import * as actions from '../state/actions'; @@ -18,9 +19,10 @@ import * as actions from '../state/actions'; */ const mapStateToProps = (state, { id, windowId }) => ({ canvases: getVisibleCanvases(state, { windowId }), + expandedRangeIds: getExpandedNodeIds(state, { companionWindowId: id, windowId }), manifesto: getManifestoInstance(state, { windowId }), treeStructure: getManifestTreeStructure(state, { windowId }), - visibleRangeIds: getVisibleRangeIds(state, { windowId }), + visibleRangeIds: getVisibleNodeIds(state, { windowId }), }); /** @@ -30,6 +32,7 @@ const mapStateToProps = (state, { id, windowId }) => ({ */ const mapDispatchToProps = (dispatch, { id, windowId }) => ({ setCanvas: (...args) => dispatch(actions.setCanvas(...args)), + toggleRangeNode: nodeId => dispatch(actions.toggleRangeNode(windowId, id, nodeId)), }); /** diff --git a/src/state/actions/companionWindow.js b/src/state/actions/companionWindow.js index 18f681ac75..23bfbdfe3a 100644 --- a/src/state/actions/companionWindow.js +++ b/src/state/actions/companionWindow.js @@ -57,3 +57,24 @@ export function removeCompanionWindow(windowId, id) { windowId, }; } + +/** */ +export function toggleRangeNode(windowId, id, nodeId) { + return (dispatch, getState) => { + const state = getState(); + const companionWindow = state.companionWindows[id]; + const expandedNodeIds = companionWindow.expandedNodeIds || []; + const payload = {}; + if (expandedNodeIds.indexOf(nodeId) === -1) { + payload.expandedNodeIds = [...expandedNodeIds, nodeId]; + } else { + payload.expandedNodeIds = expandedNodeIds.filter(item => nodeId !== item); + } + return dispatch({ + id, + payload, + type: ActionTypes.UPDATE_COMPANION_WINDOW, + windowId, + }); + }; +} diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index aa7b4865ce..d203a7fac4 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -1,9 +1,11 @@ import { createSelector } from 'reselect'; +import union from 'lodash/union'; import { getManifestTreeStructure } from './manifests'; import { getVisibleCanvases } from './canvases'; +import { getCompanionWindow } from './companionWindows'; /** */ -function getVisibleRangeIdsInSubTree(nodes, canvasIds) { +function getVisibleNodeIdsInSubTree(nodes, canvasIds) { return nodes.reduce((rangeIds, node) => { const currentRangeIds = []; const nodeContainsVisibleCanvas = canvasIds.reduce( @@ -12,7 +14,7 @@ function getVisibleRangeIdsInSubTree(nodes, canvasIds) { ); if (node.nodes.length > 0) { const subTreeVisibleRangeIds = node.nodes.length > 0 - ? getVisibleRangeIdsInSubTree(node.nodes, canvasIds) : []; + ? getVisibleNodeIdsInSubTree(node.nodes, canvasIds) : []; currentRangeIds.push(...subTreeVisibleRangeIds); } if (currentRangeIds.length > 0 || nodeContainsVisibleCanvas) { @@ -24,7 +26,7 @@ function getVisibleRangeIdsInSubTree(nodes, canvasIds) { } /** */ -export const getVisibleRangeIds = createSelector( +export const getVisibleNodeIds = createSelector( [ getManifestTreeStructure, getVisibleCanvases, @@ -34,6 +36,19 @@ export const getVisibleRangeIds = createSelector( return []; } const canvasIds = canvases.map(canvas => canvas.id); - return getVisibleRangeIdsInSubTree(tree.nodes, canvasIds); + return getVisibleNodeIdsInSubTree(tree.nodes, canvasIds); }, ); + +/** */ +export function getManuallyExpandedNodeIds(state, { companionWindowId }) { + const companionWindow = getCompanionWindow(state, { companionWindowId }); + return companionWindow.expandedNodeIds || []; +} + +/** */ +export function getExpandedNodeIds(state, { ...args }) { + const visibleNodeIds = getVisibleNodeIds(state, { ...args }); + const manuallyExpandedNodeIds = getManuallyExpandedNodeIds(state, { ...args }); + return union(manuallyExpandedNodeIds, visibleNodeIds); +} From 367daa51cf4776b57744f0e0589372a3a611d884 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Fri, 7 Feb 2020 17:53:40 +0100 Subject: [PATCH 04/21] Scroll to current range in TOC --- src/components/SidebarIndexTableOfContents.js | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index fab58a7f0d..a4614b855f 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -5,6 +5,7 @@ import ExpandMoreIcon from '@material-ui/icons/ExpandMore'; import ChevronRightIcon from '@material-ui/icons/ChevronRight'; import VisibilityIcon from '@material-ui/icons/Visibility'; import TreeItem from '@material-ui/lab/TreeItem'; +import { ScrollTo } from './ScrollTo'; /** */ export class SidebarIndexTableOfContents extends Component { @@ -21,21 +22,28 @@ export class SidebarIndexTableOfContents extends Component { } /** */ - buildTreeItems(nodes, canvasIds, visibleRangeIds) { + buildTreeItems(nodes, canvasIds, visibleRangeIds, containerRef) { return ( nodes.map(node => ( - {visibleRangeIds.indexOf(node.id) !== -1 && } - {node.label} - + + <> + {visibleRangeIds.indexOf(node.id) !== -1 && } + {node.label} + + )} onClick={() => this.selectTreeItem(node)} > - {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleRangeIds) : null} + {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleRangeIds, containerRef) : null} )) ); @@ -44,7 +52,7 @@ export class SidebarIndexTableOfContents extends Component { /** */ render() { const { - canvases, classes, treeStructure, visibleRangeIds, expandedRangeIds, + canvases, classes, treeStructure, visibleRangeIds, expandedRangeIds, containerRef, } = this.props; if (!treeStructure) { @@ -62,7 +70,7 @@ export class SidebarIndexTableOfContents extends Component { defaultEndIcon={<>} expanded={expandedRangeIds} > - {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRangeIds)} + {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRangeIds, containerRef)} ); @@ -72,6 +80,10 @@ export class SidebarIndexTableOfContents extends Component { SidebarIndexTableOfContents.propTypes = { canvases: PropTypes.arrayOf(PropTypes.object).isRequired, classes: PropTypes.objectOf(PropTypes.string).isRequired, + containerRef: PropTypes.oneOfType([ + PropTypes.func, + PropTypes.shape({ current: PropTypes.instanceOf(Element) }), + ]), expandedRangeIds: PropTypes.arrayOf(PropTypes.string).isRequired, setCanvas: PropTypes.func.isRequired, toggleRangeNode: PropTypes.func.isRequired, From ae462f3167b218d794f509049b447e12371724b6 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Wed, 12 Feb 2020 16:07:15 +0100 Subject: [PATCH 05/21] Use actual range ids in companion window TOC state --- src/components/SidebarIndexTableOfContents.js | 10 ++++----- src/containers/SidebarIndexTableOfContents.js | 10 ++++----- src/state/actions/companionWindow.js | 10 ++++----- src/state/selectors/ranges.js | 22 +++++++++---------- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index a4614b855f..8130e83eaf 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -11,8 +11,8 @@ import { ScrollTo } from './ScrollTo'; export class SidebarIndexTableOfContents extends Component { /** */ selectTreeItem(node) { - const { setCanvas, toggleRangeNode, windowId } = this.props; - toggleRangeNode(node.id); + const { setCanvas, toggleRange, windowId } = this.props; + toggleRange(node.data.id); // Do not select if there are child nodes if (node.nodes.length > 0) { return; @@ -27,16 +27,16 @@ export class SidebarIndexTableOfContents extends Component { nodes.map(node => ( <> - {visibleRangeIds.indexOf(node.id) !== -1 && } + {visibleRangeIds.indexOf(node.data.id) !== -1 && } {node.label} diff --git a/src/containers/SidebarIndexTableOfContents.js b/src/containers/SidebarIndexTableOfContents.js index 1b3e47de70..66ee5cc51a 100644 --- a/src/containers/SidebarIndexTableOfContents.js +++ b/src/containers/SidebarIndexTableOfContents.js @@ -8,8 +8,8 @@ import { getManifestoInstance, getManifestTreeStructure, getVisibleCanvases, - getVisibleNodeIds, - getExpandedNodeIds, + getVisibleRangeIds, + getExpandedRangeIds, } from '../state/selectors'; import * as actions from '../state/actions'; @@ -19,10 +19,10 @@ import * as actions from '../state/actions'; */ const mapStateToProps = (state, { id, windowId }) => ({ canvases: getVisibleCanvases(state, { windowId }), - expandedRangeIds: getExpandedNodeIds(state, { companionWindowId: id, windowId }), + expandedRangeIds: getExpandedRangeIds(state, { companionWindowId: id, windowId }), manifesto: getManifestoInstance(state, { windowId }), treeStructure: getManifestTreeStructure(state, { windowId }), - visibleRangeIds: getVisibleNodeIds(state, { windowId }), + visibleRangeIds: getVisibleRangeIds(state, { windowId }), }); /** @@ -32,7 +32,7 @@ const mapStateToProps = (state, { id, windowId }) => ({ */ const mapDispatchToProps = (dispatch, { id, windowId }) => ({ setCanvas: (...args) => dispatch(actions.setCanvas(...args)), - toggleRangeNode: nodeId => dispatch(actions.toggleRangeNode(windowId, id, nodeId)), + toggleRange: nodeId => dispatch(actions.toggleRange(windowId, id, nodeId)), }); /** diff --git a/src/state/actions/companionWindow.js b/src/state/actions/companionWindow.js index 23bfbdfe3a..949fad3e99 100644 --- a/src/state/actions/companionWindow.js +++ b/src/state/actions/companionWindow.js @@ -59,16 +59,16 @@ export function removeCompanionWindow(windowId, id) { } /** */ -export function toggleRangeNode(windowId, id, nodeId) { +export function toggleRange(windowId, id, rangeId) { return (dispatch, getState) => { const state = getState(); const companionWindow = state.companionWindows[id]; - const expandedNodeIds = companionWindow.expandedNodeIds || []; + const expandedRangeIds = companionWindow.expandedRangeIds || []; const payload = {}; - if (expandedNodeIds.indexOf(nodeId) === -1) { - payload.expandedNodeIds = [...expandedNodeIds, nodeId]; + if (expandedRangeIds.indexOf(rangeId) === -1) { + payload.expandedRangeIds = [...expandedRangeIds, rangeId]; } else { - payload.expandedNodeIds = expandedNodeIds.filter(item => nodeId !== item); + payload.expandedRangeIds = expandedRangeIds.filter(item => rangeId !== item); } return dispatch({ id, diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index d203a7fac4..13056c78ab 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -5,7 +5,7 @@ import { getVisibleCanvases } from './canvases'; import { getCompanionWindow } from './companionWindows'; /** */ -function getVisibleNodeIdsInSubTree(nodes, canvasIds) { +function getVisibleRangeIdsInSubTree(nodes, canvasIds) { return nodes.reduce((rangeIds, node) => { const currentRangeIds = []; const nodeContainsVisibleCanvas = canvasIds.reduce( @@ -14,11 +14,11 @@ function getVisibleNodeIdsInSubTree(nodes, canvasIds) { ); if (node.nodes.length > 0) { const subTreeVisibleRangeIds = node.nodes.length > 0 - ? getVisibleNodeIdsInSubTree(node.nodes, canvasIds) : []; + ? getVisibleRangeIdsInSubTree(node.nodes, canvasIds) : []; currentRangeIds.push(...subTreeVisibleRangeIds); } if (currentRangeIds.length > 0 || nodeContainsVisibleCanvas) { - currentRangeIds.push(node.id); + currentRangeIds.push(node.data.id); } rangeIds.push(...currentRangeIds); return rangeIds; @@ -26,7 +26,7 @@ function getVisibleNodeIdsInSubTree(nodes, canvasIds) { } /** */ -export const getVisibleNodeIds = createSelector( +export const getVisibleRangeIds = createSelector( [ getManifestTreeStructure, getVisibleCanvases, @@ -36,19 +36,19 @@ export const getVisibleNodeIds = createSelector( return []; } const canvasIds = canvases.map(canvas => canvas.id); - return getVisibleNodeIdsInSubTree(tree.nodes, canvasIds); + return getVisibleRangeIdsInSubTree(tree.nodes, canvasIds); }, ); /** */ -export function getManuallyExpandedNodeIds(state, { companionWindowId }) { +export function getManuallyExpandedRangeIds(state, { companionWindowId }) { const companionWindow = getCompanionWindow(state, { companionWindowId }); - return companionWindow.expandedNodeIds || []; + return companionWindow.expandedRangeIds || []; } /** */ -export function getExpandedNodeIds(state, { ...args }) { - const visibleNodeIds = getVisibleNodeIds(state, { ...args }); - const manuallyExpandedNodeIds = getManuallyExpandedNodeIds(state, { ...args }); - return union(manuallyExpandedNodeIds, visibleNodeIds); +export function getExpandedRangeIds(state, { ...args }) { + const visibleRangeIds = getVisibleRangeIds(state, { ...args }); + const manuallyExpandedRangeIds = getManuallyExpandedRangeIds(state, { ...args }); + return union(manuallyExpandedRangeIds, visibleRangeIds); } From bdece1a9fe80f2d80f02b9338c089050b6ed251b Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Thu, 13 Feb 2020 14:42:16 +0100 Subject: [PATCH 06/21] Toggle nodes with keyboard interaction onClick and onKeyDown should be replaced once material-ui TreeItem has node selection support: https://github.com/mui-org/material-ui/issues/16795 --- src/components/SidebarIndexTableOfContents.js | 13 +++- src/state/selectors/ranges.js | 63 +++++++++++++++---- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index 8130e83eaf..c018c82ee1 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -12,15 +12,25 @@ export class SidebarIndexTableOfContents extends Component { /** */ selectTreeItem(node) { const { setCanvas, toggleRange, windowId } = this.props; - toggleRange(node.data.id); // Do not select if there are child nodes if (node.nodes.length > 0) { + toggleRange(node.data.id); return; } const canvas = node.data.getCanvasIds()[0]; setCanvas(windowId, canvas); } + /** */ + handleKeyPressed(event, node) { + if (event.key === 'Enter' + || event.key === ' ' + || event.key === 'ArrowLeft' && this.props.expandedRangeIds.indexOf(node.data.id) !== -1 + || event.key === 'ArrowRight' && this.props.expandedRangeIds.indexOf(node.data.id) === -1) { + this.selectTreeItem(node); + } + } + /** */ buildTreeItems(nodes, canvasIds, visibleRangeIds, containerRef) { return ( @@ -42,6 +52,7 @@ export class SidebarIndexTableOfContents extends Component { )} onClick={() => this.selectTreeItem(node)} + onKeyDown={e => this.handleKeyPressed(e, node)} > {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleRangeIds, containerRef) : null} diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index 13056c78ab..e141ffd4b8 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -6,40 +6,77 @@ import { getCompanionWindow } from './companionWindows'; /** */ function getVisibleRangeIdsInSubTree(nodes, canvasIds) { - return nodes.reduce((rangeIds, node) => { - const currentRangeIds = []; + return nodes.reduce((rangeIdAcc, node) => { + const currentBranchRangeIds = []; + const currentLeafRangeIds = []; const nodeContainsVisibleCanvas = canvasIds.reduce( (acc, canvasId) => acc || node.data.getCanvasIds().indexOf(canvasId) !== -1, false, ); if (node.nodes.length > 0) { const subTreeVisibleRangeIds = node.nodes.length > 0 - ? getVisibleRangeIdsInSubTree(node.nodes, canvasIds) : []; - currentRangeIds.push(...subTreeVisibleRangeIds); + ? getVisibleRangeIdsInSubTree(node.nodes, canvasIds) : { + branchRangeIds: [], + leafRangeIds: [], + }; + currentBranchRangeIds.push(...subTreeVisibleRangeIds.branchRangeIds); + currentLeafRangeIds.push(...subTreeVisibleRangeIds.leafRangeIds); } - if (currentRangeIds.length > 0 || nodeContainsVisibleCanvas) { - currentRangeIds.push(node.data.id); + if (currentBranchRangeIds.length > 0 + || currentLeafRangeIds.length > 0 + || nodeContainsVisibleCanvas) { + if (node.nodes.length > 0) { + currentBranchRangeIds.push(node.data.id); + } else { + currentLeafRangeIds.push(node.data.id); + } } - rangeIds.push(...currentRangeIds); - return rangeIds; - }, []); + rangeIdAcc.branchRangeIds.push(...currentBranchRangeIds); + rangeIdAcc.leafRangeIds.push(...currentLeafRangeIds); + return rangeIdAcc; + }, { + branchRangeIds: [], + leafRangeIds: [], + }); } /** */ -export const getVisibleRangeIds = createSelector( +const getVisibleLeafAndBranchRangeIds = createSelector( [ getManifestTreeStructure, getVisibleCanvases, ], (tree, canvases) => { if (!canvases) { - return []; + return { + branchRangeIds: [], + leafRangeIds: [], + }; } const canvasIds = canvases.map(canvas => canvas.id); return getVisibleRangeIdsInSubTree(tree.nodes, canvasIds); }, ); +/** */ +export const getVisibleRangeIds = createSelector( + [ + getVisibleLeafAndBranchRangeIds, + ], + (visibleLeafAndBranchRangeIds) => { + return union(visibleLeafAndBranchRangeIds.leafRangeIds, visibleLeafAndBranchRangeIds.branchRangeIds); + }, +); + +const getVisibleBranchRangeIds = createSelector( + [ + getVisibleLeafAndBranchRangeIds, + ], + (visibleLeafAndBranchRangeIds) => { + return visibleLeafAndBranchRangeIds.branchRangeIds; + }, +); + /** */ export function getManuallyExpandedRangeIds(state, { companionWindowId }) { const companionWindow = getCompanionWindow(state, { companionWindowId }); @@ -48,7 +85,7 @@ export function getManuallyExpandedRangeIds(state, { companionWindowId }) { /** */ export function getExpandedRangeIds(state, { ...args }) { - const visibleRangeIds = getVisibleRangeIds(state, { ...args }); + const visibleBranchRangeIds = getVisibleBranchRangeIds(state, { ...args }); const manuallyExpandedRangeIds = getManuallyExpandedRangeIds(state, { ...args }); - return union(manuallyExpandedRangeIds, visibleRangeIds); + return union(manuallyExpandedRangeIds, visibleBranchRangeIds); } From cfdfc132071f8b53b175b8a0db73448f3e04ca78 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Fri, 14 Feb 2020 14:35:48 +0100 Subject: [PATCH 07/21] Take URL fragments in Range.canvases into account --- src/components/SidebarIndexTableOfContents.js | 9 ++++++--- src/state/selectors/ranges.js | 14 +++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index c018c82ee1..e54ede4781 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -12,13 +12,16 @@ export class SidebarIndexTableOfContents extends Component { /** */ selectTreeItem(node) { const { setCanvas, toggleRange, windowId } = this.props; - // Do not select if there are child nodes if (node.nodes.length > 0) { toggleRange(node.data.id); + } + // Do not select if there are no canvases listed + if (!node.data.getCanvasIds() || node.data.getCanvasIds().length === 0) { return; } - const canvas = node.data.getCanvasIds()[0]; - setCanvas(windowId, canvas); + const target = node.data.getCanvasIds()[0]; + const canvasId = target.indexOf('#') === -1 ? target : target.substr(0, target.indexOf('#')); + setCanvas(windowId, canvasId); } /** */ diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index e141ffd4b8..e82fda550c 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -1,16 +1,28 @@ import { createSelector } from 'reselect'; import union from 'lodash/union'; +import { Utils } from 'manifesto.js'; import { getManifestTreeStructure } from './manifests'; import { getVisibleCanvases } from './canvases'; import { getCompanionWindow } from './companionWindows'; +/** */ +function rangeContainsCanvasId(range, canvasId) { + const canvasIds = range.getCanvasIds(); + for (let i = 0; i < canvasIds.length; i += 1) { + if (Utils.normalisedUrlsMatch(canvasIds[i], canvasId)) { + return true; + } + } + return false; +} + /** */ function getVisibleRangeIdsInSubTree(nodes, canvasIds) { return nodes.reduce((rangeIdAcc, node) => { const currentBranchRangeIds = []; const currentLeafRangeIds = []; const nodeContainsVisibleCanvas = canvasIds.reduce( - (acc, canvasId) => acc || node.data.getCanvasIds().indexOf(canvasId) !== -1, + (acc, canvasId) => acc || rangeContainsCanvasId(node.data, canvasId), false, ); if (node.nodes.length > 0) { From 4645fc2e10bff26aa08c141ebe86b6e8beb54566 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Fri, 14 Feb 2020 15:37:00 +0100 Subject: [PATCH 08/21] Have only one Range to scroll to --- src/components/SidebarIndexTableOfContents.js | 9 ++--- src/containers/SidebarIndexTableOfContents.js | 4 ++- src/state/selectors/ranges.js | 33 +++++++++++++++---- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index e54ede4781..f9b68ec797 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -35,7 +35,7 @@ export class SidebarIndexTableOfContents extends Component { } /** */ - buildTreeItems(nodes, canvasIds, visibleRangeIds, containerRef) { + buildTreeItems(nodes, canvasIds, visibleRangeIds, containerRef, rangeIdToScrollTo) { return ( nodes.map(node => ( <> {visibleRangeIds.indexOf(node.data.id) !== -1 && } @@ -66,7 +66,7 @@ export class SidebarIndexTableOfContents extends Component { /** */ render() { const { - canvases, classes, treeStructure, visibleRangeIds, expandedRangeIds, containerRef, + canvases, classes, treeStructure, visibleRangeIds, expandedRangeIds, containerRef, rangeIdToScrollTo, } = this.props; if (!treeStructure) { @@ -84,7 +84,7 @@ export class SidebarIndexTableOfContents extends Component { defaultEndIcon={<>} expanded={expandedRangeIds} > - {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRangeIds, containerRef)} + {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRangeIds, containerRef, rangeIdToScrollTo)} ); @@ -100,6 +100,7 @@ SidebarIndexTableOfContents.propTypes = { ]), expandedRangeIds: PropTypes.arrayOf(PropTypes.string).isRequired, setCanvas: PropTypes.func.isRequired, + rangeIdToScrollTo: PropTypes.func.isRequired, toggleRangeNode: PropTypes.func.isRequired, treeStructure: PropTypes.objectOf().isRequired, visibleRangeIds: PropTypes.arrayOf(PropTypes.string).isRequired, diff --git a/src/containers/SidebarIndexTableOfContents.js b/src/containers/SidebarIndexTableOfContents.js index 66ee5cc51a..1018857668 100644 --- a/src/containers/SidebarIndexTableOfContents.js +++ b/src/containers/SidebarIndexTableOfContents.js @@ -10,6 +10,7 @@ import { getVisibleCanvases, getVisibleRangeIds, getExpandedRangeIds, + getRangeIdToScrollTo, } from '../state/selectors'; import * as actions from '../state/actions'; @@ -21,8 +22,9 @@ const mapStateToProps = (state, { id, windowId }) => ({ canvases: getVisibleCanvases(state, { windowId }), expandedRangeIds: getExpandedRangeIds(state, { companionWindowId: id, windowId }), manifesto: getManifestoInstance(state, { windowId }), + rangeIdToScrollTo: getRangeIdToScrollTo(state, { companionWindowId: id, windowId }), treeStructure: getManifestTreeStructure(state, { windowId }), - visibleRangeIds: getVisibleRangeIds(state, { windowId }), + visibleRangeIds: getVisibleRangeIds(state, { companionWindowId: id, windowId }), }); /** diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index e82fda550c..07cdc27084 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -21,6 +21,7 @@ function getVisibleRangeIdsInSubTree(nodes, canvasIds) { return nodes.reduce((rangeIdAcc, node) => { const currentBranchRangeIds = []; const currentLeafRangeIds = []; + const currentCanvasRangeIds = []; const nodeContainsVisibleCanvas = canvasIds.reduce( (acc, canvasId) => acc || rangeContainsCanvasId(node.data, canvasId), false, @@ -29,10 +30,12 @@ function getVisibleRangeIdsInSubTree(nodes, canvasIds) { const subTreeVisibleRangeIds = node.nodes.length > 0 ? getVisibleRangeIdsInSubTree(node.nodes, canvasIds) : { branchRangeIds: [], + canvasRangeIds: [], leafRangeIds: [], }; currentBranchRangeIds.push(...subTreeVisibleRangeIds.branchRangeIds); currentLeafRangeIds.push(...subTreeVisibleRangeIds.leafRangeIds); + currentCanvasRangeIds.push(...subTreeVisibleRangeIds.canvasRangeIds); } if (currentBranchRangeIds.length > 0 || currentLeafRangeIds.length > 0 @@ -42,12 +45,17 @@ function getVisibleRangeIdsInSubTree(nodes, canvasIds) { } else { currentLeafRangeIds.push(node.data.id); } + if (nodeContainsVisibleCanvas) { + currentCanvasRangeIds.push(node.data.id); + } } rangeIdAcc.branchRangeIds.push(...currentBranchRangeIds); + rangeIdAcc.canvasRangeIds.push(...currentCanvasRangeIds); rangeIdAcc.leafRangeIds.push(...currentLeafRangeIds); return rangeIdAcc; }, { branchRangeIds: [], + canvasRangeIds: [], leafRangeIds: [], }); } @@ -62,6 +70,7 @@ const getVisibleLeafAndBranchRangeIds = createSelector( if (!canvases) { return { branchRangeIds: [], + canvasRangeIds: [], leafRangeIds: [], }; } @@ -75,18 +84,21 @@ export const getVisibleRangeIds = createSelector( [ getVisibleLeafAndBranchRangeIds, ], - (visibleLeafAndBranchRangeIds) => { - return union(visibleLeafAndBranchRangeIds.leafRangeIds, visibleLeafAndBranchRangeIds.branchRangeIds); - }, + visibleLeafAndBranchRangeIds => union(visibleLeafAndBranchRangeIds.leafRangeIds, visibleLeafAndBranchRangeIds.branchRangeIds), ); const getVisibleBranchRangeIds = createSelector( [ getVisibleLeafAndBranchRangeIds, ], - (visibleLeafAndBranchRangeIds) => { - return visibleLeafAndBranchRangeIds.branchRangeIds; - }, + visibleLeafAndBranchRangeIds => visibleLeafAndBranchRangeIds.branchRangeIds, +); + +const getCanvasContainingRangeIds = createSelector( + [ + getVisibleLeafAndBranchRangeIds, + ], + visibleLeafAndBranchRangeIds => visibleLeafAndBranchRangeIds.canvasRangeIds, ); /** */ @@ -101,3 +113,12 @@ export function getExpandedRangeIds(state, { ...args }) { const manuallyExpandedRangeIds = getManuallyExpandedRangeIds(state, { ...args }); return union(manuallyExpandedRangeIds, visibleBranchRangeIds); } + +/** */ +export function getRangeIdToScrollTo(state, { ...args }) { + const canvasContainingRangeIds = getCanvasContainingRangeIds(state, { ...args }); + if (canvasContainingRangeIds) { + return canvasContainingRangeIds[0]; + } + return null; +} From 0a081c4ef5bed385321d693d8c8f0f20e0b07aae Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Fri, 14 Feb 2020 15:42:48 +0100 Subject: [PATCH 09/21] Add manifests for TOC examples in M3 wiki See https://github.com/ProjectMirador/mirador/wiki/M3-Fixtures-&-sample-IIIF-data#table-of-contents-and-sequences --- __tests__/integration/mirador/toc.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/__tests__/integration/mirador/toc.html b/__tests__/integration/mirador/toc.html index 875765b48b..04330f477f 100644 --- a/__tests__/integration/mirador/toc.html +++ b/__tests__/integration/mirador/toc.html @@ -22,6 +22,13 @@ defaultSideBarPanel: 'canvas' }, manifests: { + 'https://iiif.bodleian.ox.ac.uk/iiif/manifest/390fd0e8-9eae-475d-9564-ed916ab9035c.json': { provider: 'Bodleian Libraries' }, + 'http://dams.llgc.org.uk/iiif/newspaper/issue/3100021/manifest.json': { provider: 'The National Library of Wales' }, + 'https://iiif.lib.harvard.edu/manifests/drs:5981093': { provider: 'Houghton Library (Harvard University)' }, + 'https://cudl.lib.cam.ac.uk/iiif/MS-ADD-03965' : {}, + 'https://iiif.bodleian.ox.ac.uk/iiif/manifest/ca3dc326-4a7b-479f-a754-5aed9d2f2cb4.json': {}, + // 'https://gist.githubusercontent.com/jeffreycwitt/90b33c1c4e119e7a48b7a66ea41a48c1/raw/522b132409d6c67a78f8f26b0ceb7346a52cfe62/test-manifest-with-complicated-toc.json': {}, + // 'https://gist.githubusercontent.com/jeffreycwitt/1a75fdb4a97e1c2a98bd35797aad263d/raw/859104cb6cd7bd99f3be668f064066f4b3ba2b29/manifest-with-three-level-deep-toc.json': {}, } }); From 280490181cb8e64015e2b4e42ad6dacab00c2544 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Tue, 18 Feb 2020 12:35:53 +0100 Subject: [PATCH 10/21] Use node ids instead of Range ids for TOC A range might appear more than once in the structure tree, so it's id is not unique to the tree. --- src/components/SidebarIndexTableOfContents.js | 32 +++--- src/containers/SidebarIndexTableOfContents.js | 14 +-- src/state/actions/companionWindow.js | 2 +- src/state/selectors/ranges.js | 104 ++++++++---------- 4 files changed, 67 insertions(+), 85 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index f9b68ec797..fd6723be40 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -11,9 +11,9 @@ import { ScrollTo } from './ScrollTo'; export class SidebarIndexTableOfContents extends Component { /** */ selectTreeItem(node) { - const { setCanvas, toggleRange, windowId } = this.props; + const { setCanvas, toggleNode, windowId } = this.props; if (node.nodes.length > 0) { - toggleRange(node.data.id); + toggleNode(node.id); } // Do not select if there are no canvases listed if (!node.data.getCanvasIds() || node.data.getCanvasIds().length === 0) { @@ -28,28 +28,28 @@ export class SidebarIndexTableOfContents extends Component { handleKeyPressed(event, node) { if (event.key === 'Enter' || event.key === ' ' - || event.key === 'ArrowLeft' && this.props.expandedRangeIds.indexOf(node.data.id) !== -1 - || event.key === 'ArrowRight' && this.props.expandedRangeIds.indexOf(node.data.id) === -1) { + || event.key === 'ArrowLeft' && this.props.expandedNodeIds.indexOf(node.id) !== -1 + || event.key === 'ArrowRight' && this.props.expandedNodeIds.indexOf(node.id) === -1) { this.selectTreeItem(node); } } /** */ - buildTreeItems(nodes, canvasIds, visibleRangeIds, containerRef, rangeIdToScrollTo) { + buildTreeItems(nodes, canvasIds, visibleNodeIds, containerRef, nodeIdToScrollTo) { return ( nodes.map(node => ( <> - {visibleRangeIds.indexOf(node.data.id) !== -1 && } + {visibleNodeIds.indexOf(node.id) !== -1 && } {node.label} @@ -57,7 +57,7 @@ export class SidebarIndexTableOfContents extends Component { onClick={() => this.selectTreeItem(node)} onKeyDown={e => this.handleKeyPressed(e, node)} > - {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleRangeIds, containerRef) : null} + {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleNodeIds, containerRef) : null} )) ); @@ -66,7 +66,7 @@ export class SidebarIndexTableOfContents extends Component { /** */ render() { const { - canvases, classes, treeStructure, visibleRangeIds, expandedRangeIds, containerRef, rangeIdToScrollTo, + canvases, classes, treeStructure, visibleNodeIds, expandedNodeIds, containerRef, nodeIdToScrollTo, } = this.props; if (!treeStructure) { @@ -82,9 +82,9 @@ export class SidebarIndexTableOfContents extends Component { defaultCollapseIcon={} defaultExpandIcon={} defaultEndIcon={<>} - expanded={expandedRangeIds} + expanded={expandedNodeIds} > - {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleRangeIds, containerRef, rangeIdToScrollTo)} + {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleNodeIds, containerRef, nodeIdToScrollTo)} ); @@ -98,11 +98,11 @@ SidebarIndexTableOfContents.propTypes = { PropTypes.func, PropTypes.shape({ current: PropTypes.instanceOf(Element) }), ]), - expandedRangeIds: PropTypes.arrayOf(PropTypes.string).isRequired, + expandedNodeIds: PropTypes.arrayOf(PropTypes.string).isRequired, setCanvas: PropTypes.func.isRequired, - rangeIdToScrollTo: PropTypes.func.isRequired, - toggleRangeNode: PropTypes.func.isRequired, + nodeIdToScrollTo: PropTypes.func.isRequired, + toggleNode: PropTypes.func.isRequired, treeStructure: PropTypes.objectOf().isRequired, - visibleRangeIds: PropTypes.arrayOf(PropTypes.string).isRequired, + visibleNodeIds: PropTypes.arrayOf(PropTypes.string).isRequired, windowId: PropTypes.string.isRequired, }; diff --git a/src/containers/SidebarIndexTableOfContents.js b/src/containers/SidebarIndexTableOfContents.js index 1018857668..1104700310 100644 --- a/src/containers/SidebarIndexTableOfContents.js +++ b/src/containers/SidebarIndexTableOfContents.js @@ -8,9 +8,9 @@ import { getManifestoInstance, getManifestTreeStructure, getVisibleCanvases, - getVisibleRangeIds, - getExpandedRangeIds, - getRangeIdToScrollTo, + getVisibleNodeIds, + getExpandedNodeIds, + getNodeIdToScrollTo, } from '../state/selectors'; import * as actions from '../state/actions'; @@ -20,11 +20,11 @@ import * as actions from '../state/actions'; */ const mapStateToProps = (state, { id, windowId }) => ({ canvases: getVisibleCanvases(state, { windowId }), - expandedRangeIds: getExpandedRangeIds(state, { companionWindowId: id, windowId }), + expandedNodeIds: getExpandedNodeIds(state, { companionWindowId: id, windowId }), manifesto: getManifestoInstance(state, { windowId }), - rangeIdToScrollTo: getRangeIdToScrollTo(state, { companionWindowId: id, windowId }), + nodeIdToScrollTo: getNodeIdToScrollTo(state, { companionWindowId: id, windowId }), treeStructure: getManifestTreeStructure(state, { windowId }), - visibleRangeIds: getVisibleRangeIds(state, { companionWindowId: id, windowId }), + visibleNodeIds: getVisibleNodeIds(state, { companionWindowId: id, windowId }), }); /** @@ -34,7 +34,7 @@ const mapStateToProps = (state, { id, windowId }) => ({ */ const mapDispatchToProps = (dispatch, { id, windowId }) => ({ setCanvas: (...args) => dispatch(actions.setCanvas(...args)), - toggleRange: nodeId => dispatch(actions.toggleRange(windowId, id, nodeId)), + toggleNode: nodeId => dispatch(actions.toggleNode(windowId, id, nodeId)), }); /** diff --git a/src/state/actions/companionWindow.js b/src/state/actions/companionWindow.js index 949fad3e99..04ba9965ae 100644 --- a/src/state/actions/companionWindow.js +++ b/src/state/actions/companionWindow.js @@ -59,7 +59,7 @@ export function removeCompanionWindow(windowId, id) { } /** */ -export function toggleRange(windowId, id, rangeId) { +export function toggleNode(windowId, id, rangeId) { return (dispatch, getState) => { const state = getState(); const companionWindow = state.companionWindows[id]; diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index 07cdc27084..48f7ac6bf0 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -17,108 +17,90 @@ function rangeContainsCanvasId(range, canvasId) { } /** */ -function getVisibleRangeIdsInSubTree(nodes, canvasIds) { - return nodes.reduce((rangeIdAcc, node) => { - const currentBranchRangeIds = []; - const currentLeafRangeIds = []; - const currentCanvasRangeIds = []; +function getVisibleNodeIdsInSubTree(nodes, canvasIds) { + return nodes.reduce((nodeIdAcc, node) => { + const result = []; const nodeContainsVisibleCanvas = canvasIds.reduce( (acc, canvasId) => acc || rangeContainsCanvasId(node.data, canvasId), false, ); - if (node.nodes.length > 0) { - const subTreeVisibleRangeIds = node.nodes.length > 0 - ? getVisibleRangeIdsInSubTree(node.nodes, canvasIds) : { - branchRangeIds: [], - canvasRangeIds: [], - leafRangeIds: [], - }; - currentBranchRangeIds.push(...subTreeVisibleRangeIds.branchRangeIds); - currentLeafRangeIds.push(...subTreeVisibleRangeIds.leafRangeIds); - currentCanvasRangeIds.push(...subTreeVisibleRangeIds.canvasRangeIds); + const subTreeVisibleNodeIds = node.nodes.length > 0 + ? getVisibleNodeIdsInSubTree(node.nodes, canvasIds) + : []; + if (nodeContainsVisibleCanvas || subTreeVisibleNodeIds.length > 0) { + result.push({ + containsVisibleCanvas: nodeContainsVisibleCanvas, + id: node.id, + leaf: node.nodes.length === 0, + }); } - if (currentBranchRangeIds.length > 0 - || currentLeafRangeIds.length > 0 - || nodeContainsVisibleCanvas) { - if (node.nodes.length > 0) { - currentBranchRangeIds.push(node.data.id); - } else { - currentLeafRangeIds.push(node.data.id); - } - if (nodeContainsVisibleCanvas) { - currentCanvasRangeIds.push(node.data.id); - } - } - rangeIdAcc.branchRangeIds.push(...currentBranchRangeIds); - rangeIdAcc.canvasRangeIds.push(...currentCanvasRangeIds); - rangeIdAcc.leafRangeIds.push(...currentLeafRangeIds); - return rangeIdAcc; - }, { - branchRangeIds: [], - canvasRangeIds: [], - leafRangeIds: [], - }); + result.push(...nodeIdAcc); + result.push(...subTreeVisibleNodeIds); + return result; + }, []); } /** */ -const getVisibleLeafAndBranchRangeIds = createSelector( +const getVisibleLeafAndBranchNodeIds = createSelector( [ getManifestTreeStructure, getVisibleCanvases, ], (tree, canvases) => { if (!canvases) { - return { - branchRangeIds: [], - canvasRangeIds: [], - leafRangeIds: [], - }; + return []; } const canvasIds = canvases.map(canvas => canvas.id); - return getVisibleRangeIdsInSubTree(tree.nodes, canvasIds); + return getVisibleNodeIdsInSubTree(tree.nodes, canvasIds); }, ); /** */ -export const getVisibleRangeIds = createSelector( +export const getVisibleNodeIds = createSelector( [ - getVisibleLeafAndBranchRangeIds, + getVisibleLeafAndBranchNodeIds, ], - visibleLeafAndBranchRangeIds => union(visibleLeafAndBranchRangeIds.leafRangeIds, visibleLeafAndBranchRangeIds.branchRangeIds), + visibleLeafAndBranchNodeIds => visibleLeafAndBranchNodeIds.map(item => item.id), ); -const getVisibleBranchRangeIds = createSelector( +const getVisibleBranchNodeIds = createSelector( [ - getVisibleLeafAndBranchRangeIds, + getVisibleLeafAndBranchNodeIds, ], - visibleLeafAndBranchRangeIds => visibleLeafAndBranchRangeIds.branchRangeIds, + visibleLeafAndBranchNodeIds => visibleLeafAndBranchNodeIds.reduce( + (acc, item) => (item.leaf ? acc : [...acc, item.id]), + [], + ), ); -const getCanvasContainingRangeIds = createSelector( +const getCanvasContainingNodeIds = createSelector( [ - getVisibleLeafAndBranchRangeIds, + getVisibleLeafAndBranchNodeIds, ], - visibleLeafAndBranchRangeIds => visibleLeafAndBranchRangeIds.canvasRangeIds, + visibleLeafAndBranchNodeIds => visibleLeafAndBranchNodeIds.reduce( + (acc, item) => (item.containsVisibleCanvas ? [...acc, item.id] : acc), + [], + ), ); /** */ -export function getManuallyExpandedRangeIds(state, { companionWindowId }) { +export function getManuallyExpandedNodeIds(state, { companionWindowId }) { const companionWindow = getCompanionWindow(state, { companionWindowId }); return companionWindow.expandedRangeIds || []; } /** */ -export function getExpandedRangeIds(state, { ...args }) { - const visibleBranchRangeIds = getVisibleBranchRangeIds(state, { ...args }); - const manuallyExpandedRangeIds = getManuallyExpandedRangeIds(state, { ...args }); - return union(manuallyExpandedRangeIds, visibleBranchRangeIds); +export function getExpandedNodeIds(state, { ...args }) { + const visibleBranchNodeIds = getVisibleBranchNodeIds(state, { ...args }); + const manuallyExpandedNodeIds = getManuallyExpandedNodeIds(state, { ...args }); + return union(manuallyExpandedNodeIds, visibleBranchNodeIds); } /** */ -export function getRangeIdToScrollTo(state, { ...args }) { - const canvasContainingRangeIds = getCanvasContainingRangeIds(state, { ...args }); - if (canvasContainingRangeIds) { - return canvasContainingRangeIds[0]; +export function getNodeIdToScrollTo(state, { ...args }) { + const canvasContainingNodeIds = getCanvasContainingNodeIds(state, { ...args }); + if (canvasContainingNodeIds) { + return canvasContainingNodeIds[0]; } return null; } From 0ec146f2c7600012f0983a2fcea709e7b59a94db Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Tue, 18 Feb 2020 16:03:12 +0100 Subject: [PATCH 11/21] Allow automatically opened TOC nodes to be closed --- src/state/actions/action-types.js | 1 + src/state/actions/companionWindow.js | 18 ++++++------------ src/state/reducers/companionWindows.js | 8 ++++++++ src/state/selectors/ranges.js | 19 +++++++++++++------ 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/state/actions/action-types.js b/src/state/actions/action-types.js index 3b67e7f8d0..f5c1633784 100644 --- a/src/state/actions/action-types.js +++ b/src/state/actions/action-types.js @@ -3,6 +3,7 @@ const ActionTypes = { ADD_COMPANION_WINDOW: 'ADD_COMPANION_WINDOW', UPDATE_COMPANION_WINDOW: 'UPDATE_COMPANION_WINDOW', REMOVE_COMPANION_WINDOW: 'REMOVE_COMPANION_WINDOW', + TOGGLE_TOC_NODE: 'TOGGLE_TOC_NODE', UPDATE_WINDOW: 'UPDATE_WINDOW', HIGHLIGHT_ANNOTATION: 'HIGHLIGHT_ANNOTATION', diff --git a/src/state/actions/companionWindow.js b/src/state/actions/companionWindow.js index 04ba9965ae..0272283bf6 100644 --- a/src/state/actions/companionWindow.js +++ b/src/state/actions/companionWindow.js @@ -1,6 +1,6 @@ import uuid from 'uuid/v4'; import ActionTypes from './action-types'; -import { getCompanionWindowIdsForPosition } from '../selectors'; +import { getCompanionWindowIdsForPosition, getVisibleNodeIds } from '../selectors'; const defaultProps = { content: null, @@ -59,21 +59,15 @@ export function removeCompanionWindow(windowId, id) { } /** */ -export function toggleNode(windowId, id, rangeId) { +export function toggleNode(windowId, id, nodeId) { return (dispatch, getState) => { const state = getState(); - const companionWindow = state.companionWindows[id]; - const expandedRangeIds = companionWindow.expandedRangeIds || []; - const payload = {}; - if (expandedRangeIds.indexOf(rangeId) === -1) { - payload.expandedRangeIds = [...expandedRangeIds, rangeId]; - } else { - payload.expandedRangeIds = expandedRangeIds.filter(item => rangeId !== item); - } + const visibleNodeIds = getVisibleNodeIds(state, { id, windowId }); return dispatch({ id, - payload, - type: ActionTypes.UPDATE_COMPANION_WINDOW, + nodeId, + type: ActionTypes.TOGGLE_TOC_NODE, + visibleNodeIds, windowId, }); }; diff --git a/src/state/reducers/companionWindows.js b/src/state/reducers/companionWindows.js index 26b98ff786..5460d912c2 100644 --- a/src/state/reducers/companionWindows.js +++ b/src/state/reducers/companionWindows.js @@ -25,6 +25,14 @@ export function companionWindowsReducer(state = {}, action) { return removeIn(state, [action.id]); case ActionTypes.IMPORT_MIRADOR_STATE: return action.state.companionWindows; + case ActionTypes.TOGGLE_TOC_NODE: + return updateIn(state, [[action.id], 'tocNodes'], {}, orig => merge(orig, { + [action.nodeId]: state[action.id].tocNodes && state[action.id].tocNodes[action.nodeId] ? { + expanded: !state[action.id].tocNodes[action.nodeId].expanded, + } : { + expanded: !(action.visibleNodeIds && action.visibleNodeIds.indexOf(action.nodeId) !== -1), + }, + })); default: return state; } diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index 48f7ac6bf0..55b93d7377 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -1,5 +1,6 @@ import { createSelector } from 'reselect'; import union from 'lodash/union'; +import without from 'lodash/without'; import { Utils } from 'manifesto.js'; import { getManifestTreeStructure } from './manifests'; import { getVisibleCanvases } from './canvases'; @@ -84,16 +85,22 @@ const getCanvasContainingNodeIds = createSelector( ); /** */ -export function getManuallyExpandedNodeIds(state, { companionWindowId }) { +export function getManuallyExpandedNodeIds(state, { companionWindowId }, expanded) { const companionWindow = getCompanionWindow(state, { companionWindowId }); - return companionWindow.expandedRangeIds || []; + return companionWindow.tocNodes ? Object.keys(companionWindow.tocNodes).reduce( + (acc, nodeId) => (companionWindow.tocNodes[nodeId].expanded === expanded + ? [...acc, nodeId] + : acc), + [], + ) : []; } /** */ -export function getExpandedNodeIds(state, { ...args }) { - const visibleBranchNodeIds = getVisibleBranchNodeIds(state, { ...args }); - const manuallyExpandedNodeIds = getManuallyExpandedNodeIds(state, { ...args }); - return union(manuallyExpandedNodeIds, visibleBranchNodeIds); +export function getExpandedNodeIds(state, { companionWindowId, windowId }) { + const visibleBranchNodeIds = getVisibleBranchNodeIds(state, { companionWindowId, windowId }); + const manuallyExpandedNodeIds = getManuallyExpandedNodeIds(state, { companionWindowId }, true); + const manuallyClosedNodeIds = getManuallyExpandedNodeIds(state, { companionWindowId }, false); + return without(union(manuallyExpandedNodeIds, visibleBranchNodeIds), ...manuallyClosedNodeIds); } /** */ From bff67bd90fa23c82f03132e9a50634eeaac75d41 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Wed, 19 Feb 2020 17:18:51 +0100 Subject: [PATCH 12/21] Make sure to scroll to first treenode that contains a canvas --- src/components/SidebarIndexTableOfContents.js | 2 +- src/state/selectors/ranges.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index fd6723be40..b4959b3fd3 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -57,7 +57,7 @@ export class SidebarIndexTableOfContents extends Component { onClick={() => this.selectTreeItem(node)} onKeyDown={e => this.handleKeyPressed(e, node)} > - {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleNodeIds, containerRef) : null} + {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleNodeIds, containerRef, nodeIdToScrollTo) : null} )) ); diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index 55b93d7377..b639153f20 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -21,6 +21,7 @@ function rangeContainsCanvasId(range, canvasId) { function getVisibleNodeIdsInSubTree(nodes, canvasIds) { return nodes.reduce((nodeIdAcc, node) => { const result = []; + result.push(...nodeIdAcc); const nodeContainsVisibleCanvas = canvasIds.reduce( (acc, canvasId) => acc || rangeContainsCanvasId(node.data, canvasId), false, @@ -35,7 +36,6 @@ function getVisibleNodeIdsInSubTree(nodes, canvasIds) { leaf: node.nodes.length === 0, }); } - result.push(...nodeIdAcc); result.push(...subTreeVisibleNodeIds); return result; }, []); From d5b41a224495d14bc228fdda6514eb0592d1f657 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Fri, 28 Feb 2020 16:54:41 +0100 Subject: [PATCH 13/21] Remove unnecessary props in SidebarIndexTableOfContents --- src/components/SidebarIndexTableOfContents.js | 14 +++++++------- src/containers/SidebarIndexTableOfContents.js | 4 ---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index b4959b3fd3..65b70d176f 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -35,7 +35,10 @@ export class SidebarIndexTableOfContents extends Component { } /** */ - buildTreeItems(nodes, canvasIds, visibleNodeIds, containerRef, nodeIdToScrollTo) { + buildTreeItems(nodes, visibleNodeIds, containerRef, nodeIdToScrollTo) { + if (!nodes) { + return null; + } return ( nodes.map(node => ( this.selectTreeItem(node)} onKeyDown={e => this.handleKeyPressed(e, node)} > - {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, canvasIds, visibleNodeIds, containerRef, nodeIdToScrollTo) : null} + {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, visibleNodeIds, containerRef, nodeIdToScrollTo) : null} )) ); @@ -66,15 +69,13 @@ export class SidebarIndexTableOfContents extends Component { /** */ render() { const { - canvases, classes, treeStructure, visibleNodeIds, expandedNodeIds, containerRef, nodeIdToScrollTo, + classes, treeStructure, visibleNodeIds, expandedNodeIds, containerRef, nodeIdToScrollTo, } = this.props; if (!treeStructure) { return <>; } - const canvasIds = canvases.map(canvas => canvas.id); - return ( <> } expanded={expandedNodeIds} > - {this.buildTreeItems(treeStructure.nodes, canvasIds, visibleNodeIds, containerRef, nodeIdToScrollTo)} + {this.buildTreeItems(treeStructure.nodes, visibleNodeIds, containerRef, nodeIdToScrollTo)} ); @@ -92,7 +93,6 @@ export class SidebarIndexTableOfContents extends Component { } SidebarIndexTableOfContents.propTypes = { - canvases: PropTypes.arrayOf(PropTypes.object).isRequired, classes: PropTypes.objectOf(PropTypes.string).isRequired, containerRef: PropTypes.oneOfType([ PropTypes.func, diff --git a/src/containers/SidebarIndexTableOfContents.js b/src/containers/SidebarIndexTableOfContents.js index 1104700310..6771c5da64 100644 --- a/src/containers/SidebarIndexTableOfContents.js +++ b/src/containers/SidebarIndexTableOfContents.js @@ -5,9 +5,7 @@ import { withStyles } from '@material-ui/core/styles'; import { withPlugins } from '../extend/withPlugins'; import { SidebarIndexTableOfContents } from '../components/SidebarIndexTableOfContents'; import { - getManifestoInstance, getManifestTreeStructure, - getVisibleCanvases, getVisibleNodeIds, getExpandedNodeIds, getNodeIdToScrollTo, @@ -19,9 +17,7 @@ import * as actions from '../state/actions'; * mapStateToProps - to hook up connect */ const mapStateToProps = (state, { id, windowId }) => ({ - canvases: getVisibleCanvases(state, { windowId }), expandedNodeIds: getExpandedNodeIds(state, { companionWindowId: id, windowId }), - manifesto: getManifestoInstance(state, { windowId }), nodeIdToScrollTo: getNodeIdToScrollTo(state, { companionWindowId: id, windowId }), treeStructure: getManifestTreeStructure(state, { windowId }), visibleNodeIds: getVisibleNodeIds(state, { companionWindowId: id, windowId }), From 074668ef0443f609c25f7c1bd8f3ca7df6fd2806 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Fri, 28 Feb 2020 16:58:24 +0100 Subject: [PATCH 14/21] Add tests for SidebarIndexTableOfContents --- __tests__/fixtures/version-2/structures.json | 159 ++++++++++++++ .../SidebarIndexTableOfContents.test.js | 194 ++++++++++++++++++ src/components/SidebarIndexTableOfContents.js | 3 +- 3 files changed, 355 insertions(+), 1 deletion(-) create mode 100644 __tests__/fixtures/version-2/structures.json create mode 100644 __tests__/src/components/SidebarIndexTableOfContents.test.js diff --git a/__tests__/fixtures/version-2/structures.json b/__tests__/fixtures/version-2/structures.json new file mode 100644 index 0000000000..90c4592625 --- /dev/null +++ b/__tests__/fixtures/version-2/structures.json @@ -0,0 +1,159 @@ +{ + "@context": "http://iiif.io/api/presentation/2/context.json", + "@type": "sc:Manifest", + "@id": "http://foo.test/1/manifest", + "label": "Manifest to be used for SidebarIndexTableOfContents.test.js", + "sequences" : [ + { + "@type": "sc:Sequence", + "canvases": [ + { + "@id": "http://foo.test/1/canvas/c1", + "@type": "sc:Canvas", + "label": "Canvas: 1" + }, + { + "@id": "http://foo.test/1/canvas/c2", + "@type": "sc:Canvas", + "label": "Canvas: 2" + }, + { + "@id": "http://foo.test/1/canvas/c3", + "@type": "sc:Canvas", + "label": "Canvas: 3" + }, + { + "@id": "http://foo.test/1/canvas/c4", + "@type": "sc:Canvas", + "label": "Canvas: 4" + }, + { + "@id": "http://foo.test/1/canvas/c5", + "@type": "sc:Canvas", + "label": "Canvas: 5" + }, + { + "@id": "http://foo.test/1/canvas/c6", + "@type": "sc:Canvas", + "label": "Canvas: 6" + }, + { + "@id": "http://foo.test/1/canvas/c7", + "@type": "sc:Canvas", + "label": "Canvas: 7" + }, + { + "@id": "http://foo.test/1/canvas/c8", + "@type": "sc:Canvas", + "label": "Canvas: 8" + }, + { + "@id": "http://foo.test/1/canvas/c9", + "@type": "sc:Canvas", + "label": "Canvas: 9" + } + ] + } + ], + "structures": [ + { + "@id": "http://foo.test/1/range/root", + "@type": "sc:Range", + "viewingHint": "top", + "ranges": [ + "http://foo.test/1/range/1", + "http://foo.test/1/range/2" + ], + "canvases": [] + }, + { + "@id": "http://foo.test/1/range/1", + "@type": "sc:Range", + "ranges": [ + "http://foo.test/1/range/1-1", + "http://foo.test/1/range/1-2", + "http://foo.test/1/range/1-3" + ], + "canvases": [ + "http://foo.test/1/canvas/c1", + "http://foo.test/1/canvas/c2", + "http://foo.test/1/canvas/c3", + "http://foo.test/1/canvas/c4" + ] + }, + { + "@id": "http://foo.test/1/range/1-1", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c1", + "http://foo.test/1/canvas/c2" + ] + }, + { + "@id": "http://foo.test/1/range/1-2", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c2", + "http://foo.test/1/canvas/c3" + ] + }, + { + "@id": "http://foo.test/1/range/1-3", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c4" + ] + }, + { + "@id": "http://foo.test/1/range/2", + "@type": "sc:Range", + "ranges": [ + "http://foo.test/1/range/2-1", + "http://foo.test/1/range/2-2", + "http://foo.test/1/range/2-3" + ], + "canvases": [] + }, + { + "@id": "http://foo.test/1/range/2-1", + "@type": "sc:Range", + "ranges": [], + "canvases": [] + }, + { + "@id": "http://foo.test/1/range/2-2", + "@type": "sc:Range", + "ranges": [ + "http://foo.test/1/range/2-2-1", + "http://foo.test/1/range/2-2-2" + ] + }, + { + "@id": "http://foo.test/1/range/2-2-1", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c5", + "http://foo.test/1/canvas/c6" + ] + }, + { + "@id": "http://foo.test/1/range/2-2-2", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c7", + "http://foo.test/1/canvas/c8" + ] + }, + { + "@id": "http://foo.test/1/range/2-3", + "@type": "sc:Range", + "ranges": [], + "canvases": [] + } + ] +} \ No newline at end of file diff --git a/__tests__/src/components/SidebarIndexTableOfContents.test.js b/__tests__/src/components/SidebarIndexTableOfContents.test.js new file mode 100644 index 0000000000..40b8dadc73 --- /dev/null +++ b/__tests__/src/components/SidebarIndexTableOfContents.test.js @@ -0,0 +1,194 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import manifesto from 'manifesto.js'; +import TreeItem from '@material-ui/lab/TreeItem'; +import TreeView from '@material-ui/lab/TreeView'; +import { SidebarIndexTableOfContents } from '../../../src/components/SidebarIndexTableOfContents'; +import manifestJson from '../../fixtures/version-2/structures.json'; + +/** + * Create shallow enzyme wrapper for SidebarIndexTableOfContents component + * @param {*} props + */ +function createWrapper(props) { + const manifest = manifesto.create(manifestJson); + return shallow( + , + ); +} + +/** + * Create necessary props to simulate keydown event with specific key + */ +function createKeydownProps(key) { + return [ + 'keydown', + { + key, + }, + ]; +} + +describe('SidebarIndexTableOfContents', () => { + let toggleNode; + let setCanvas; + + beforeEach(() => { + toggleNode = jest.fn(); + setCanvas = jest.fn(); + }); + + + it('renders a tree item for every node', () => { + const structuresWrapper = createWrapper({}); + expect(structuresWrapper.find(TreeItem)).toHaveLength(10); + const simpleTreeWrapper = createWrapper({ + treeStructure: { + nodes: [ + { + id: '0', + nodes: [ + { + id: '0-0', + nodes: [], + }, + { + id: '0-1', + nodes: [], + }, + ], + }, + ], + }, + }); + expect(simpleTreeWrapper.find(TreeItem)).toHaveLength(3); + }); + + it('accepts missing nodes property for tress structure and tree nodes', () => { + const noNodesWrapper = createWrapper({ + treeStructure: { nodes: undefined }, + }); + expect(noNodesWrapper.find(TreeItem)).toHaveLength(0); + const noChildNodesWrapper = createWrapper({ + treeStructure: { + nodes: [{ id: '0' }], + }, + }); + expect(noChildNodesWrapper.find(TreeItem)).toHaveLength(1); + }); + + it('toggles branch nodes on click, but not leaf nodes', () => { + const wrapper = createWrapper({ setCanvas, toggleNode }); + const treeView = wrapper.children(TreeView).at(0); + + const node0 = treeView.childAt(0); + expect(node0.prop('nodeId')).toBe('0-0'); + node0.simulate('click'); + node0.simulate('click'); + expect(toggleNode).toHaveBeenCalledTimes(2); + + const node00 = node0.children().at(0); + expect(node00.prop('nodeId')).toBe('0-0-0'); + node00.simulate('click'); + node00.simulate('click'); + expect(toggleNode).toHaveBeenCalledTimes(2); + + const node1 = treeView.childAt(1); + expect(node1.prop('nodeId')).toBe('0-1'); + node1.simulate('click'); + expect(toggleNode).toHaveBeenCalledTimes(3); + }); + + it('collapses branch nodes (i.e. toggles open branch nodes) with left arrow key', () => { + const wrapper = createWrapper({ + expandedNodeIds: ['0-0'], + setCanvas, + toggleNode, + }); + const treeView = wrapper.children(TreeView).at(0); + + const node0 = treeView.childAt(0); + expect(node0.prop('nodeId')).toBe('0-0'); + node0.simulate(...createKeydownProps('ArrowLeft')); + expect(toggleNode).toHaveBeenCalledTimes(1); + + const node00 = node0.children().at(0); + expect(node00.prop('nodeId')).toBe('0-0-0'); + const node1 = treeView.childAt(1); + expect(node1.prop('nodeId')).toBe('0-1'); + + node00.simulate(...createKeydownProps('ArrowLeft')); + node1.simulate(...createKeydownProps('ArrowLeft')); + expect(toggleNode).toHaveBeenCalledTimes(1); + }); + + it('expands branch nodes (i.e. toggles closed branch nodes) with right arrow key', () => { + const wrapper = createWrapper({ + expandedNodeIds: ['0-0'], + setCanvas, + toggleNode, + }); + const treeView = wrapper.children(TreeView).at(0); + const node0 = treeView.childAt(0); + expect(node0.prop('nodeId')).toBe('0-0'); + const node00 = node0.children().at(0); + expect(node00.prop('nodeId')).toBe('0-0-0'); + + node0.simulate(...createKeydownProps('ArrowRight')); + node00.simulate(...createKeydownProps('ArrowRight')); + expect(toggleNode).toHaveBeenCalledTimes(0); + + const node1 = treeView.childAt(1); + expect(node1.prop('nodeId')).toBe('0-1'); + node1.simulate(...createKeydownProps('ArrowRight')); + expect(toggleNode).toHaveBeenCalledTimes(1); + }); + + it('toggles branch nodes (but not leaf nodes) with Space or Enter key', () => { + const wrapper = createWrapper({ setCanvas, toggleNode }); + const treeView = wrapper.children(TreeView).at(0); + const node0 = treeView.childAt(0); + node0.simulate(...createKeydownProps('Enter')); + expect(toggleNode).toHaveBeenCalledTimes(1); + node0.simulate(...createKeydownProps(' ')); + expect(toggleNode).toHaveBeenCalledTimes(2); + node0.simulate(...createKeydownProps('Spacebar')); + expect(toggleNode).toHaveBeenCalledTimes(3); + node0.simulate(...createKeydownProps('Tab')); + node0.children().at(0).simulate(...createKeydownProps('Enter')); + node0.children().at(0).simulate(...createKeydownProps(' ')); + expect(toggleNode).toHaveBeenCalledTimes(3); + treeView.childAt(1).simulate(...createKeydownProps('Enter')); + treeView.childAt(1).simulate(...createKeydownProps(' ')); + expect(toggleNode).toHaveBeenCalledTimes(5); + }); + + it('calls setCanvas only on click for ranges with canvases', () => { + const wrapper = createWrapper({ setCanvas, toggleNode }); + const treeView = wrapper.children(TreeView).at(0); + const node0 = treeView.childAt(0); + expect(node0.prop('nodeId')).toBe('0-0'); + node0.simulate('click'); + expect(setCanvas).toHaveBeenCalledTimes(1); + node0.childAt(0).simulate('click'); + expect(setCanvas).toHaveBeenCalledTimes(2); + node0.childAt(1).simulate('click'); + expect(setCanvas).toHaveBeenCalledTimes(3); + node0.childAt(2).simulate('click'); + expect(setCanvas).toHaveBeenCalledTimes(4); + + const node1 = treeView.childAt(1); + expect(node1.prop('nodeId')).toBe('0-1'); + node1.simulate(...createKeydownProps('ArrowRight')); + expect(setCanvas).toHaveBeenCalledTimes(4); + }); +}); diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index 65b70d176f..47e6f12653 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -28,6 +28,7 @@ export class SidebarIndexTableOfContents extends Component { handleKeyPressed(event, node) { if (event.key === 'Enter' || event.key === ' ' + || event.key === 'Spacebar' || event.key === 'ArrowLeft' && this.props.expandedNodeIds.indexOf(node.id) !== -1 || event.key === 'ArrowRight' && this.props.expandedNodeIds.indexOf(node.id) === -1) { this.selectTreeItem(node); @@ -60,7 +61,7 @@ export class SidebarIndexTableOfContents extends Component { onClick={() => this.selectTreeItem(node)} onKeyDown={e => this.handleKeyPressed(e, node)} > - {node.nodes.length > 0 ? this.buildTreeItems(node.nodes, visibleNodeIds, containerRef, nodeIdToScrollTo) : null} + {node.nodes && node.nodes.length > 0 ? this.buildTreeItems(node.nodes, visibleNodeIds, containerRef, nodeIdToScrollTo) : null} )) ); From 4bffcced46fe1d887133b5dfc56cc409de5e40de Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Mon, 2 Mar 2020 10:40:56 +0100 Subject: [PATCH 15/21] Fix code style --- src/components/SidebarIndexTableOfContents.js | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index 47e6f12653..11ee5a4b3a 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -26,11 +26,12 @@ export class SidebarIndexTableOfContents extends Component { /** */ handleKeyPressed(event, node) { + const { expandedNodeIds } = this.props; if (event.key === 'Enter' || event.key === ' ' || event.key === 'Spacebar' - || event.key === 'ArrowLeft' && this.props.expandedNodeIds.indexOf(node.id) !== -1 - || event.key === 'ArrowRight' && this.props.expandedNodeIds.indexOf(node.id) === -1) { + || (event.key === 'ArrowLeft' && expandedNodeIds.indexOf(node.id) !== -1) + || (event.key === 'ArrowRight' && expandedNodeIds.indexOf(node.id) === -1)) { this.selectTreeItem(node); } } @@ -46,22 +47,27 @@ export class SidebarIndexTableOfContents extends Component { key={node.id} nodeId={node.id} label={( - - <> - {visibleNodeIds.indexOf(node.id) !== -1 && } - {node.label} - - + + <> + {visibleNodeIds.indexOf(node.id) !== -1 && } + {node.label} + + )} onClick={() => this.selectTreeItem(node)} onKeyDown={e => this.handleKeyPressed(e, node)} > - {node.nodes && node.nodes.length > 0 ? this.buildTreeItems(node.nodes, visibleNodeIds, containerRef, nodeIdToScrollTo) : null} + {node.nodes && node.nodes.length > 0 ? this.buildTreeItems( + node.nodes, + visibleNodeIds, + containerRef, + nodeIdToScrollTo, + ) : null} )) ); @@ -95,13 +101,13 @@ export class SidebarIndexTableOfContents extends Component { SidebarIndexTableOfContents.propTypes = { classes: PropTypes.objectOf(PropTypes.string).isRequired, - containerRef: PropTypes.oneOfType([ + containerRef: PropTypes.oneOfType([ PropTypes.func, PropTypes.shape({ current: PropTypes.instanceOf(Element) }), - ]), + ]).isRequired, expandedNodeIds: PropTypes.arrayOf(PropTypes.string).isRequired, - setCanvas: PropTypes.func.isRequired, nodeIdToScrollTo: PropTypes.func.isRequired, + setCanvas: PropTypes.func.isRequired, toggleNode: PropTypes.func.isRequired, treeStructure: PropTypes.objectOf().isRequired, visibleNodeIds: PropTypes.arrayOf(PropTypes.string).isRequired, From 0351f31b4d2f6b7213b657c635430f70ba3f2ab1 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Mon, 2 Mar 2020 12:57:01 +0100 Subject: [PATCH 16/21] Run test with explicit WindowSideBarCanvasPanel variant Default variant has changed to 'tableOfContents', test for index list will fail with default. --- __tests__/src/components/WindowSideBarCanvasPanel.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/__tests__/src/components/WindowSideBarCanvasPanel.test.js b/__tests__/src/components/WindowSideBarCanvasPanel.test.js index 5f6cf795da..92a12bc2f5 100644 --- a/__tests__/src/components/WindowSideBarCanvasPanel.test.js +++ b/__tests__/src/components/WindowSideBarCanvasPanel.test.js @@ -25,6 +25,7 @@ function createWrapper(props) { config={{ canvasNavigation: { height: 100 } }} updateVariant={() => {}} selectedCanvases={[canvases[1]]} + variant="compact" {...props} />, ); From 63f463903aade5f777ebc8d9b28eb5926076a431 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Mon, 2 Mar 2020 15:14:56 +0100 Subject: [PATCH 17/21] Add more SidebarIndexTableOfContents component tests, fix Component --- .../SidebarIndexTableOfContents.test.js | 26 +++++++++++++++++++ src/components/SidebarIndexTableOfContents.js | 10 ++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/__tests__/src/components/SidebarIndexTableOfContents.test.js b/__tests__/src/components/SidebarIndexTableOfContents.test.js index 40b8dadc73..0d1afd4828 100644 --- a/__tests__/src/components/SidebarIndexTableOfContents.test.js +++ b/__tests__/src/components/SidebarIndexTableOfContents.test.js @@ -47,6 +47,12 @@ describe('SidebarIndexTableOfContents', () => { setCanvas = jest.fn(); }); + it('does not render a TreeView if the tree structure is missing', () => { + const wrapper = createWrapper({ + treeStructure: undefined, + }); + expect(wrapper.children().length).toBe(0); + }); it('renders a tree item for every node', () => { const structuresWrapper = createWrapper({}); @@ -191,4 +197,24 @@ describe('SidebarIndexTableOfContents', () => { node1.simulate(...createKeydownProps('ArrowRight')); expect(setCanvas).toHaveBeenCalledTimes(4); }); + + it('does not select a canvas when opening a node with the right arrow key', () => { + const wrapper = createWrapper({ setCanvas, toggleNode }); + const treeView = wrapper.children(TreeView).at(0); + const node0 = treeView.childAt(0); + expect(node0.prop('nodeId')).toBe('0-0'); + node0.simulate(...createKeydownProps('ArrowRight')); + expect(setCanvas).toHaveBeenCalledTimes(0); + expect(toggleNode).toHaveBeenCalledTimes(1); + }); + + it('does not select a canvas when closing a node with the left arrow key', () => { + const wrapper = createWrapper({ expandedNodeIds: ['0-0'], setCanvas, toggleNode }); + const treeView = wrapper.children(TreeView).at(0); + const node0 = treeView.childAt(0); + expect(node0.prop('nodeId')).toBe('0-0'); + node0.simulate(...createKeydownProps('ArrowLeft')); + expect(setCanvas).toHaveBeenCalledTimes(0); + expect(toggleNode).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/components/SidebarIndexTableOfContents.js b/src/components/SidebarIndexTableOfContents.js index 11ee5a4b3a..4e9e4691e6 100644 --- a/src/components/SidebarIndexTableOfContents.js +++ b/src/components/SidebarIndexTableOfContents.js @@ -26,14 +26,16 @@ export class SidebarIndexTableOfContents extends Component { /** */ handleKeyPressed(event, node) { - const { expandedNodeIds } = this.props; + const { expandedNodeIds, toggleNode } = this.props; if (event.key === 'Enter' || event.key === ' ' - || event.key === 'Spacebar' - || (event.key === 'ArrowLeft' && expandedNodeIds.indexOf(node.id) !== -1) - || (event.key === 'ArrowRight' && expandedNodeIds.indexOf(node.id) === -1)) { + || event.key === 'Spacebar') { this.selectTreeItem(node); } + if ((event.key === 'ArrowLeft' && expandedNodeIds.indexOf(node.id) !== -1) + || (event.key === 'ArrowRight' && expandedNodeIds.indexOf(node.id) === -1 && node.nodes.length > 0)) { + toggleNode(node.id); + } } /** */ From 607180132e7bf121b8970ead176334c9bfc6dd43 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Mon, 2 Mar 2020 18:00:21 +0100 Subject: [PATCH 18/21] Add TOC related tests for companionWindow reducer --- .../src/reducers/companionWindows.test.js | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/__tests__/src/reducers/companionWindows.test.js b/__tests__/src/reducers/companionWindows.test.js index 1c7f2d8d61..9e29cd940e 100644 --- a/__tests__/src/reducers/companionWindows.test.js +++ b/__tests__/src/reducers/companionWindows.test.js @@ -109,6 +109,79 @@ describe('companionWindowsReducer', () => { expect(companionWindowsReducer(beforeState, action)).toEqual(expectedState); }); }); + + describe('TOGGLE_TOC_NODE', () => { + const action = { + id: 'cw123', + nodeId: '0-1', + type: ActionTypes.TOGGLE_TOC_NODE, + }; + + it('should add the id of a toggled node that do not exist in the state', () => { + const emptyBeforeState = { + cw123: {}, + cw456: {}, + }; + const expectedStateFromEmpty = { + cw123: { + tocNodes: { + '0-1': { expanded: true }, + }, + }, + cw456: {}, + }; + expect(companionWindowsReducer(emptyBeforeState, action)).toEqual(expectedStateFromEmpty); + + const filledBeforeState = { + cw123: { + tocNodes: { + '0-0': { expanded: true }, + '0-2-0': { expanded: true }, + }, + }, + cw456: {}, + }; + const expectedStateAfterFilled = { + cw123: { + tocNodes: { + '0-0': { expanded: true }, + '0-1': { expanded: true }, + '0-2-0': { expanded: true }, + }, + }, + cw456: {}, + }; + expect(companionWindowsReducer(filledBeforeState, action)).toEqual(expectedStateAfterFilled); + }); + + it('should switch the expanded value for existing nodeIds in the state', () => { + const stateWithTrue = { + cw123: { + tocNodes: { + '0-0': { expanded: true }, + '0-1': { expanded: true }, + '0-2-0': { expanded: true }, + }, + }, + cw456: {}, + }; + + const stateWithFalse = { + cw123: { + tocNodes: { + '0-0': { expanded: true }, + '0-1': { expanded: false }, + '0-2-0': { expanded: true }, + }, + }, + cw456: {}, + }; + + expect(companionWindowsReducer(stateWithTrue, action)).toEqual(stateWithFalse); + expect(companionWindowsReducer(stateWithFalse, action)).toEqual(stateWithTrue); + }); + }); + it('should handle IMPORT_MIRADOR_STATE', () => { expect(companionWindowsReducer({}, { state: { companionWindows: { new: 'stuff' } }, From 50d93fc73064e90b7500151b2961cc0af20f3838 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Tue, 3 Mar 2020 15:09:59 +0100 Subject: [PATCH 19/21] Refactor toggle toc node action --- .../src/reducers/companionWindows.test.js | 31 ++++++++++++++----- src/state/actions/companionWindow.js | 13 ++++++-- src/state/reducers/companionWindows.js | 8 +---- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/__tests__/src/reducers/companionWindows.test.js b/__tests__/src/reducers/companionWindows.test.js index 9e29cd940e..0c7393d15c 100644 --- a/__tests__/src/reducers/companionWindows.test.js +++ b/__tests__/src/reducers/companionWindows.test.js @@ -111,9 +111,22 @@ describe('companionWindowsReducer', () => { }); describe('TOGGLE_TOC_NODE', () => { - const action = { + const actionOpen = { id: 'cw123', - nodeId: '0-1', + payload: { + '0-1': { + expanded: true, + }, + }, + type: ActionTypes.TOGGLE_TOC_NODE, + }; + const actionClose = { + id: 'cw123', + payload: { + '0-1': { + expanded: false, + }, + }, type: ActionTypes.TOGGLE_TOC_NODE, }; @@ -130,9 +143,9 @@ describe('companionWindowsReducer', () => { }, cw456: {}, }; - expect(companionWindowsReducer(emptyBeforeState, action)).toEqual(expectedStateFromEmpty); + expect(companionWindowsReducer(emptyBeforeState, actionOpen)).toEqual(expectedStateFromEmpty); - const filledBeforeState = { + const beforeState = { cw123: { tocNodes: { '0-0': { expanded: true }, @@ -151,10 +164,10 @@ describe('companionWindowsReducer', () => { }, cw456: {}, }; - expect(companionWindowsReducer(filledBeforeState, action)).toEqual(expectedStateAfterFilled); + expect(companionWindowsReducer(beforeState, actionOpen)).toEqual(expectedStateAfterFilled); }); - it('should switch the expanded value for existing nodeIds in the state', () => { + it('should update expanded value for existing nodeIds in the state', () => { const stateWithTrue = { cw123: { tocNodes: { @@ -177,8 +190,10 @@ describe('companionWindowsReducer', () => { cw456: {}, }; - expect(companionWindowsReducer(stateWithTrue, action)).toEqual(stateWithFalse); - expect(companionWindowsReducer(stateWithFalse, action)).toEqual(stateWithTrue); + expect(companionWindowsReducer(stateWithTrue, actionOpen)).toEqual(stateWithTrue); + expect(companionWindowsReducer(stateWithFalse, actionOpen)).toEqual(stateWithTrue); + expect(companionWindowsReducer(stateWithTrue, actionClose)).toEqual(stateWithFalse); + expect(companionWindowsReducer(stateWithFalse, actionClose)).toEqual(stateWithFalse); }); }); diff --git a/src/state/actions/companionWindow.js b/src/state/actions/companionWindow.js index 0272283bf6..bd8ac0e057 100644 --- a/src/state/actions/companionWindow.js +++ b/src/state/actions/companionWindow.js @@ -1,6 +1,6 @@ import uuid from 'uuid/v4'; import ActionTypes from './action-types'; -import { getCompanionWindowIdsForPosition, getVisibleNodeIds } from '../selectors'; +import { getCompanionWindowIdsForPosition, getManuallyExpandedNodeIds, getVisibleNodeIds } from '../selectors'; const defaultProps = { content: null, @@ -62,12 +62,19 @@ export function removeCompanionWindow(windowId, id) { export function toggleNode(windowId, id, nodeId) { return (dispatch, getState) => { const state = getState(); + const collapsedNodeIds = getManuallyExpandedNodeIds(state, { companionWindowId: id }, false); + const expandedNodeIds = getManuallyExpandedNodeIds(state, { companionWindowId: id }, true); const visibleNodeIds = getVisibleNodeIds(state, { id, windowId }); + const expand = collapsedNodeIds.indexOf(nodeId) !== -1 + || (expandedNodeIds.indexOf(nodeId) === -1 && visibleNodeIds.indexOf(nodeId) === -1); return dispatch({ id, - nodeId, + payload: { + [nodeId]: { + expanded: expand, + }, + }, type: ActionTypes.TOGGLE_TOC_NODE, - visibleNodeIds, windowId, }); }; diff --git a/src/state/reducers/companionWindows.js b/src/state/reducers/companionWindows.js index 5460d912c2..b4f7db43ef 100644 --- a/src/state/reducers/companionWindows.js +++ b/src/state/reducers/companionWindows.js @@ -26,13 +26,7 @@ export function companionWindowsReducer(state = {}, action) { case ActionTypes.IMPORT_MIRADOR_STATE: return action.state.companionWindows; case ActionTypes.TOGGLE_TOC_NODE: - return updateIn(state, [[action.id], 'tocNodes'], {}, orig => merge(orig, { - [action.nodeId]: state[action.id].tocNodes && state[action.id].tocNodes[action.nodeId] ? { - expanded: !state[action.id].tocNodes[action.nodeId].expanded, - } : { - expanded: !(action.visibleNodeIds && action.visibleNodeIds.indexOf(action.nodeId) !== -1), - }, - })); + return updateIn(state, [[action.id], 'tocNodes'], {}, orig => merge(orig, action.payload)); default: return state; } From 1b5c3965c0e71d933e66e9d722525774b288ae65 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Wed, 4 Mar 2020 18:01:18 +0100 Subject: [PATCH 20/21] Add tests for toc nodes selectors --- __tests__/fixtures/version-2/structures.json | 135 +++++++++++++++--- .../SidebarIndexTableOfContents.test.js | 2 +- __tests__/src/selectors/ranges.test.js | 132 +++++++++++++++++ src/state/selectors/ranges.js | 26 +++- 4 files changed, 270 insertions(+), 25 deletions(-) create mode 100644 __tests__/src/selectors/ranges.test.js diff --git a/__tests__/fixtures/version-2/structures.json b/__tests__/fixtures/version-2/structures.json index 90c4592625..bcf680dc57 100644 --- a/__tests__/fixtures/version-2/structures.json +++ b/__tests__/fixtures/version-2/structures.json @@ -51,6 +51,21 @@ "@id": "http://foo.test/1/canvas/c9", "@type": "sc:Canvas", "label": "Canvas: 9" + }, + { + "@id": "http://foo.test/1/canvas/c10", + "@type": "sc:Canvas", + "label": "Canvas: 9" + }, + { + "@id": "http://foo.test/1/canvas/c11", + "@type": "sc:Canvas", + "label": "Canvas: 9" + }, + { + "@id": "http://foo.test/1/canvas/c12", + "@type": "sc:Canvas", + "label": "Canvas: 9" } ] } @@ -61,18 +76,19 @@ "@type": "sc:Range", "viewingHint": "top", "ranges": [ - "http://foo.test/1/range/1", - "http://foo.test/1/range/2" + "http://foo.test/1/range/0-0", + "http://foo.test/1/range/0-1", + "http://foo.test/1/range/0-2" ], "canvases": [] }, { - "@id": "http://foo.test/1/range/1", + "@id": "http://foo.test/1/range/0-0", "@type": "sc:Range", "ranges": [ - "http://foo.test/1/range/1-1", - "http://foo.test/1/range/1-2", - "http://foo.test/1/range/1-3" + "http://foo.test/1/range/0-0-0", + "http://foo.test/1/range/0-0-1", + "http://foo.test/1/range/0-0-2" ], "canvases": [ "http://foo.test/1/canvas/c1", @@ -82,7 +98,7 @@ ] }, { - "@id": "http://foo.test/1/range/1-1", + "@id": "http://foo.test/1/range/0-0-0", "@type": "sc:Range", "ranges": [], "canvases": [ @@ -91,7 +107,7 @@ ] }, { - "@id": "http://foo.test/1/range/1-2", + "@id": "http://foo.test/1/range/0-0-1", "@type": "sc:Range", "ranges": [], "canvases": [ @@ -100,7 +116,7 @@ ] }, { - "@id": "http://foo.test/1/range/1-3", + "@id": "http://foo.test/1/range/0-0-2", "@type": "sc:Range", "ranges": [], "canvases": [ @@ -108,31 +124,34 @@ ] }, { - "@id": "http://foo.test/1/range/2", + "@id": "http://foo.test/1/range/0-1", "@type": "sc:Range", "ranges": [ - "http://foo.test/1/range/2-1", - "http://foo.test/1/range/2-2", - "http://foo.test/1/range/2-3" + "http://foo.test/1/range/0-1-0", + "http://foo.test/1/range/0-1-1", + "http://foo.test/1/range/0-1-2" ], "canvases": [] }, { - "@id": "http://foo.test/1/range/2-1", + "@id": "http://foo.test/1/range/0-1-0", "@type": "sc:Range", "ranges": [], "canvases": [] }, { - "@id": "http://foo.test/1/range/2-2", + "@id": "http://foo.test/1/range/0-1-1", "@type": "sc:Range", "ranges": [ - "http://foo.test/1/range/2-2-1", - "http://foo.test/1/range/2-2-2" + "http://foo.test/1/range/0-1-1-0", + "http://foo.test/1/range/0-1-1-1" + ], + "canvases": [ + "http://foo.test/1/canvas/c6" ] }, { - "@id": "http://foo.test/1/range/2-2-1", + "@id": "http://foo.test/1/range/0-1-1-0", "@type": "sc:Range", "ranges": [], "canvases": [ @@ -141,19 +160,95 @@ ] }, { - "@id": "http://foo.test/1/range/2-2-2", + "@id": "http://foo.test/1/range/0-1-1-1", "@type": "sc:Range", "ranges": [], "canvases": [ + "http://foo.test/1/canvas/c6", "http://foo.test/1/canvas/c7", "http://foo.test/1/canvas/c8" ] }, { - "@id": "http://foo.test/1/range/2-3", + "@id": "http://foo.test/1/range/0-1-2", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c8" + ] + }, + { + "@id": "http://foo.test/1/range/0-1-2-0", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c8", + "http://foo.test/1/canvas/c9" + ] + }, + { + "@id": "http://foo.test/1/range/0-2", + "@type": "sc:Range", + "ranges": [ + "http://foo.test/1/range/0-2-0", + "http://foo.test/1/range/0-2-1", + "http://foo.test/1/range/0-2-2" + ], + "canvases": [] + }, + { + "@id": "http://foo.test/1/range/0-2-0", "@type": "sc:Range", "ranges": [], "canvases": [] + }, + { + "@id": "http://foo.test/1/range/0-2-1", + "@type": "sc:Range", + "ranges": [ + "http://foo.test/1/range/0-2-1-0", + "http://foo.test/1/range/0-2-1-1" + ], + "canvases": [ + "http://foo.test/1/canvas/c10" + ] + }, + { + "@id": "http://foo.test/1/range/0-2-1-0", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c9" + ] + }, + { + "@id": "http://foo.test/1/range/0-2-1-1", + "@type": "sc:Range", + "ranges": [], + "canvases": [] + }, + { + "@id": "http://foo.test/1/range/0-2-2", + "@type": "sc:Range", + "ranges": [ + "http://foo.test/1/range/0-2-2-0", + "http://foo.test/1/range/0-2-2-1" + ], + "canvases": [] + }, + { + "@id": "http://foo.test/1/range/0-2-2-0", + "@type": "sc:Range", + "ranges": [], + "canvases": [] + }, + { + "@id": "http://foo.test/1/range/0-2-2-1", + "@type": "sc:Range", + "ranges": [], + "canvases": [ + "http://foo.test/1/canvas/c9" + ] } ] } \ No newline at end of file diff --git a/__tests__/src/components/SidebarIndexTableOfContents.test.js b/__tests__/src/components/SidebarIndexTableOfContents.test.js index 0d1afd4828..0f2b565897 100644 --- a/__tests__/src/components/SidebarIndexTableOfContents.test.js +++ b/__tests__/src/components/SidebarIndexTableOfContents.test.js @@ -56,7 +56,7 @@ describe('SidebarIndexTableOfContents', () => { it('renders a tree item for every node', () => { const structuresWrapper = createWrapper({}); - expect(structuresWrapper.find(TreeItem)).toHaveLength(10); + expect(structuresWrapper.find(TreeItem)).toHaveLength(18); const simpleTreeWrapper = createWrapper({ treeStructure: { nodes: [ diff --git a/__tests__/src/selectors/ranges.test.js b/__tests__/src/selectors/ranges.test.js new file mode 100644 index 0000000000..cfe552b2f3 --- /dev/null +++ b/__tests__/src/selectors/ranges.test.js @@ -0,0 +1,132 @@ +import { setIn } from 'immutable'; +import manifestJson from '../../fixtures/version-2/structures.json'; +import { + getVisibleNodeIds, + getManuallyExpandedNodeIds, + getExpandedNodeIds, + getNodeIdToScrollTo, +} from '../../../src/state/selectors'; + +const state = { + companionWindows: { + cw123: {}, + cw456: {}, + }, + manifests: { + mID: { + id: 'mID', + json: manifestJson, + }, + }, + windows: { + w1: { + canvasId: 'http://foo.test/1/canvas/c6', + companionWindowIds: ['cw123', 'cw456'], + id: 'w1', + manifestId: 'mID', + view: 'book', + }, + }, +}; + +const expandedNodesState = setIn(state, ['companionWindows', 'cw123', 'tocNodes'], { + '0-0': { + expanded: false, + }, + '0-1': { + expanded: true, + }, + '0-1-1': { + expanded: true, + }, + '0-1-2': { + expanded: false, + }, +}); + +describe('getVisibleNodeIds', () => { + it('contains node ids for all ranges which contain currently visible canvases, and for their parents', () => { + const visibleNodeIds = getVisibleNodeIds(state, { windowId: 'w1' }); + expect(visibleNodeIds).toEqual(expect.arrayContaining([ + '0-1', + '0-1-1', + '0-1-1-0', + '0-1-1-1', + ])); + expect(visibleNodeIds.length).toBe(4); + }); +}); + +describe('getManuallyExpandedNodeIds', () => { + it('returns empty array if there are no manually opened or closed nodes', () => { + expect(getManuallyExpandedNodeIds(state, { companionWindowId: 'cw123', windowId: 'w1' }, true)).toEqual([]); + expect(getManuallyExpandedNodeIds(state, { companionWindowId: 'cw123', windowId: 'w1' }, false)).toEqual([]); + }); + + it('returns manually opened and closed node ids correctly', () => { + expect(getManuallyExpandedNodeIds(expandedNodesState, { companionWindowId: 'cw123' }, true)).toEqual(['0-1', '0-1-1']); + expect(getManuallyExpandedNodeIds(expandedNodesState, { companionWindowId: 'cw123' }, false)).toEqual(['0-0', '0-1-2']); + }); +}); + +describe('getExpandedNodeIds', () => { + it('returns manually expanded node ids and visible, non collapsed branch node ids', () => { + const canvas8BookViewState = setIn(expandedNodesState, ['windows', 'w1', 'canvasId'], 'http://foo.test/1/canvas/c8'); + const canvas8BookViewVisibleNodeIds = getExpandedNodeIds(canvas8BookViewState, { companionWindowId: 'cw123', windowId: 'w1' }); + expect(canvas8BookViewVisibleNodeIds).toEqual(expect.arrayContaining([ + '0-1', + '0-1-1', + '0-2', + '0-2-1', + '0-2-2', + ])); + expect(canvas8BookViewVisibleNodeIds.length).toBe(5); + + const canvas8SingleViewState = setIn(canvas8BookViewState, ['windows', 'w1', 'view'], 'single'); + const canvas8SingleViewVisibleNodeIds = getExpandedNodeIds(canvas8SingleViewState, { companionWindowId: 'cw123', windowId: 'w1' }); + expect(canvas8SingleViewVisibleNodeIds).toEqual(expect.arrayContaining([ + '0-1', + '0-1-1', + ])); + expect(canvas8SingleViewVisibleNodeIds.length).toBe(2); + }); + + it('returns a combination of manually opened and current canvas containing node ids', () => { + const canvas9State = setIn(expandedNodesState, ['windows', 'w1', 'canvasId'], 'http://foo.test/1/canvas/c9'); + const expandedNodeIds = getExpandedNodeIds(canvas9State, { companionWindowId: 'cw123', windowId: 'w1' }); + expect(expandedNodeIds).toEqual(expect.arrayContaining([ + '0-1', + '0-1-1', + '0-2', + '0-2-1', + '0-2-2', + ])); + expect(expandedNodeIds.length).toBe(5); + }); +}); + +describe('getNodeIdToScrollTo', () => { + it('returns first leaf node with visible canvas', () => { + expect(getNodeIdToScrollTo(state, { companionWindowId: 'cw123', windowId: 'w1' })).toBe('0-1-1-0'); + }); + + it('returns branch node with visible canvas if it is the deepest in the tree to contain a canvas', () => { + const canvas10State = setIn(expandedNodesState, ['windows', 'w1', 'canvasId'], 'http://foo.test/1/canvas/c10'); + expect(getNodeIdToScrollTo(canvas10State, { companionWindowId: 'cw123', windowId: 'w1' })).toBe('0-2-1'); + }); + + it('returns the deepest non hidden branch node if leaf node or its parent node are collapsed', () => { + const closedParentState1 = setIn(state, ['companionWindows', 'cw123', 'tocNodes'], { '0-1-1': { expanded: false } }); + expect(getNodeIdToScrollTo(closedParentState1, { companionWindowId: 'cw123', windowId: 'w1' })).toBe('0-1-1'); + const closedParentState2 = setIn(state, ['companionWindows', 'cw123', 'tocNodes'], { + '0-1': { expanded: false }, + '0-1-1': { expanded: false }, + }); + expect(getNodeIdToScrollTo(closedParentState2, { companionWindowId: 'cw123', windowId: 'w1' })).toBe('0-1'); + }); + + it('returns no node id if current canvas is not contained in any range', () => { + const rangeFreeCanvasState = setIn(expandedNodesState, ['windows', 'w1', 'canvasId'], 'http://foo.test/1/canvas/c12'); + expect(getNodeIdToScrollTo(rangeFreeCanvasState, { companionWindowId: 'cw123', windowId: 'w1' })).toBe(null); + }); +}); diff --git a/src/state/selectors/ranges.js b/src/state/selectors/ranges.js index b639153f20..f3e26aa48a 100644 --- a/src/state/selectors/ranges.js +++ b/src/state/selectors/ranges.js @@ -17,6 +17,17 @@ function rangeContainsCanvasId(range, canvasId) { return false; } +/** */ +function getAllParentIds(node) { + if (node.parentNode === undefined) { + return []; + } + if (node.parentNode.parentNode === undefined) { + return [node.parentNode.id]; + } + return [...getAllParentIds(node.parentNode), node.parentNode.id]; +} + /** */ function getVisibleNodeIdsInSubTree(nodes, canvasIds) { return nodes.reduce((nodeIdAcc, node) => { @@ -29,14 +40,15 @@ function getVisibleNodeIdsInSubTree(nodes, canvasIds) { const subTreeVisibleNodeIds = node.nodes.length > 0 ? getVisibleNodeIdsInSubTree(node.nodes, canvasIds) : []; + result.push(...subTreeVisibleNodeIds); if (nodeContainsVisibleCanvas || subTreeVisibleNodeIds.length > 0) { result.push({ containsVisibleCanvas: nodeContainsVisibleCanvas, id: node.id, leaf: node.nodes.length === 0, + parentIds: getAllParentIds(node), }); } - result.push(...subTreeVisibleNodeIds); return result; }, []); } @@ -79,7 +91,7 @@ const getCanvasContainingNodeIds = createSelector( getVisibleLeafAndBranchNodeIds, ], visibleLeafAndBranchNodeIds => visibleLeafAndBranchNodeIds.reduce( - (acc, item) => (item.containsVisibleCanvas ? [...acc, item.id] : acc), + (acc, item) => (item.containsVisibleCanvas ? [...acc, item] : acc), [], ), ); @@ -106,8 +118,14 @@ export function getExpandedNodeIds(state, { companionWindowId, windowId }) { /** */ export function getNodeIdToScrollTo(state, { ...args }) { const canvasContainingNodeIds = getCanvasContainingNodeIds(state, { ...args }); - if (canvasContainingNodeIds) { - return canvasContainingNodeIds[0]; + const collapsedNodeIds = getManuallyExpandedNodeIds(state, args, false); + if (canvasContainingNodeIds && canvasContainingNodeIds.length > 0) { + for (let i = 0; i < canvasContainingNodeIds[0].parentIds.length; i += 1) { + if (collapsedNodeIds.indexOf(canvasContainingNodeIds[0].parentIds[i]) !== -1) { + return canvasContainingNodeIds[0].parentIds[i]; + } + } + return canvasContainingNodeIds[0].id; } return null; } From 8950dce2cd33b0ed41bf0ee305a94fce502c4c63 Mon Sep 17 00:00:00 2001 From: Lutz Helm Date: Thu, 5 Mar 2020 11:51:22 +0100 Subject: [PATCH 21/21] Add tests for companionWindow action 'toggleNode' --- __tests__/src/actions/companionWindow.test.js | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/__tests__/src/actions/companionWindow.test.js b/__tests__/src/actions/companionWindow.test.js index 23633844fa..916fa42030 100644 --- a/__tests__/src/actions/companionWindow.test.js +++ b/__tests__/src/actions/companionWindow.test.js @@ -1,6 +1,11 @@ import * as actions from '../../../src/state/actions'; import ActionTypes from '../../../src/state/actions/action-types'; +jest.mock('../../../src/state/selectors', () => ({ + getManuallyExpandedNodeIds: (state, args, expanded) => (expanded ? ['openVisible', 'open'] : ['closedVisible', 'closed']), + getVisibleNodeIds: (state, args) => ['openVisible', 'closedVisible', 'visible'], +})); + describe('companionWindow actions', () => { describe('addCompanionWindow', () => { it('should return correct action object', () => { @@ -96,4 +101,68 @@ describe('companionWindow actions', () => { expect(action.windowId).toBe('window'); }); }); + + describe('toggleNode', () => { + let mockDispatch; + let mockGetState; + + beforeEach(() => { + mockDispatch = jest.fn(() => ({})); + mockGetState = jest.fn(() => ({})); + }); + + it('returns a collapsing action for visible nodes that are not present in the state', () => { + const thunk = actions.toggleNode('window1', 'cw1', 'visible'); + thunk(mockDispatch, mockGetState); + + const action = mockDispatch.mock.calls[0][0]; + expect(action.id).toBe('cw1'); + expect(action.windowId).toBe('window1'); + expect(action.type).toBe(ActionTypes.TOGGLE_TOC_NODE); + expect(action.payload).toMatchObject({ visible: { expanded: false } }); + }); + + it('returns an expanding action for non visible nodes that are not present in the state', () => { + const thunk = actions.toggleNode('window1', 'cw1', 'foo'); + thunk(mockDispatch, mockGetState); + + const action = mockDispatch.mock.calls[0][0]; + expect(action.id).toBe('cw1'); + expect(action.windowId).toBe('window1'); + expect(action.type).toBe(ActionTypes.TOGGLE_TOC_NODE); + expect(action.payload).toMatchObject({ foo: { expanded: true } }); + }); + + it('returns a correct action any node that is present in the state', () => { + const openVisibleThunk = actions.toggleNode('window1', 'cw1', 'openVisible'); + openVisibleThunk(mockDispatch, mockGetState); + + const openThunk = actions.toggleNode('window1', 'cw1', 'open'); + openThunk(mockDispatch, mockGetState); + + const closedVisibleThunk = actions.toggleNode('window1', 'cw1', 'closedVisible'); + closedVisibleThunk(mockDispatch, mockGetState); + + const closedThunk = actions.toggleNode('window1', 'cw1', 'closed'); + closedThunk(mockDispatch, mockGetState); + + const actionForOpenVisible = mockDispatch.mock.calls[0][0]; + expect(actionForOpenVisible.id).toBe('cw1'); + expect(actionForOpenVisible.windowId).toBe('window1'); + expect(actionForOpenVisible.type).toBe(ActionTypes.TOGGLE_TOC_NODE); + expect(actionForOpenVisible.payload).toMatchObject({ openVisible: { expanded: false } }); + + const actionForOpen = mockDispatch.mock.calls[1][0]; + expect(actionForOpen.type).toBe(ActionTypes.TOGGLE_TOC_NODE); + expect(actionForOpen.payload).toMatchObject({ open: { expanded: false } }); + + const actionForClosedVisible = mockDispatch.mock.calls[2][0]; + expect(actionForClosedVisible.type).toBe(ActionTypes.TOGGLE_TOC_NODE); + expect(actionForClosedVisible.payload).toMatchObject({ closedVisible: { expanded: true } }); + + const actionForClosed = mockDispatch.mock.calls[3][0]; + expect(actionForClosed.type).toBe(ActionTypes.TOGGLE_TOC_NODE); + expect(actionForClosed.payload).toMatchObject({ closed: { expanded: true } }); + }); + }); });