Skip to content
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

Mass thumb can move on its own #292

Closed
Nancy-Salpepi opened this issue Nov 13, 2023 · 11 comments
Closed

Mass thumb can move on its own #292

Nancy-Salpepi opened this issue Nov 13, 2023 · 11 comments
Assignees
Labels

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air M1 chip

Operating System
14.0

Browser
Safari 17

Problem description
Discovered while testing #274 on main:
Reaching the max mass by continually pressing the increment button and then moving the thumb left, will cause the thumb to move on its own until it reaches the max value. Even pressing Reset All will not stop it from moving.

--This occurs on both screens with all masses.
--This doesn't happen if I press and hold the increment button until the max value is reached.

Steps to reproduce
Here is an example:

  1. On the Intro screen, press the increment button for Mass 1 continuously until the max mass is reached and the button is disabled.
  2. Decrease the mass by moving the slider thumb.

Visuals

sliderThumbMoves.mp4
@Nancy-Salpepi Nancy-Salpepi added the type:bug Something isn't working label Nov 13, 2023
@Nancy-Salpepi
Copy link
Author

This also occurs with the decrement button using similar steps

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 13, 2023

Reproduced in main.

This problem does not occur in 1.3.0-dev.4 that I published 11/9/23 @ 3:25PM MT. So this must be related to changes that were made since then.

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 13, 2023

Hang on... I do see the problem in 1.3.0-dev.4. You need to press the increment/decrement button multiple times to get to the max/min, versus holding the button down. Very weird.

@AgustinVallejo
Copy link
Contributor

It's also happening in the baseline and in the published version! I also noticed that if you press the button again it stops, so perhaps there's something in the endCallback that we can look into?

@AgustinVallejo
Copy link
Contributor

AgustinVallejo commented Nov 13, 2023

I commented this chunk in ValuesColumnNode.ts:119 and it fixed itself. Looking if it's the right thing to do or it would break something:

        arrowButtonOptions: {
          fireOnDown: true
        },

I thought commenting the fireOnDown would change the proper behavior of the button but it seems the same as the baseline, minus the problem. Any thoughts @pixelzoom?

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 13, 2023

Thoughts:

Removing fireDown: true is a good thing to do. I don't know why that would have been added in the first place, because it's not standard behavior for PhET buttons. Buttons typically fire on 'up'.

I suspect that this may be a general problem with NumberControl and/or ArrowButton, and should probably be investigated. Or at least reported in a scenery-phet issue.

@AgustinVallejo
Copy link
Contributor

Assigning back to @Nancy-Salpepi to review, please close if OK.

@pixelzoom
Copy link
Contributor

I confirmed that this is a general problem with NumberControl, and reported in phetsims/scenery-phet#825.

@Nancy-Salpepi
Copy link
Author

This is fixed on main.
So sorry for the delay!

@phet-dev
Copy link
Contributor

Reopening because there is a TODO marked for this issue.

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 29, 2023

I converted the TODO in NumberControl to a comment that refers to the proper scenery-phet issue. See phetsims/scenery-phet@17db17d.

Closing again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants