It is possible to "undo" after pressing the Erase button

[Nancy-Salpepi]

Test device MacBook Air (and Dell) Operating System macOS 13.2.1 (and Win10)

Browser safari, chrome and FF

Problem description For https://github.com/phetsims/qa/issues/924, it is possible to "undo" after pressing the Erase button. This was not possible in dev.25.

Steps to reproduce

1. On any screen, click on 2 different areas in the graph to make a curve with 2 hills.
2. Press the Erase button
3. Press the Undo button

NOTE: If you only make one hill instead of 2, press the Erase button and then press the Undo button--it looks like the Undo button is working correctly because you don't see anything happen (but it has actually undone that last action of making the hill).

Visuals

https://user-images.githubusercontent.com/87318828/229852151-a59ffa5a-5266-4512-a123-12835e807c63.mp4

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Calculus Grapher‬ URL: https://phet-dev.colorado.edu/html/calculus-grapher/1.0.0-rc.1/phet/calculus-grapher_all_phet.html Version: 1.0.0-rc.1 2023-03-28 17:54:45 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/16.3 Safari/605.1.15 Language: en-US Window: 1185x705 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

[pixelzoom]

Good catch. This problem was introduced in https://github.com/phetsims/calculus-grapher/commit/ff903f8238002dffe7764bb8ab46e98355947bcc for https://github.com/phetsims/calculus-grapher/issues/299, which was reported during dev testing.

In TransformedCurve, erase does not clear the undo stack.

Proposed fix:

Add this to CurvePoint.ts (and it should be used in reset):

public clearUndoStack() {
  this.undoStack.length = 0;
}

Then in TransformedCurve.ts:

  public erase(): void {

-   // Set the point values to zero and smooth type.
+   // Set the curve to y = 0, and clear the undo stack.
    this.points.forEach( point => {
      point.y = 0;
      point.pointType = 'smooth';
+     point.clearUndoStack();
    } );

    // Signal that this Curve has changed.
    this.curveChangedEmitter.emit();
  }

[pixelzoom]

We should also be certain that "erase" is not an action that one can "undo". Because another way to approach this would be to save the curve before doing the erase, so that the erase can be undone. In which case the only change needed is:

  public erase(): void {
+    this.save();

[veillette]

DIscussed with @pixelzoom, that it would be nice to consider erase as simply another operation. Just as smooth can be undo, the erase button could be undoable as well. So that the second option is preferable.

After checking, I found that the flash simulation was capable of undoing the eraser operation.

[pixelzoom]

In the above commits (master and 1.0), erase is treated the same as any other operation, and can be undone. @amanda-phet FYI.

@Nancy-Salpepi Could you please have a look in master, and report back on whether this makes sense to you? If it does, please change the status of this issue to "fixed-awaiting-deploy", and unassign yourself.

[Nancy-Salpepi]

I like this change a lot and it is working nicely in master.

[veillette]

To verify in the next RC:

Start the PhET brand sim.

1. Go to the Advanced screen.
2. Add multiple curves with different modes onto the f(x) graph.
3. Press the Erase Button
4. Verify that the curve is zeroed
5. Add a curve to the graph.
6. Press the undo to check that you recover the y=0 state
7. Press the undo button additional time to check that you still have the entire history of the curve.

In essence, the erase is simply another operation. Just as the functionality of the smooth button can be undo, the erase button is undoable as well.

Also Check that the reset all button truly resets the curve, that is that pressing the undo button after a reset all is a no-op operation.

[Nancy-Salpepi]

This looks good in rc.2 Closing