Resolves #222: moving one clipping slider caused other sliders to reset. This turns out to have been another stale closure issue.
What went wrong
So we've got this state variable called viewerSettings, right? It holds all the global settings for the viewer. Most of its keys are primitive types (e.g. density: number; showBoundingBox: boolean;). But the clipping ranges set by these sliders live in an object: region: { x: [number, number], y: [number, number], z: [number, number] };. This meant that when a clipping slider had to change a value in only one axis, it had to know the current value in the others to keep the update pure:
...and the closure that we passed down to the sliders to make the above call was holding on to outdated versions of region.
What I did to (hopefully) fix it
Whenever changeViewerSetting is asked to update a setting whose value is an object, I let it take a partial object:
changeViewerSetting("region", { x: [min, max] });
Now there's no reason for that closure to hold on to region anymore. Letting changeViewerSetting take an occasional partial object took a bit of extra type trickery, so I tried to document the expanded types to ease understanding a bit.
Review size: small (~10min)
Resolves #222: moving one clipping slider caused other sliders to reset. This turns out to have been another stale closure issue.
What went wrong
So we've got this state variable called
viewerSettings, right? It holds all the global settings for the viewer. Most of its keys are primitive types (e.g.density: number; showBoundingBox: boolean;). But the clipping ranges set by these sliders live in an object:region: { x: [number, number], y: [number, number], z: [number, number] };. This meant that when a clipping slider had to change a value in only one axis, it had to know the current value in the others to keep the update pure:...and the closure that we passed down to the sliders to make the above call was holding on to outdated versions of
region.What I did to (hopefully) fix it
Whenever
changeViewerSettingis asked to update a setting whose value is an object, I let it take a partial object:Now there's no reason for that closure to hold on to
regionanymore. LettingchangeViewerSettingtake an occasional partial object took a bit of extra type trickery, so I tried to document the expanded types to ease understanding a bit.