From ca88ae8da5eaa4ef9c21bebd18f724fbfca45a06 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Mon, 18 Nov 2024 11:11:25 +1300 Subject: [PATCH 1/2] [entropy] Fix out-of-sync entropy-events state Changing between "Entropy - Events" results in two dispatches (`ENTROPY_COUNTS_TOGGLE` and `ENTROPY_DATA`) however the entropy code would ignore the first update if it didn't contain new entropy data. This bug has probably existed for the life of the entropy panel however was masked by those two actions happening back-to-back and being (always?) bundled together by redux. Recent work in #1879 (v2.60.0) meant these actions were no longer back-to-back and thus the bug was revealed. Closes #1905 --- src/components/entropy/entropyD3.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/entropy/entropyD3.js b/src/components/entropy/entropyD3.js index 2cc3f7f35..9edd6c275 100644 --- a/src/components/entropy/entropyD3.js +++ b/src/components/entropy/entropyD3.js @@ -64,8 +64,9 @@ EntropyChart.prototype.update = function update({ this._setZoomBounds(); } + if (showCounts!==undefined) this.showCounts = showCounts; + if (newBars || selectedPositions!==undefined) { - if (showCounts!==undefined) this.showCounts = showCounts; if (newBars) { this.bars = newBars[0]; this._updateOffsets(); @@ -75,6 +76,8 @@ EntropyChart.prototype.update = function update({ if (selectedPositions !== undefined) { this.selectedPositions = selectedPositions; } + /* TODO XXX: there's a potential bug here if selectedCds is set but we don't enter this code block + due to the (newBars || selectedPositions!==undefined) conditional */ if (selectedCds || selectedPositions !== undefined) { this._setZoomCoordinates(zoomMin, zoomMax, !!selectedCds); } From 117deb2f58d70d48a3bdc7594b77c61fcf3f2cf3 Mon Sep 17 00:00:00 2001 From: james hadfield Date: Mon, 18 Nov 2024 10:29:32 +1300 Subject: [PATCH 2/2] Revert skipping of entropy calc on leading edge This reverts commit 0c91724786d5b7f556defbcd002a9c003fac4962. That commit was the first performance optimisation within #1879. The idea was to skip the expensive recalculation when initially dragging the date slider. In other contexts this change was a negative, specifically when there was only one action (e.g. changing entropy to events) as we essentially introduced a 500ms delay. Note that when the panel is off-screen subsequent work in #1879 means that the recalculation is skipped anyway, so reverting this commit only affects performance when the panel is on screen and we're dragging the date slider. To remedy this we should add a code path which specifically skips the leading edge calculation when dragging the date slider. Essentially closes #1899 - that issue was partly the recalculation on-demand which remains, but greatly exacerbated by the unnecessary 500ms delay. --- src/actions/entropy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actions/entropy.js b/src/actions/entropy.js index de15c6bb7..a20159c96 100644 --- a/src/actions/entropy.js +++ b/src/actions/entropy.js @@ -24,7 +24,7 @@ export const updateEntropyVisibility = debounce((dispatch, getState) => { const [data, maxYVal] = calcEntropyInView(tree.nodes, tree.visibility, entropy.selectedCds, entropy.showCounts); dispatch({type: types.ENTROPY_DATA, data, maxYVal}); -}, 500, { leading: false, trailing: true }); +}, 500, { leading: true, trailing: true }); /** * Returns a thunk which makes zero or one dispatches to update the entropy reducer