feat: slider drag creates single undo action (#888)

v3/src/components/slider/slider-model.test.ts

@@ -32,6 +32,7 @@ describe("SliderModel", () => {
       sliderModel: { globalValue: g1.id }
     }, { sharedModelManager: mockSharedModelManagerWithGlobalValueManager })
     const slider = tree.sliderModel
+    expect(slider.globalValueManager).toBeDefined()
 
     expect(isSliderModel()).toBe(false)
     expect(isSliderModel(slider)).toBe(true)
@@ -68,6 +69,7 @@ describe("SliderModel", () => {
       sliderModel: { globalValue: g1.id }
     }, { sharedModelManager: mockSharedModelManagerWithoutGlobalValueManager })
     const slider = tree.sliderModel
+    expect(slider.globalValueManager).toBeUndefined()
     expect(isSliderModel()).toBe(false)
     expect(isSliderModel(slider)).toBe(true)
     destroy(tree)
@@ -84,4 +86,50 @@ describe("SliderModel", () => {
     destroy(tree)
   })
 
+  it("can update value dynamically (without undo) and then with undo", () => {
+    const tree = Tree.create({
+      globalValue: g1,
+      sliderModel: { globalValue: g1.id }
+    })
+    const slider = tree.sliderModel
+    expect(slider.isUpdatingDynamically).toBe(false)
+    const initialValue = slider.value
+    const dynamicValue = initialValue + 1
+    const finalValue = dynamicValue + 1
+    slider.setDynamicValue(dynamicValue)
+    expect(slider.isUpdatingDynamically).toBe(true)
+    expect(slider.value).toBe(dynamicValue)
+    expect(slider.globalValue.value).toBe(initialValue)
+    slider.applyUndoableAction(() => slider.setValue(finalValue), "Undo slider change", "Redo slider change")
+    expect(slider.isUpdatingDynamically).toBe(false)
+    expect(slider.value).toBe(finalValue)
+    expect(slider.globalValue.value).toBe(finalValue)
+    expect(slider.dynamicValue).toBeUndefined()
+  })
+
+  it("responds to axis domain changes", () => {
+    const tree = Tree.create({
+      globalValue: g1,
+      sliderModel: { globalValue: g1.id }
+    })
+    const slider = tree.sliderModel
+    slider.setAxisMax(20)
+    slider.setAxisMin(10)
+    expect(slider.value).toBe(10)
+    slider.setAxisMin(0)
+    slider.setAxisMax(5)
+    expect(slider.value).toBe(5)
+
+    expect(slider.validateValue(-1, () => slider.axis.min, () => slider.axis.max)).toBe(0)
+    expect(slider.validateValue(3, () => slider.axis.min, () => slider.axis.max)).toBe(3)
+    expect(slider.validateValue(6, () => slider.axis.min, () => slider.axis.max)).toBe(5)
+
+    slider.setAxisMax(20)
+    slider.setAxisMin(10)
+    slider.encompassValue(0)
+    expect(slider.axis.min).toBe(-2)
+    slider.setAxisMin(10)
+    slider.encompassValue(30)
+    expect(slider.axis.max).toBe(32)
+  })
 })

v3/src/components/slider/slider-model.ts

@@ -25,16 +25,23 @@ export const SliderModel = TileContentModel
     _animationRate: types.maybe(types.number),  // frames per second
     axis: types.optional(NumericAxisModel, { place: 'bottom', min: -0.5, max: 11.5 })
   })
+  .volatile(self => ({
+    // defined while dragging (or animating?), undefined otherwise
+    dynamicValue: undefined as number | undefined
+  }))
   .views(self => ({
     get name() {
       return self.globalValue.name
     },
     get value() {
-      return self.globalValue.value
+      return self.dynamicValue ?? self.globalValue.value
     },
     get domain() {
       return self.axis.domain
     },
+    get isUpdatingDynamically() {
+      return self.dynamicValue != null
+    },
     get increment() {
       // TODO: implement v2 algorithm which determines default increment from axis bounds
       return self.multipleOf || 0.5
@@ -48,8 +55,8 @@ export const SliderModel = TileContentModel
       return sharedModels?.[0] as IGlobalValueManager | undefined
     }
   }))
