-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Should the cueing arrows reappear after pressing Erase button? #299
Comments
We can discuss it as a group but the reason for this behavior lies in The reset in public reset(): void {
this.wasManipulatedProperty.reset();
// Reset every CurvePoint to its initial state.
this.points.forEach( point => point.reset() );
this.curveChangedEmitter.emit();
} In some ways we are overloading the term 'reset in public reset(): void {
this.wasManipulatedProperty.reset();
this.resetPoints()
}
public resetPoints(): void{
// Reset every CurvePoint to its initial state.
this.points.forEach( point => point.reset() );
this.curveChangedEmitter.emit();
} |
I don't feel strongly either way. But when the curve is reset to y=0, it tends to disappear (vusually) into the x-axis, so the cueing arrows seem like a useful reminder. But if we do decide not to show the cueing arrows after Eraser... Isn't it also odd that the cueing arrows reappear after pressing the "Reset All" button ? Why do the cueing arrows ever appear after the user has interacted with the curve? |
@veillette said:
Agreed. But rather than |
One other thought... Does the EraserButton currently behave correctly for saved PhET-iO state? I don't think so. My feeling is that it should always set the curve to |
My recommendation is:
|
Signed-off-by: Martin Veillette <[email protected]>
The commit above introduces a new method that "erases" the curve by setting its point values to zero and its point type to smooth. The method is invoked when pressing the eraser button. I have not done any changes to the Reset All button and reset functionality of transformed curve so their functionality is preserved. I tested the functionality of it in studio, by loading an initial curve onto the sim. The erase button sets the curve values to zero, and the reset button sets it to its initial value. |
@Nancy-Salpepi please verify in master, close if OK. |
The implementation and behavior looks nice to me. But we haven't decided whether the cueing arrows should be restored or not (they currently are not). Should we wait to have @Nancy-Salpepi review until we've discussed with @amanda-phet, so that @Nancy-Salpepi is only reviewing once? |
That's right, lets discuss with @amanda-phet first. You are off the hook @Nancy-Salpepi :) |
Discussed with @pixelzoom and @veillette We'd like the erase button to reset the curve to y=0 and not show cueing arrows. |
We are good to go then. The previous commit sets the curve to zero for phet brand and phet-io when erasing . The cueing arrows will not be present afterwards if the user had previously interacting with the curve. I'll note that pressing the erase button before interacting with the curve will not remove the cueing arrows. @Nancy-Salpepi please verify in master, close if OK. |
This looks good in master for both brands. Closing! |
Test device
MacBook Air (m1 chip)
Operating System
13.2.1
Browser
Safari 16.3
Problem description
For phetsims/qa#921, I noticed that on all of the screens, pressing the Erase button results in the yellow hint arrows reappearing. I wasn't sure if that was the intended design or if the hint arrows should only reappear on Reset All (like in Center and Variability)?
The text was updated successfully, but these errors were encountered: