phetsims / calculus-grapher

"Calculus Grapher" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 4 forks source link

Undo button, resetting to initial state when reaching undo limit seems odd #287

Closed marlitas closed 1 year ago

marlitas commented 1 year ago

from https://github.com/phetsims/calculus-grapher/issues/268

It seems strange to me that once a user reaches the undo limit, their 21st button hit resets to the initial state.

const pointState = ( this.undoStack.length === 0 ) ? this.initialState : this.undoStack.pop()!;

Why not hold on the last saved state in the stack (don't pop), and just grey out the undo button once the limit is reached? This is a standard UI practice. I've interacted multiple times with software where I am limited to a certain amount of undos and the undo button is then greyed out when I reach that limit. The first that comes to mind is the Adobe suite.

veillette commented 1 year ago

I'll note the relation to #219, which we can revisit.

pixelzoom commented 1 year ago

I agree that an empty undo stack shouldn't reset to the initial state -- it should simply do nothing. I'm not convinced about disabling the Undo button -- we decided that was not the effort in #219.

pixelzoom commented 1 year ago

In the above commit, CurvePoint undo is now a no-op when the stack is empty.

I'm going to make an executive decision and say that we're not going to revisit #219. It's just too complicated to keep track of which curve we're editing, and whether it has anything to undo. And nothing has changed since our last discussion and decision.

@veillette if the above change looks good to you, please close.

veillette commented 1 year ago

Since we save the initial state in the undo stack, this is only relevant if we get above the 20 operations. That seems reasonable to me. Closing