-  .actions(self => ({
-    setValue(n: number) {
+  .views(self => ({
+    constrainValue(value: number) {
       // keep value in bounds of axis min and max when thumbnail is dragged
       const keepValueInBounds = (num: number) => {
         if (num < self.axis.min) return self.axis.min
@@ -58,14 +65,18 @@ export const SliderModel = TileContentModel
       }
 
       if (self.multipleOf) {
-        n = Math.round(n / self.multipleOf) * self.multipleOf
-        n = keepValueInBounds(n)
-      } else {
-        n = keepValueInBounds(n)
+        value = Math.round(value / self.multipleOf) * self.multipleOf
       }
-      self.globalValue.setValue(n)
-
-      withUndoRedoStrings("DG.Undo.slider.change", "DG.Redo.slider.change")
+      return keepValueInBounds(value)
+    }
+  }))
+  .actions(self => ({
+    setDynamicValue(value: number) {
+      self.dynamicValue = self.constrainValue(value)
+    },
+    setValue(value: number) {
+      self.globalValue.setValue(self.constrainValue(value))
+      self.dynamicValue = undefined
     },
   }))
   .actions(self => ({
@@ -76,7 +87,7 @@ export const SliderModel = TileContentModel
           // keep the thumbnail within axis bounds when axis bounds are changed
           if (self.value < self.axis.min) self.setValue(self.axis.min)
           if (self.value > self.axis.max) self.setValue(self.axis.max)
-        }
+        }, { name: "SliderModel [axis.domain]" }
       ))
     },
     afterAttachToDocument() {
@@ -93,7 +104,7 @@ export const SliderModel = TileContentModel
             // once we're added to the document, update the shared model reference
             globalValueManager && sharedModelManager.addTileSharedModel(self, globalValueManager)
           }
-        }, { fireImmediately: true }
+        }, { name: "SliderModel [sharedModelManager]", fireImmediately: true }
       ))
     },
     beforeDestroy() {
@@ -153,6 +164,14 @@ export const SliderModel = TileContentModel
       }
     },
   }))
+.actions(self => ({
+  // performs the specified action so that response actions are included and undo/redo strings assigned
+  applyUndoableAction<T = unknown>(actionFn: () => T, undoStringKey: string, redoStringKey: string) {
+    const result = actionFn()
+    withUndoRedoStrings(undoStringKey, redoStringKey)
+    return result
+  }
+}))
 
 export interface ISliderModel extends Instance<typeof SliderModel> {}
 

v3/src/components/slider/slider-thumb.tsx

@@ -37,17 +37,29 @@ export const CodapSliderThumb = observer(function CodapSliderThumb({sliderContai
   useEffect(() => {
     const containerX = sliderContainer?.getBoundingClientRect().x
 
-    const handlePointerMove = (e: PointerEvent) => {
+    function getSliderValueFromEvent(e: PointerEvent) {
       if ((containerX != null) && isDragging) {
         const pixelTarget = e.clientX + downOffset.current
-        const scaledValue = scale?.getDataCoordinate(pixelTarget - containerX).data ?? 0
-        sliderModel.setValue(scaledValue)
+        return scale?.getDataCoordinate(pixelTarget - containerX).data ?? 0
+      }
+    }
+
+    const handlePointerMove = (e: PointerEvent) => {
+      const sliderValue = getSliderValueFromEvent(e)
+      if (sliderValue != null) {
+        sliderModel.setDynamicValue(sliderValue)
       }
       e.preventDefault()
       e.stopImmediatePropagation()
     }
 
     const handlePointerUp = (e: PointerEvent) => {
+      const sliderValue = getSliderValueFromEvent(e)
+      if (sliderValue != null) {
+        sliderModel.applyUndoableAction(
+          () => sliderModel.setValue(sliderValue),
+          "DG.Undo.slider.change", "DG.Redo.slider.change")
+      }
       downOffset.current = 0
       setIsDragging(false)
       e.preventDefault()