phetsims / arithmetic

"Arithmetic" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/arithmetic
GNU General Public License v3.0
5 stars 5 forks source link

backspace key clears the user's guess #178

Closed pixelzoom closed 7 years ago

pixelzoom commented 7 years ago

Cleaning up the implementation of the backspace button in NumberKeypad (see https://github.com/phetsims/scenery-phet/issues/280#issuecomment-259477319) causes Arithmetic to behave undesirably. The backspace key should edit the user's previous guess, not clear it.

To demonstrate: • start sim • go to Multiply screen • type '22' on keypad • press 'Check' button • press backspace on keypad • should display '2', but displays ''

The problem is that Arithmetic's state machine is relying on the unnecessary valueStringProperty.value = '' that was removed from the backspace button listener in https://github.com/phetsims/scenery-phet/issues/280.

To identify the locations in Arithmetic that are causing the problem, I added this to NumberKeypad constructor, immediately after the definition of valueStringProperty:

    this.valueStringProperty.link( function( value ) {
      console.log( 'value=' + value );
      debugger;
    } );

Stepping through the demonstration example described above, I identified 2 places where a single press of backspace incorrectly clears the keypad:

(1) WorkspaceNode line 184:

182      model.inputProperty.link( function( input ) {
183        if ( model.state === GameState.DISPLAYING_INCORRECT_ANSWER_FEEDBACK ) {
184          model.retryProblem();
185        }
186        updateCheckButtonState();
186      } );

...where model.retryProblem is defined in ArithmeticModel, line 156:

155    retryProblem: function() {
156      this.inputProperty.reset();
157      this.state = GameState.AWAITING_USER_INPUT;
158    },

(2) WorkspaceNode line 146:

145        else if ( newGameState === GameState.AWAITING_USER_INPUT ) {
146         keypad.clear();
147        }

Assigned to @jbphet to fix.

pixelzoom commented 7 years ago

Btw... I think that this problem is a result of the dubious armForNewEntry feature in NumberKeypad. The responsibility for clearing the keypad is misplaced - it should be up to the client to clear it explicitly by calling NumberKeypad.clear. And if doing something special with the backspace button is important (which it obviously is in arithmetic and area-builder) then we could add the ability to listen to the backspace button.

jbphet commented 7 years ago

I fixed this by moving around where some of the clearing of the user entries occurs. I'd like a little QA time on this to make sure that I caught all cases. The main use case is when the user enters an incorrect value and then, instead of pressing the "Try Again" button, they either start entering a new value or press the backspace key.

Assigning to @phet-steele for testing.

phet-steele commented 7 years ago

Works just fine 👍