search

allen-cell-animated / website-3d-cell-viewer

Other
5 stars 5 forks source link

Time slider stops moving after being dragged #280

Closed toloudis closed 2 months ago

toloudis commented 3 months ago

Description

Can get into a state where the time slider stops moving during playback.

Expected Behavior

Time slider should move during playback.

Reproduction

https://dev-aics-dtp-001.int.allencell.org/website-3d-cell-viewer/viewer?url=https%3A%2F%2Fdev-aics-dtp-001.int.allencell.org%2F%2Fassay-dev%2Fcomputational%2Fdata%2Fholistic%2Fendos%2Ffeasibility%2Fcdh5.ome.zarr

press play, the time slider moves.

Then pause, then drag slider ahead, then press play.

BUG Slider no longer moves!!!

Environment

Windows, Chrome

ShrimpCryptid commented 3 months ago

Temporarily fixed by disabling memoization and built to production. Will need @frasercl to look at this when he gets back.

From debugging, I think the issue is that NouiSlider is considering itself "interacted with" and then stops syncing with the provided start value. We don't have direct control over the value it has selected, only the start field; normally, we change start as soon as we receive an onChange callback so the selected value + the start is always in sync. Then, if we change start again, NouiSlider considers itself still at the initial "default" state and changes its value to match the new start value.

image

It looks like an extra render is happening between after the onChange callback and before onEnd is called where the old start value slips through... and then NouiSlider considers itself edited/interacted with and no longer returns to the start value when we change it later.

ShrimpCryptid commented 3 months ago

Validation for temporary fix: https://volumeviewer.allencell.org/viewer?url=https%3A%2F%2Fdev-aics-dtp-001.int.allencell.org%2F%2Fassay-dev%2Fcomputational%2Fdata%2Fholistic%2Fendos%2Ffeasibility%2Fcdh5.ome.zarr

frasercl commented 3 months ago

This looks like a very reasonable interim fix - in fact, I'll have to verify whether the problem SmarterSlider was trying to solve still exists. It might be that the odd performance issues aren't with us anymore, or else aren't worth the odd bugs.