From ef8fe27a64275f43147847e92207ed078b59d144 Mon Sep 17 00:00:00 2001 From: Zh Date: Tue, 14 Jul 2020 20:03:36 -0400 Subject: [PATCH 1/9] add timeout util and test --- package.json | 1 + src/util/timeoutQueue.js | 25 ++++++++++++++++ test/unit/util/timeoutQueue.test.js | 44 +++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 src/util/timeoutQueue.js create mode 100644 test/unit/util/timeoutQueue.test.js diff --git a/package.json b/package.json index 26f2407cb..1c315e739 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "gzip-and-upload": "env bash ./scripts/gzip-and-upload.sh", "build-docs": "echo 'see ./docs-src/README.md'", "test": "jest test/*.js", + "test:unit": "jest test/unit/*", "integration-test": "NODE_ENV=test ENV=dev jest ./test/integration/*.js --config puppeteer.config.js", "integration-test:ci": "start-server-and-test server http://localhost:4000 integration-test", "smoke-test": "NODE_ENV=test ENV=dev jest ./test/smoke-test/urls.test.js --config puppeteer.config.js", diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js new file mode 100644 index 000000000..5cd736867 --- /dev/null +++ b/src/util/timeoutQueue.js @@ -0,0 +1,25 @@ +const timeouts = {}; + +export const addTimeout = (component, fn, delay) => { + if (!timeouts[component]) timeouts[component] = []; + const newTimeout = setTimeout(() => { + fn(); + timeouts[component] = timeouts[component].filter((to) => to !== newTimeout); + }, delay); + timeouts[component].push(newTimeout); + return newTimeout; +}; + +export const removeTimeout = (component, timeout) => { + if (timeouts[component].some((to) => to === timeout)) { + clearTimeout(timeout); + timeouts[component] = timeouts[component].filter((to) => to !== timeout); + } +}; + +export const clearAllTimeouts = (component) => { + timeouts[component].forEach((to) => { + clearTimeout(to); + }); + timeouts[component] = []; +}; diff --git a/test/unit/util/timeoutQueue.test.js b/test/unit/util/timeoutQueue.test.js new file mode 100644 index 000000000..a67d7acea --- /dev/null +++ b/test/unit/util/timeoutQueue.test.js @@ -0,0 +1,44 @@ +import { addTimeout, removeTimeout, clearAllTimeouts } from '../../../src/util/timeoutQueue'; + +const wait = async (ms) => new Promise((resolve) => { + setTimeout(resolve, ms); +}); + +test('Add and execute timeouts', async () => { + let a = false; + let b = false; + addTimeout('test', () => { a = true; }, 100); + addTimeout('test', () => { b = true; }, 200); + await wait(70); + expect(a).toBe(false); + expect(b).toBe(false); + await wait(70); + expect(a).toBe(true); + expect(b).toBe(false); + await wait(70); + expect(b).toBe(true); +}); + +test('Added timeouts are recallable', async () => { + let b = false; + const toB = addTimeout('test', () => { b = true; }, 200); + await wait(110); + removeTimeout('test', toB); + await wait(100); + expect(b).toBe(false); +}); + +test('Can clear all timeout of a component', async () => { + let a = false; + let bOne = false; + let bTwo = false; + addTimeout('a', () => { a = true; }, 200); + addTimeout('b', () => { bOne = true; }, 200); + addTimeout('b', () => { bTwo = true; }, 200); + await wait(110); + clearAllTimeouts('b'); + await wait(100); + expect(a).toBe(true); + expect(bOne).toBe(false); + expect(bTwo).toBe(false); +}); From db61cc05f1733a468fb2a837eb3313695f7cc4ab Mon Sep 17 00:00:00 2001 From: Zh Date: Tue, 14 Jul 2020 21:27:40 -0400 Subject: [PATCH 2/9] replaced generic timeouts with queued in components --- src/components/map/map.js | 18 +++++++++++++----- src/components/narrative/ReactPageScroller.js | 17 +++++++++++++---- src/components/narrative/index.js | 2 ++ src/components/tree/phyloTree/change.js | 3 ++- src/components/tree/phyloTree/labels.js | 3 ++- src/components/tree/tangle/index.js | 5 +++-- src/components/tree/tree.js | 4 ++++ src/util/timeoutQueue.js | 12 +++++++----- test/unit/util/timeoutQueue.test.js | 9 +++++++++ 9 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/components/map/map.js b/src/components/map/map.js index 3ec2b2c5b..c9549c5bc 100644 --- a/src/components/map/map.js +++ b/src/components/map/map.js @@ -25,6 +25,7 @@ import { MAP_ANIMATION_PLAY_PAUSE_BUTTON } from "../../actions/types"; import { timerStart, timerEnd } from "../../util/perf"; import { tabSingle, darkGrey, lightGrey, goColor, pauseColor } from "../../globalStyles"; import ErrorBoundary from "../../util/errorBoundry"; +import { addTimeout, removeTimeout, clearAllTimeouts } from "../../util/timeoutQueue"; import Legend from "../tree/legend/legend"; import "../../css/mapbox.css"; @@ -144,10 +145,12 @@ class Map extends React.Component { if (this.state.map && (this.props.width !== nextProps.width || this.props.height !== nextProps.height)) { // first, clear any existing timeout if (this.map_timeout) { - window.clearTimeout(this.map_timeout); + removeTimeout(this.map_timeout); + this.map_timeout = null; } // delay to resize map (when complete, narrative will re-focus map on data) - this.map_timeout = window.setTimeout( + this.map_timeout = addTimeout( + 'map', this.invalidateMapSize.bind(this), this.props.narrativeMode ? 100 : 750 ); @@ -302,7 +305,7 @@ class Map extends React.Component { and the map is ready before the data (??). It's imperitive that this method runs so if the data's not ready yet we try to rerun it after a short time. This could be improved */ - window.setTimeout(() => this.respondToLeafletEvent(leafletEvent), 50); + addTimeout('map', () => this.respondToLeafletEvent(leafletEvent), 50); return; } @@ -640,10 +643,12 @@ class Map extends React.Component { const maxZoom = this.getMaxZoomForFittingMapToData(); // first, clear any existing timeout if (this.bounds_timeout) { - window.clearTimeout(this.bounds_timeout); + removeTimeout(this.bounds_timeout); + this.bounds_timeout = null; } // delay to change map bounds - this.bounds_timeout = window.setTimeout( + this.bounds_timeout = addTimeout( + 'map', (map) => { map.fitBounds(window.L.latLngBounds(SWNE[0], SWNE[1]), {maxZoom}); }, @@ -689,6 +694,9 @@ class Map extends React.Component { ); } + componentWillUnmount() { + clearAllTimeouts('map'); + } } const WithTranslation = withTranslation()(Map); diff --git a/src/components/narrative/ReactPageScroller.js b/src/components/narrative/ReactPageScroller.js index 59acee9e2..df5d164d1 100644 --- a/src/components/narrative/ReactPageScroller.js +++ b/src/components/narrative/ReactPageScroller.js @@ -4,6 +4,7 @@ import React from "react"; import PropTypes from "prop-types" import {isNull, isNil, isEqual} from "lodash"; +import { addTimeout } from '../../util/timeoutQueue'; const previousTouchMove = Symbol(); const scrolling = Symbol(); @@ -114,7 +115,9 @@ export default class ReactPageScroller extends React.Component { this.props.pageOnChange(this.state.componentIndex); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState((prevState) => ({componentIndex: prevState.componentIndex - 1}), () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -139,7 +142,9 @@ export default class ReactPageScroller extends React.Component { this.props.pageOnChange(this.state.componentIndex + 2); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState((prevState) => ({componentIndex: prevState.componentIndex + 1}), () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -228,7 +233,9 @@ export default class ReactPageScroller extends React.Component { ); - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState({componentIndex: number, componentsToRender: componentsToRender}, () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -268,7 +275,9 @@ export default class ReactPageScroller extends React.Component { pageOnChange(number + 1); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState({componentIndex: number}, () => { this[scrolling] = false; this[previousTouchMove] = null; diff --git a/src/components/narrative/index.js b/src/components/narrative/index.js index 07ddb3985..00143162b 100644 --- a/src/components/narrative/index.js +++ b/src/components/narrative/index.js @@ -17,6 +17,7 @@ import ReactPageScroller from "./ReactPageScroller"; import { changePage, EXPERIMENTAL_showMainDisplayMarkdown } from "../../actions/navigation"; import { CHANGE_URL_QUERY_BUT_NOT_REDUX_STATE } from "../../actions/types"; import { narrativeNavBarHeight } from "../../util/globals"; +import { clearAllTimeouts } from '../../util/timeoutQueue'; /* regarding refs: https://reactjs.org/docs/refs-and-the-dom.html#exposing-dom-refs-to-parent-components */ const progressHeight = 25; @@ -197,6 +198,7 @@ class Narrative extends React.Component { query: queryString.parse(this.props.blocks[this.props.currentInFocusBlockIdx].url) }); Mousetrap.unbind(['left', 'right', 'up', 'down']); + clearAllTimeouts('narrative'); } } export default Narrative; diff --git a/src/components/tree/phyloTree/change.js b/src/components/tree/phyloTree/change.js index e6b0edc79..3959c9e2b 100644 --- a/src/components/tree/phyloTree/change.js +++ b/src/components/tree/phyloTree/change.js @@ -2,6 +2,7 @@ import { timerFlush } from "d3-timer"; import { calcConfidenceWidth } from "./confidence"; import { applyToChildren } from "./helpers"; import { timerStart, timerEnd } from "../../../util/perf"; +import { addTimeout } from "../../../util/timeoutQueue"; import { NODE_VISIBLE } from "../../../util/globals"; import { getBranchVisibility, strokeForBranch } from "./renderers"; import { shouldDisplayTemporalConfidence } from "../../../reducers/controls"; @@ -221,7 +222,7 @@ export const modifySVGInStages = function modifySVGInStages(elemsToUpdate, svgPr if (!--inProgress) { /* decrement counter. When hits 0 run block */ const updateTips = createUpdateCall(".tip", svgPropsToUpdate); genericSelectAndModify(this.svg, ".tip", updateTips, transitionTimeMoveTips); - setTimeout(step3, transitionTimeMoveTips); + addTimeout('tree', step3, transitionTimeMoveTips); } }; diff --git a/src/components/tree/phyloTree/labels.js b/src/components/tree/phyloTree/labels.js index 29348c509..4e1158bfd 100644 --- a/src/components/tree/phyloTree/labels.js +++ b/src/components/tree/phyloTree/labels.js @@ -1,5 +1,6 @@ import { timerFlush } from "d3-timer"; import { NODE_VISIBLE } from "../../../util/globals"; +import { addTimeout } from "../../../util/timeoutQueue"; export const updateTipLabels = function updateTipLabels(dt) { if ("tipLabels" in this.groups) { @@ -29,7 +30,7 @@ export const updateTipLabels = function updateTipLabels(dt) { fontSize = this.params.tipLabelFontSizeL2; } - window.setTimeout(() => { + addTimeout('tree', () => { this.groups.tipLabels .selectAll('.tipLabel') .data(inViewVisibleTips) diff --git a/src/components/tree/tangle/index.js b/src/components/tree/tangle/index.js index c7df2a5a0..fd457a76e 100644 --- a/src/components/tree/tangle/index.js +++ b/src/components/tree/tangle/index.js @@ -2,6 +2,7 @@ import React from "react"; import { select } from "d3-selection"; import 'd3-transition'; +import { addTimeout, removeTimeout } from "../../../util/timeoutQueue"; const makeTipPathGenerator = (props) => (idxs) => { const tip1 = props.leftNodes[idxs[0]].shell; @@ -72,11 +73,11 @@ class Tangle extends React.Component { componentDidUpdate(prevProps) { if (this.props.width !== prevProps.width || this.props.height !== prevProps.height) { if (this.timeout) { - window.clearTimeout(this.timeout); + removeTimeout(this.timeout); } else { select(this.d3ref).selectAll(".tangleLine").remove(); } - this.timeout = window.setTimeout(this.drawLines.bind(this), 1000); + this.timeout = addTimeout('tree', this.drawLines.bind(this), 1000); } } render() { diff --git a/src/components/tree/tree.js b/src/components/tree/tree.js index 16e3c6095..0a64748b7 100644 --- a/src/components/tree/tree.js +++ b/src/components/tree/tree.js @@ -13,6 +13,7 @@ import { renderTree } from "./reactD3Interface/initialRender"; import Tangle from "./tangle"; import { attemptUntangle } from "../../util/globals"; import ErrorBoundary from "../../util/errorBoundry"; +import { clearAllTimeouts } from "../../util/timeoutQueue"; import { untangleTreeToo } from "./tangle/untangling"; export const spaceBetweenTrees = 100; @@ -179,6 +180,9 @@ class Tree extends React.Component { ); } + componentWillUnmount() { + clearAllTimeouts('tree'); + } } const WithTranslation = withTranslation()(Tree); diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js index 5cd736867..4276388b2 100644 --- a/src/util/timeoutQueue.js +++ b/src/util/timeoutQueue.js @@ -1,9 +1,9 @@ const timeouts = {}; -export const addTimeout = (component, fn, delay) => { +export const addTimeout = (component, fn, delay, ...args) => { if (!timeouts[component]) timeouts[component] = []; const newTimeout = setTimeout(() => { - fn(); + fn(...args); timeouts[component] = timeouts[component].filter((to) => to !== newTimeout); }, delay); timeouts[component].push(newTimeout); @@ -18,8 +18,10 @@ export const removeTimeout = (component, timeout) => { }; export const clearAllTimeouts = (component) => { - timeouts[component].forEach((to) => { - clearTimeout(to); - }); + if (timeouts[component]) { + timeouts[component].forEach((to) => { + clearTimeout(to); + }); + } timeouts[component] = []; }; diff --git a/test/unit/util/timeoutQueue.test.js b/test/unit/util/timeoutQueue.test.js index a67d7acea..ec68ae30d 100644 --- a/test/unit/util/timeoutQueue.test.js +++ b/test/unit/util/timeoutQueue.test.js @@ -19,6 +19,15 @@ test('Add and execute timeouts', async () => { expect(b).toBe(true); }); +test('Timeouts can execute with arg', async () => { + let a = 1; + addTimeout('test', (val) => { a = val; }, 100, 2); + await wait(60); + expect(a).toBe(1); + await wait(60); + expect(a).toBe(2); +}); + test('Added timeouts are recallable', async () => { let b = false; const toB = addTimeout('test', () => { b = true; }, 200); From 5bb61e5c7b31847414e2fbac01566468689ac4a6 Mon Sep 17 00:00:00 2001 From: Zh Date: Tue, 14 Jul 2020 20:03:36 -0400 Subject: [PATCH 3/9] add timeout util and test --- package.json | 1 + src/util/timeoutQueue.js | 25 ++++++++++++++++ test/unit/util/timeoutQueue.test.js | 44 +++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 src/util/timeoutQueue.js create mode 100644 test/unit/util/timeoutQueue.test.js diff --git a/package.json b/package.json index f88e1a837..0cf982da2 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "gzip-and-upload": "env bash ./scripts/gzip-and-upload.sh", "build-docs": "echo 'see ./docs-src/README.md'", "test": "jest test/*.js", + "test:unit": "jest test/unit/*", "integration-test": "NODE_ENV=test ENV=dev jest ./test/integration/*.js --config puppeteer.config.js", "integration-test:ci": "start-server-and-test server http://localhost:4000 integration-test", "smoke-test": "NODE_ENV=test ENV=dev jest ./test/smoke-test/urls.test.js --config puppeteer.config.js", diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js new file mode 100644 index 000000000..5cd736867 --- /dev/null +++ b/src/util/timeoutQueue.js @@ -0,0 +1,25 @@ +const timeouts = {}; + +export const addTimeout = (component, fn, delay) => { + if (!timeouts[component]) timeouts[component] = []; + const newTimeout = setTimeout(() => { + fn(); + timeouts[component] = timeouts[component].filter((to) => to !== newTimeout); + }, delay); + timeouts[component].push(newTimeout); + return newTimeout; +}; + +export const removeTimeout = (component, timeout) => { + if (timeouts[component].some((to) => to === timeout)) { + clearTimeout(timeout); + timeouts[component] = timeouts[component].filter((to) => to !== timeout); + } +}; + +export const clearAllTimeouts = (component) => { + timeouts[component].forEach((to) => { + clearTimeout(to); + }); + timeouts[component] = []; +}; diff --git a/test/unit/util/timeoutQueue.test.js b/test/unit/util/timeoutQueue.test.js new file mode 100644 index 000000000..a67d7acea --- /dev/null +++ b/test/unit/util/timeoutQueue.test.js @@ -0,0 +1,44 @@ +import { addTimeout, removeTimeout, clearAllTimeouts } from '../../../src/util/timeoutQueue'; + +const wait = async (ms) => new Promise((resolve) => { + setTimeout(resolve, ms); +}); + +test('Add and execute timeouts', async () => { + let a = false; + let b = false; + addTimeout('test', () => { a = true; }, 100); + addTimeout('test', () => { b = true; }, 200); + await wait(70); + expect(a).toBe(false); + expect(b).toBe(false); + await wait(70); + expect(a).toBe(true); + expect(b).toBe(false); + await wait(70); + expect(b).toBe(true); +}); + +test('Added timeouts are recallable', async () => { + let b = false; + const toB = addTimeout('test', () => { b = true; }, 200); + await wait(110); + removeTimeout('test', toB); + await wait(100); + expect(b).toBe(false); +}); + +test('Can clear all timeout of a component', async () => { + let a = false; + let bOne = false; + let bTwo = false; + addTimeout('a', () => { a = true; }, 200); + addTimeout('b', () => { bOne = true; }, 200); + addTimeout('b', () => { bTwo = true; }, 200); + await wait(110); + clearAllTimeouts('b'); + await wait(100); + expect(a).toBe(true); + expect(bOne).toBe(false); + expect(bTwo).toBe(false); +}); From 2a56f1812f2c83f79abde772f288192c126721d1 Mon Sep 17 00:00:00 2001 From: Zh Date: Tue, 14 Jul 2020 21:27:40 -0400 Subject: [PATCH 4/9] replaced generic timeouts with queued in components --- src/components/map/map.js | 19 ++++++++++++++----- src/components/narrative/ReactPageScroller.js | 17 +++++++++++++---- src/components/narrative/index.js | 5 +++++ src/components/tree/phyloTree/change.js | 3 ++- src/components/tree/phyloTree/labels.js | 3 ++- src/components/tree/tangle/index.js | 5 +++-- src/components/tree/tree.js | 4 ++++ src/util/timeoutQueue.js | 12 +++++++----- test/unit/util/timeoutQueue.test.js | 9 +++++++++ 9 files changed, 59 insertions(+), 18 deletions(-) diff --git a/src/components/map/map.js b/src/components/map/map.js index 0527f4f42..9cfaf2443 100644 --- a/src/components/map/map.js +++ b/src/components/map/map.js @@ -25,6 +25,7 @@ import { MAP_ANIMATION_PLAY_PAUSE_BUTTON } from "../../actions/types"; import { timerStart, timerEnd } from "../../util/perf"; import { tabSingle, darkGrey, lightGrey, goColor, pauseColor } from "../../globalStyles"; import ErrorBoundary from "../../util/errorBoundry"; +import { addTimeout, removeTimeout, clearAllTimeouts } from "../../util/timeoutQueue"; import Legend from "../tree/legend/legend"; import "../../css/mapbox.css"; @@ -144,10 +145,12 @@ class Map extends React.Component { if (this.state.map && (this.props.width !== nextProps.width || this.props.height !== nextProps.height)) { // first, clear any existing timeout if (this.map_timeout) { - window.clearTimeout(this.map_timeout); + removeTimeout(this.map_timeout); + this.map_timeout = null; } // delay to resize map (when complete, narrative will re-focus map on data) - this.map_timeout = window.setTimeout( + this.map_timeout = addTimeout( + 'map', this.invalidateMapSize.bind(this), this.props.narrativeMode ? 100 : 750 ); @@ -302,7 +305,7 @@ class Map extends React.Component { and the map is ready before the data (??). It's imperitive that this method runs so if the data's not ready yet we try to rerun it after a short time. This could be improved */ - window.setTimeout(() => this.respondToLeafletEvent(leafletEvent), 50); + addTimeout('map', () => this.respondToLeafletEvent(leafletEvent), 50); return; } @@ -640,10 +643,12 @@ class Map extends React.Component { const maxZoom = this.getMaxZoomForFittingMapToData(); // first, clear any existing timeout if (this.bounds_timeout) { - window.clearTimeout(this.bounds_timeout); + removeTimeout(this.bounds_timeout); + this.bounds_timeout = null; } // delay to change map bounds - this.bounds_timeout = window.setTimeout( + this.bounds_timeout = addTimeout( + 'map', (map) => { map.fitBounds(window.L.latLngBounds(SWNE[0], SWNE[1]), {maxZoom}); }, @@ -690,8 +695,12 @@ class Map extends React.Component { ); } componentWillUnmount() { +<<<<<<< HEAD this.state.map.off("moveend"); this.state.map.off("resize"); +======= + clearAllTimeouts('map'); +>>>>>>> replaced generic timeouts with queued in components } } diff --git a/src/components/narrative/ReactPageScroller.js b/src/components/narrative/ReactPageScroller.js index 59acee9e2..df5d164d1 100644 --- a/src/components/narrative/ReactPageScroller.js +++ b/src/components/narrative/ReactPageScroller.js @@ -4,6 +4,7 @@ import React from "react"; import PropTypes from "prop-types" import {isNull, isNil, isEqual} from "lodash"; +import { addTimeout } from '../../util/timeoutQueue'; const previousTouchMove = Symbol(); const scrolling = Symbol(); @@ -114,7 +115,9 @@ export default class ReactPageScroller extends React.Component { this.props.pageOnChange(this.state.componentIndex); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState((prevState) => ({componentIndex: prevState.componentIndex - 1}), () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -139,7 +142,9 @@ export default class ReactPageScroller extends React.Component { this.props.pageOnChange(this.state.componentIndex + 2); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState((prevState) => ({componentIndex: prevState.componentIndex + 1}), () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -228,7 +233,9 @@ export default class ReactPageScroller extends React.Component { ); - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState({componentIndex: number, componentsToRender: componentsToRender}, () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -268,7 +275,9 @@ export default class ReactPageScroller extends React.Component { pageOnChange(number + 1); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState({componentIndex: number}, () => { this[scrolling] = false; this[previousTouchMove] = null; diff --git a/src/components/narrative/index.js b/src/components/narrative/index.js index aa4984879..556efda15 100644 --- a/src/components/narrative/index.js +++ b/src/components/narrative/index.js @@ -15,7 +15,11 @@ import { import ReactPageScroller from "./ReactPageScroller"; import { changePage } from "../../actions/navigation"; import { narrativeNavBarHeight } from "../../util/globals"; +<<<<<<< HEAD import {TOGGLE_NARRATIVE} from "../../actions/types"; +======= +import { clearAllTimeouts } from '../../util/timeoutQueue'; +>>>>>>> replaced generic timeouts with queued in components /* regarding refs: https://reactjs.org/docs/refs-and-the-dom.html#exposing-dom-refs-to-parent-components */ const progressHeight = 25; @@ -174,6 +178,7 @@ class Narrative extends React.Component { } componentWillUnmount() { Mousetrap.unbind(['left', 'right', 'up', 'down']); + clearAllTimeouts('narrative'); } } export default Narrative; diff --git a/src/components/tree/phyloTree/change.js b/src/components/tree/phyloTree/change.js index e6b0edc79..3959c9e2b 100644 --- a/src/components/tree/phyloTree/change.js +++ b/src/components/tree/phyloTree/change.js @@ -2,6 +2,7 @@ import { timerFlush } from "d3-timer"; import { calcConfidenceWidth } from "./confidence"; import { applyToChildren } from "./helpers"; import { timerStart, timerEnd } from "../../../util/perf"; +import { addTimeout } from "../../../util/timeoutQueue"; import { NODE_VISIBLE } from "../../../util/globals"; import { getBranchVisibility, strokeForBranch } from "./renderers"; import { shouldDisplayTemporalConfidence } from "../../../reducers/controls"; @@ -221,7 +222,7 @@ export const modifySVGInStages = function modifySVGInStages(elemsToUpdate, svgPr if (!--inProgress) { /* decrement counter. When hits 0 run block */ const updateTips = createUpdateCall(".tip", svgPropsToUpdate); genericSelectAndModify(this.svg, ".tip", updateTips, transitionTimeMoveTips); - setTimeout(step3, transitionTimeMoveTips); + addTimeout('tree', step3, transitionTimeMoveTips); } }; diff --git a/src/components/tree/phyloTree/labels.js b/src/components/tree/phyloTree/labels.js index 29348c509..4e1158bfd 100644 --- a/src/components/tree/phyloTree/labels.js +++ b/src/components/tree/phyloTree/labels.js @@ -1,5 +1,6 @@ import { timerFlush } from "d3-timer"; import { NODE_VISIBLE } from "../../../util/globals"; +import { addTimeout } from "../../../util/timeoutQueue"; export const updateTipLabels = function updateTipLabels(dt) { if ("tipLabels" in this.groups) { @@ -29,7 +30,7 @@ export const updateTipLabels = function updateTipLabels(dt) { fontSize = this.params.tipLabelFontSizeL2; } - window.setTimeout(() => { + addTimeout('tree', () => { this.groups.tipLabels .selectAll('.tipLabel') .data(inViewVisibleTips) diff --git a/src/components/tree/tangle/index.js b/src/components/tree/tangle/index.js index c7df2a5a0..fd457a76e 100644 --- a/src/components/tree/tangle/index.js +++ b/src/components/tree/tangle/index.js @@ -2,6 +2,7 @@ import React from "react"; import { select } from "d3-selection"; import 'd3-transition'; +import { addTimeout, removeTimeout } from "../../../util/timeoutQueue"; const makeTipPathGenerator = (props) => (idxs) => { const tip1 = props.leftNodes[idxs[0]].shell; @@ -72,11 +73,11 @@ class Tangle extends React.Component { componentDidUpdate(prevProps) { if (this.props.width !== prevProps.width || this.props.height !== prevProps.height) { if (this.timeout) { - window.clearTimeout(this.timeout); + removeTimeout(this.timeout); } else { select(this.d3ref).selectAll(".tangleLine").remove(); } - this.timeout = window.setTimeout(this.drawLines.bind(this), 1000); + this.timeout = addTimeout('tree', this.drawLines.bind(this), 1000); } } render() { diff --git a/src/components/tree/tree.js b/src/components/tree/tree.js index 16e3c6095..0a64748b7 100644 --- a/src/components/tree/tree.js +++ b/src/components/tree/tree.js @@ -13,6 +13,7 @@ import { renderTree } from "./reactD3Interface/initialRender"; import Tangle from "./tangle"; import { attemptUntangle } from "../../util/globals"; import ErrorBoundary from "../../util/errorBoundry"; +import { clearAllTimeouts } from "../../util/timeoutQueue"; import { untangleTreeToo } from "./tangle/untangling"; export const spaceBetweenTrees = 100; @@ -179,6 +180,9 @@ class Tree extends React.Component { ); } + componentWillUnmount() { + clearAllTimeouts('tree'); + } } const WithTranslation = withTranslation()(Tree); diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js index 5cd736867..4276388b2 100644 --- a/src/util/timeoutQueue.js +++ b/src/util/timeoutQueue.js @@ -1,9 +1,9 @@ const timeouts = {}; -export const addTimeout = (component, fn, delay) => { +export const addTimeout = (component, fn, delay, ...args) => { if (!timeouts[component]) timeouts[component] = []; const newTimeout = setTimeout(() => { - fn(); + fn(...args); timeouts[component] = timeouts[component].filter((to) => to !== newTimeout); }, delay); timeouts[component].push(newTimeout); @@ -18,8 +18,10 @@ export const removeTimeout = (component, timeout) => { }; export const clearAllTimeouts = (component) => { - timeouts[component].forEach((to) => { - clearTimeout(to); - }); + if (timeouts[component]) { + timeouts[component].forEach((to) => { + clearTimeout(to); + }); + } timeouts[component] = []; }; diff --git a/test/unit/util/timeoutQueue.test.js b/test/unit/util/timeoutQueue.test.js index a67d7acea..ec68ae30d 100644 --- a/test/unit/util/timeoutQueue.test.js +++ b/test/unit/util/timeoutQueue.test.js @@ -19,6 +19,15 @@ test('Add and execute timeouts', async () => { expect(b).toBe(true); }); +test('Timeouts can execute with arg', async () => { + let a = 1; + addTimeout('test', (val) => { a = val; }, 100, 2); + await wait(60); + expect(a).toBe(1); + await wait(60); + expect(a).toBe(2); +}); + test('Added timeouts are recallable', async () => { let b = false; const toB = addTimeout('test', () => { b = true; }, 200); From 5457b8396eebaaf81ab84f6ebf3925086e007693 Mon Sep 17 00:00:00 2001 From: Zh Date: Thu, 13 Aug 2020 00:28:00 -0400 Subject: [PATCH 5/9] fixed removeTimeout calls without component arg --- src/components/map/map.js | 9 ++------- src/components/narrative/index.js | 3 --- src/components/tree/tangle/index.js | 2 +- src/util/timeoutQueue.js | 6 ++++++ 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/components/map/map.js b/src/components/map/map.js index 9cfaf2443..4e1c7f1e1 100644 --- a/src/components/map/map.js +++ b/src/components/map/map.js @@ -145,7 +145,7 @@ class Map extends React.Component { if (this.state.map && (this.props.width !== nextProps.width || this.props.height !== nextProps.height)) { // first, clear any existing timeout if (this.map_timeout) { - removeTimeout(this.map_timeout); + removeTimeout('map', this.map_timeout); this.map_timeout = null; } // delay to resize map (when complete, narrative will re-focus map on data) @@ -643,7 +643,7 @@ class Map extends React.Component { const maxZoom = this.getMaxZoomForFittingMapToData(); // first, clear any existing timeout if (this.bounds_timeout) { - removeTimeout(this.bounds_timeout); + removeTimeout('map', this.bounds_timeout); this.bounds_timeout = null; } // delay to change map bounds @@ -695,12 +695,7 @@ class Map extends React.Component { ); } componentWillUnmount() { -<<<<<<< HEAD - this.state.map.off("moveend"); - this.state.map.off("resize"); -======= clearAllTimeouts('map'); ->>>>>>> replaced generic timeouts with queued in components } } diff --git a/src/components/narrative/index.js b/src/components/narrative/index.js index 556efda15..1e40ae660 100644 --- a/src/components/narrative/index.js +++ b/src/components/narrative/index.js @@ -15,11 +15,8 @@ import { import ReactPageScroller from "./ReactPageScroller"; import { changePage } from "../../actions/navigation"; import { narrativeNavBarHeight } from "../../util/globals"; -<<<<<<< HEAD import {TOGGLE_NARRATIVE} from "../../actions/types"; -======= import { clearAllTimeouts } from '../../util/timeoutQueue'; ->>>>>>> replaced generic timeouts with queued in components /* regarding refs: https://reactjs.org/docs/refs-and-the-dom.html#exposing-dom-refs-to-parent-components */ const progressHeight = 25; diff --git a/src/components/tree/tangle/index.js b/src/components/tree/tangle/index.js index fd457a76e..287b899cc 100644 --- a/src/components/tree/tangle/index.js +++ b/src/components/tree/tangle/index.js @@ -73,7 +73,7 @@ class Tangle extends React.Component { componentDidUpdate(prevProps) { if (this.props.width !== prevProps.width || this.props.height !== prevProps.height) { if (this.timeout) { - removeTimeout(this.timeout); + removeTimeout('tree', this.timeout); } else { select(this.d3ref).selectAll(".tangleLine").remove(); } diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js index 4276388b2..d97ee4bfc 100644 --- a/src/util/timeoutQueue.js +++ b/src/util/timeoutQueue.js @@ -11,9 +11,15 @@ export const addTimeout = (component, fn, delay, ...args) => { }; export const removeTimeout = (component, timeout) => { + if (!timeouts[component]) { + console.error('Cannot remove timeout for component', component, 'ref', timeout); + return; + } if (timeouts[component].some((to) => to === timeout)) { clearTimeout(timeout); timeouts[component] = timeouts[component].filter((to) => to !== timeout); + } else { + console.error('Timeout is not in component', component, 'ref', timeout); } }; From fbd5a33218d97151d1c73e4e7c03b496154ebb3b Mon Sep 17 00:00:00 2001 From: Zh Date: Tue, 14 Jul 2020 20:03:36 -0400 Subject: [PATCH 6/9] add timeout util and test --- package.json | 1 + src/util/timeoutQueue.js | 25 ++++++++++++++++ test/unit/util/timeoutQueue.test.js | 44 +++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 src/util/timeoutQueue.js create mode 100644 test/unit/util/timeoutQueue.test.js diff --git a/package.json b/package.json index f88e1a837..0cf982da2 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "gzip-and-upload": "env bash ./scripts/gzip-and-upload.sh", "build-docs": "echo 'see ./docs-src/README.md'", "test": "jest test/*.js", + "test:unit": "jest test/unit/*", "integration-test": "NODE_ENV=test ENV=dev jest ./test/integration/*.js --config puppeteer.config.js", "integration-test:ci": "start-server-and-test server http://localhost:4000 integration-test", "smoke-test": "NODE_ENV=test ENV=dev jest ./test/smoke-test/urls.test.js --config puppeteer.config.js", diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js new file mode 100644 index 000000000..5cd736867 --- /dev/null +++ b/src/util/timeoutQueue.js @@ -0,0 +1,25 @@ +const timeouts = {}; + +export const addTimeout = (component, fn, delay) => { + if (!timeouts[component]) timeouts[component] = []; + const newTimeout = setTimeout(() => { + fn(); + timeouts[component] = timeouts[component].filter((to) => to !== newTimeout); + }, delay); + timeouts[component].push(newTimeout); + return newTimeout; +}; + +export const removeTimeout = (component, timeout) => { + if (timeouts[component].some((to) => to === timeout)) { + clearTimeout(timeout); + timeouts[component] = timeouts[component].filter((to) => to !== timeout); + } +}; + +export const clearAllTimeouts = (component) => { + timeouts[component].forEach((to) => { + clearTimeout(to); + }); + timeouts[component] = []; +}; diff --git a/test/unit/util/timeoutQueue.test.js b/test/unit/util/timeoutQueue.test.js new file mode 100644 index 000000000..a67d7acea --- /dev/null +++ b/test/unit/util/timeoutQueue.test.js @@ -0,0 +1,44 @@ +import { addTimeout, removeTimeout, clearAllTimeouts } from '../../../src/util/timeoutQueue'; + +const wait = async (ms) => new Promise((resolve) => { + setTimeout(resolve, ms); +}); + +test('Add and execute timeouts', async () => { + let a = false; + let b = false; + addTimeout('test', () => { a = true; }, 100); + addTimeout('test', () => { b = true; }, 200); + await wait(70); + expect(a).toBe(false); + expect(b).toBe(false); + await wait(70); + expect(a).toBe(true); + expect(b).toBe(false); + await wait(70); + expect(b).toBe(true); +}); + +test('Added timeouts are recallable', async () => { + let b = false; + const toB = addTimeout('test', () => { b = true; }, 200); + await wait(110); + removeTimeout('test', toB); + await wait(100); + expect(b).toBe(false); +}); + +test('Can clear all timeout of a component', async () => { + let a = false; + let bOne = false; + let bTwo = false; + addTimeout('a', () => { a = true; }, 200); + addTimeout('b', () => { bOne = true; }, 200); + addTimeout('b', () => { bTwo = true; }, 200); + await wait(110); + clearAllTimeouts('b'); + await wait(100); + expect(a).toBe(true); + expect(bOne).toBe(false); + expect(bTwo).toBe(false); +}); From 155e995b0687c7aae38fe9e56743d102c9556a81 Mon Sep 17 00:00:00 2001 From: Zh Date: Tue, 14 Jul 2020 21:27:40 -0400 Subject: [PATCH 7/9] replaced generic timeouts with queued in components --- src/components/map/map.js | 19 ++++++++++++++----- src/components/narrative/ReactPageScroller.js | 17 +++++++++++++---- src/components/narrative/index.js | 5 +++++ src/components/tree/phyloTree/change.js | 3 ++- src/components/tree/phyloTree/labels.js | 3 ++- src/components/tree/tangle/index.js | 5 +++-- src/components/tree/tree.js | 4 ++++ src/util/timeoutQueue.js | 12 +++++++----- test/unit/util/timeoutQueue.test.js | 9 +++++++++ 9 files changed, 59 insertions(+), 18 deletions(-) diff --git a/src/components/map/map.js b/src/components/map/map.js index 0527f4f42..9cfaf2443 100644 --- a/src/components/map/map.js +++ b/src/components/map/map.js @@ -25,6 +25,7 @@ import { MAP_ANIMATION_PLAY_PAUSE_BUTTON } from "../../actions/types"; import { timerStart, timerEnd } from "../../util/perf"; import { tabSingle, darkGrey, lightGrey, goColor, pauseColor } from "../../globalStyles"; import ErrorBoundary from "../../util/errorBoundry"; +import { addTimeout, removeTimeout, clearAllTimeouts } from "../../util/timeoutQueue"; import Legend from "../tree/legend/legend"; import "../../css/mapbox.css"; @@ -144,10 +145,12 @@ class Map extends React.Component { if (this.state.map && (this.props.width !== nextProps.width || this.props.height !== nextProps.height)) { // first, clear any existing timeout if (this.map_timeout) { - window.clearTimeout(this.map_timeout); + removeTimeout(this.map_timeout); + this.map_timeout = null; } // delay to resize map (when complete, narrative will re-focus map on data) - this.map_timeout = window.setTimeout( + this.map_timeout = addTimeout( + 'map', this.invalidateMapSize.bind(this), this.props.narrativeMode ? 100 : 750 ); @@ -302,7 +305,7 @@ class Map extends React.Component { and the map is ready before the data (??). It's imperitive that this method runs so if the data's not ready yet we try to rerun it after a short time. This could be improved */ - window.setTimeout(() => this.respondToLeafletEvent(leafletEvent), 50); + addTimeout('map', () => this.respondToLeafletEvent(leafletEvent), 50); return; } @@ -640,10 +643,12 @@ class Map extends React.Component { const maxZoom = this.getMaxZoomForFittingMapToData(); // first, clear any existing timeout if (this.bounds_timeout) { - window.clearTimeout(this.bounds_timeout); + removeTimeout(this.bounds_timeout); + this.bounds_timeout = null; } // delay to change map bounds - this.bounds_timeout = window.setTimeout( + this.bounds_timeout = addTimeout( + 'map', (map) => { map.fitBounds(window.L.latLngBounds(SWNE[0], SWNE[1]), {maxZoom}); }, @@ -690,8 +695,12 @@ class Map extends React.Component { ); } componentWillUnmount() { +<<<<<<< HEAD this.state.map.off("moveend"); this.state.map.off("resize"); +======= + clearAllTimeouts('map'); +>>>>>>> replaced generic timeouts with queued in components } } diff --git a/src/components/narrative/ReactPageScroller.js b/src/components/narrative/ReactPageScroller.js index 59acee9e2..df5d164d1 100644 --- a/src/components/narrative/ReactPageScroller.js +++ b/src/components/narrative/ReactPageScroller.js @@ -4,6 +4,7 @@ import React from "react"; import PropTypes from "prop-types" import {isNull, isNil, isEqual} from "lodash"; +import { addTimeout } from '../../util/timeoutQueue'; const previousTouchMove = Symbol(); const scrolling = Symbol(); @@ -114,7 +115,9 @@ export default class ReactPageScroller extends React.Component { this.props.pageOnChange(this.state.componentIndex); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState((prevState) => ({componentIndex: prevState.componentIndex - 1}), () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -139,7 +142,9 @@ export default class ReactPageScroller extends React.Component { this.props.pageOnChange(this.state.componentIndex + 2); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState((prevState) => ({componentIndex: prevState.componentIndex + 1}), () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -228,7 +233,9 @@ export default class ReactPageScroller extends React.Component { ); - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState({componentIndex: number, componentsToRender: componentsToRender}, () => { this[scrolling] = false; this[previousTouchMove] = null; @@ -268,7 +275,9 @@ export default class ReactPageScroller extends React.Component { pageOnChange(number + 1); } - setTimeout(() => { + addTimeout( + 'narrative', + () => { this.setState({componentIndex: number}, () => { this[scrolling] = false; this[previousTouchMove] = null; diff --git a/src/components/narrative/index.js b/src/components/narrative/index.js index aa4984879..556efda15 100644 --- a/src/components/narrative/index.js +++ b/src/components/narrative/index.js @@ -15,7 +15,11 @@ import { import ReactPageScroller from "./ReactPageScroller"; import { changePage } from "../../actions/navigation"; import { narrativeNavBarHeight } from "../../util/globals"; +<<<<<<< HEAD import {TOGGLE_NARRATIVE} from "../../actions/types"; +======= +import { clearAllTimeouts } from '../../util/timeoutQueue'; +>>>>>>> replaced generic timeouts with queued in components /* regarding refs: https://reactjs.org/docs/refs-and-the-dom.html#exposing-dom-refs-to-parent-components */ const progressHeight = 25; @@ -174,6 +178,7 @@ class Narrative extends React.Component { } componentWillUnmount() { Mousetrap.unbind(['left', 'right', 'up', 'down']); + clearAllTimeouts('narrative'); } } export default Narrative; diff --git a/src/components/tree/phyloTree/change.js b/src/components/tree/phyloTree/change.js index e6b0edc79..3959c9e2b 100644 --- a/src/components/tree/phyloTree/change.js +++ b/src/components/tree/phyloTree/change.js @@ -2,6 +2,7 @@ import { timerFlush } from "d3-timer"; import { calcConfidenceWidth } from "./confidence"; import { applyToChildren } from "./helpers"; import { timerStart, timerEnd } from "../../../util/perf"; +import { addTimeout } from "../../../util/timeoutQueue"; import { NODE_VISIBLE } from "../../../util/globals"; import { getBranchVisibility, strokeForBranch } from "./renderers"; import { shouldDisplayTemporalConfidence } from "../../../reducers/controls"; @@ -221,7 +222,7 @@ export const modifySVGInStages = function modifySVGInStages(elemsToUpdate, svgPr if (!--inProgress) { /* decrement counter. When hits 0 run block */ const updateTips = createUpdateCall(".tip", svgPropsToUpdate); genericSelectAndModify(this.svg, ".tip", updateTips, transitionTimeMoveTips); - setTimeout(step3, transitionTimeMoveTips); + addTimeout('tree', step3, transitionTimeMoveTips); } }; diff --git a/src/components/tree/phyloTree/labels.js b/src/components/tree/phyloTree/labels.js index 29348c509..4e1158bfd 100644 --- a/src/components/tree/phyloTree/labels.js +++ b/src/components/tree/phyloTree/labels.js @@ -1,5 +1,6 @@ import { timerFlush } from "d3-timer"; import { NODE_VISIBLE } from "../../../util/globals"; +import { addTimeout } from "../../../util/timeoutQueue"; export const updateTipLabels = function updateTipLabels(dt) { if ("tipLabels" in this.groups) { @@ -29,7 +30,7 @@ export const updateTipLabels = function updateTipLabels(dt) { fontSize = this.params.tipLabelFontSizeL2; } - window.setTimeout(() => { + addTimeout('tree', () => { this.groups.tipLabels .selectAll('.tipLabel') .data(inViewVisibleTips) diff --git a/src/components/tree/tangle/index.js b/src/components/tree/tangle/index.js index c7df2a5a0..fd457a76e 100644 --- a/src/components/tree/tangle/index.js +++ b/src/components/tree/tangle/index.js @@ -2,6 +2,7 @@ import React from "react"; import { select } from "d3-selection"; import 'd3-transition'; +import { addTimeout, removeTimeout } from "../../../util/timeoutQueue"; const makeTipPathGenerator = (props) => (idxs) => { const tip1 = props.leftNodes[idxs[0]].shell; @@ -72,11 +73,11 @@ class Tangle extends React.Component { componentDidUpdate(prevProps) { if (this.props.width !== prevProps.width || this.props.height !== prevProps.height) { if (this.timeout) { - window.clearTimeout(this.timeout); + removeTimeout(this.timeout); } else { select(this.d3ref).selectAll(".tangleLine").remove(); } - this.timeout = window.setTimeout(this.drawLines.bind(this), 1000); + this.timeout = addTimeout('tree', this.drawLines.bind(this), 1000); } } render() { diff --git a/src/components/tree/tree.js b/src/components/tree/tree.js index 16e3c6095..0a64748b7 100644 --- a/src/components/tree/tree.js +++ b/src/components/tree/tree.js @@ -13,6 +13,7 @@ import { renderTree } from "./reactD3Interface/initialRender"; import Tangle from "./tangle"; import { attemptUntangle } from "../../util/globals"; import ErrorBoundary from "../../util/errorBoundry"; +import { clearAllTimeouts } from "../../util/timeoutQueue"; import { untangleTreeToo } from "./tangle/untangling"; export const spaceBetweenTrees = 100; @@ -179,6 +180,9 @@ class Tree extends React.Component { ); } + componentWillUnmount() { + clearAllTimeouts('tree'); + } } const WithTranslation = withTranslation()(Tree); diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js index 5cd736867..4276388b2 100644 --- a/src/util/timeoutQueue.js +++ b/src/util/timeoutQueue.js @@ -1,9 +1,9 @@ const timeouts = {}; -export const addTimeout = (component, fn, delay) => { +export const addTimeout = (component, fn, delay, ...args) => { if (!timeouts[component]) timeouts[component] = []; const newTimeout = setTimeout(() => { - fn(); + fn(...args); timeouts[component] = timeouts[component].filter((to) => to !== newTimeout); }, delay); timeouts[component].push(newTimeout); @@ -18,8 +18,10 @@ export const removeTimeout = (component, timeout) => { }; export const clearAllTimeouts = (component) => { - timeouts[component].forEach((to) => { - clearTimeout(to); - }); + if (timeouts[component]) { + timeouts[component].forEach((to) => { + clearTimeout(to); + }); + } timeouts[component] = []; }; diff --git a/test/unit/util/timeoutQueue.test.js b/test/unit/util/timeoutQueue.test.js index a67d7acea..ec68ae30d 100644 --- a/test/unit/util/timeoutQueue.test.js +++ b/test/unit/util/timeoutQueue.test.js @@ -19,6 +19,15 @@ test('Add and execute timeouts', async () => { expect(b).toBe(true); }); +test('Timeouts can execute with arg', async () => { + let a = 1; + addTimeout('test', (val) => { a = val; }, 100, 2); + await wait(60); + expect(a).toBe(1); + await wait(60); + expect(a).toBe(2); +}); + test('Added timeouts are recallable', async () => { let b = false; const toB = addTimeout('test', () => { b = true; }, 200); From 7c8e95bab6a637c30d58ccbe2fce75fa98e6672a Mon Sep 17 00:00:00 2001 From: Zh Date: Thu, 13 Aug 2020 00:28:00 -0400 Subject: [PATCH 8/9] fixed removeTimeout calls without component arg --- src/components/map/map.js | 9 ++------- src/components/narrative/index.js | 3 --- src/components/tree/tangle/index.js | 2 +- src/util/timeoutQueue.js | 6 ++++++ 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/components/map/map.js b/src/components/map/map.js index 9cfaf2443..4e1c7f1e1 100644 --- a/src/components/map/map.js +++ b/src/components/map/map.js @@ -145,7 +145,7 @@ class Map extends React.Component { if (this.state.map && (this.props.width !== nextProps.width || this.props.height !== nextProps.height)) { // first, clear any existing timeout if (this.map_timeout) { - removeTimeout(this.map_timeout); + removeTimeout('map', this.map_timeout); this.map_timeout = null; } // delay to resize map (when complete, narrative will re-focus map on data) @@ -643,7 +643,7 @@ class Map extends React.Component { const maxZoom = this.getMaxZoomForFittingMapToData(); // first, clear any existing timeout if (this.bounds_timeout) { - removeTimeout(this.bounds_timeout); + removeTimeout('map', this.bounds_timeout); this.bounds_timeout = null; } // delay to change map bounds @@ -695,12 +695,7 @@ class Map extends React.Component { ); } componentWillUnmount() { -<<<<<<< HEAD - this.state.map.off("moveend"); - this.state.map.off("resize"); -======= clearAllTimeouts('map'); ->>>>>>> replaced generic timeouts with queued in components } } diff --git a/src/components/narrative/index.js b/src/components/narrative/index.js index 556efda15..1e40ae660 100644 --- a/src/components/narrative/index.js +++ b/src/components/narrative/index.js @@ -15,11 +15,8 @@ import { import ReactPageScroller from "./ReactPageScroller"; import { changePage } from "../../actions/navigation"; import { narrativeNavBarHeight } from "../../util/globals"; -<<<<<<< HEAD import {TOGGLE_NARRATIVE} from "../../actions/types"; -======= import { clearAllTimeouts } from '../../util/timeoutQueue'; ->>>>>>> replaced generic timeouts with queued in components /* regarding refs: https://reactjs.org/docs/refs-and-the-dom.html#exposing-dom-refs-to-parent-components */ const progressHeight = 25; diff --git a/src/components/tree/tangle/index.js b/src/components/tree/tangle/index.js index fd457a76e..287b899cc 100644 --- a/src/components/tree/tangle/index.js +++ b/src/components/tree/tangle/index.js @@ -73,7 +73,7 @@ class Tangle extends React.Component { componentDidUpdate(prevProps) { if (this.props.width !== prevProps.width || this.props.height !== prevProps.height) { if (this.timeout) { - removeTimeout(this.timeout); + removeTimeout('tree', this.timeout); } else { select(this.d3ref).selectAll(".tangleLine").remove(); } diff --git a/src/util/timeoutQueue.js b/src/util/timeoutQueue.js index 4276388b2..d97ee4bfc 100644 --- a/src/util/timeoutQueue.js +++ b/src/util/timeoutQueue.js @@ -11,9 +11,15 @@ export const addTimeout = (component, fn, delay, ...args) => { }; export const removeTimeout = (component, timeout) => { + if (!timeouts[component]) { + console.error('Cannot remove timeout for component', component, 'ref', timeout); + return; + } if (timeouts[component].some((to) => to === timeout)) { clearTimeout(timeout); timeouts[component] = timeouts[component].filter((to) => to !== timeout); + } else { + console.error('Timeout is not in component', component, 'ref', timeout); } }; From 6d5af0a4662e4fa4cef6df85ce3458a11e8d912c Mon Sep 17 00:00:00 2001 From: Zh Date: Thu, 13 Aug 2020 19:19:52 -0400 Subject: [PATCH 9/9] fix merging error --- package-lock.json | 2 +- src/components/map/map.js | 2 ++ src/components/tree/tangle/index.js | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index bfe9dd472..9235ad5eb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "auspice", - "version": "2.17.4", + "version": "2.18.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/src/components/map/map.js b/src/components/map/map.js index 4e1c7f1e1..a818a0aea 100644 --- a/src/components/map/map.js +++ b/src/components/map/map.js @@ -695,6 +695,8 @@ class Map extends React.Component { ); } componentWillUnmount() { + this.state.map.off("moveend"); + this.state.map.off("resize"); clearAllTimeouts('map'); } } diff --git a/src/components/tree/tangle/index.js b/src/components/tree/tangle/index.js index 287b899cc..11a036480 100644 --- a/src/components/tree/tangle/index.js +++ b/src/components/tree/tangle/index.js @@ -74,6 +74,7 @@ class Tangle extends React.Component { if (this.props.width !== prevProps.width || this.props.height !== prevProps.height) { if (this.timeout) { removeTimeout('tree', this.timeout); + this.timeout = null; } else { select(this.d3ref).selectAll(".tangleLine").remove(); }