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

Jumpy Number Cards out of Carousel on Lab Screen #223

Closed
Tracked by #932
veillette opened this issue Apr 7, 2023 · 15 comments
Closed
Tracked by #932

Jumpy Number Cards out of Carousel on Lab Screen #223

veillette opened this issue Apr 7, 2023 · 15 comments

Comments

@veillette
Copy link

Test device
Dell XPS 15

Operating System
Windows 10

Browser
Chrome

Problem description
While working on phetsims/qa#929, I noticed that while working on the lab screens, the dragging behavior of cards out out the carrousel is somewhat jumpy with a mouse. When attempting to drag a number card, a card appears while you press the card. The location of the card is such that the cursor is beneath the number (nice!). However, once a drag is initiated, the card suddenly moves below the carrousel and the cursor is no longer tied to the position of the card.

It is worth noting that
https://phet-dev.colorado.edu/html/number-play/1.1.0-dev.10/phet/number-play_all_phet.html?screens=4
does not exhibit this behavior.

See below for a video/gif
JumpyCards

I think that 1.1.0-rc.1 wants to ensure that on the first drag the number card is guaranteed to end up in the play area. Although it does succeed in that respect, however, it seems a bit disconcerting to have the cards jump around to achieve this goal.

@veillette veillette changed the title Jumpy cards on Lab Screen Jumpy Number Cards out of Carousel on Lab Screen Apr 7, 2023
@veillette
Copy link
Author

I'll note that in 1.1.0-rc.1 it is not possible to return cards to the carousel whereas it is possible in 1.1.0-dev.10, but I am assuming that it is a deliberate decision.

chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 7, 2023
@chrisklus
Copy link
Contributor

Thanks @veillette! Looks like an accidental use of the symbol cards' bounds Property instead of the number cards' that happened during some cleanup.

Fixed in the above commit, assigning @veillette or @KatieWoe to please confirm on phettest, thanks!

SHA for patching: 24776163b74acb9760d44509c9dbc50f1af1104e.

@chrisklus chrisklus assigned veillette and KatieWoe and unassigned chrisklus Apr 8, 2023
@chrisklus chrisklus added type:bug Something isn't working status:ready-for-review labels Apr 8, 2023
@veillette
Copy link
Author

@chrisklus: the change above fixes the problem with the number picker carousel on master. Well done.

However, I should point out that the vertical number operation carousel shows a buggy behavior on master, whereas it is perfectly fine on 1.1.0-rc.1. You cannot return the operations in the carousel on master.

@veillette veillette assigned chrisklus and unassigned KatieWoe and veillette Apr 10, 2023
chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 10, 2023
@chrisklus
Copy link
Contributor

Sorry @veillette! That was very silly of me (again). Should be good to go this time, can you please test on phettest?

@chrisklus chrisklus assigned veillette and unassigned chrisklus Apr 10, 2023
@veillette
Copy link
Author

veillette commented Apr 10, 2023

Thanks @chrisklus . The dragging behavior of the number cards and symbol cards work fine on master/phettest. Both type of cards can be returned to their respective carousel as expected.

@veillette
Copy link
Author

Assigning back to @chrisklus

@chrisklus
Copy link
Contributor

Thanks @veillette!

chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 10, 2023
chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 10, 2023
chrisklus added a commit that referenced this issue Apr 10, 2023
@chrisklus
Copy link
Contributor

@zepumph and I applied the SHAs from this issue to the release branch, 1.1, then confirmed locally. We are ready to leave this for RC spot check testing. This can be closed once confirmed in the RC spot check, thanks!

@KatieWoe
Copy link

This looks fixed to me for the most part, but I did notice that moving the cards around caused a sort of flickering of the numbers on the other cards. Is this related and/or an issue?

Number.Play.-.Google.Chrome.2023-04-13.10-46-11.mp4

@Nancy-Salpepi
Copy link

I also saw that if I grabbed a card close to the top, it initially goes a bit offscreen and then is a little jumpy when I begin to bring it down. Not sure how big of a deal it is.

jumpy.mp4

@veillette
Copy link
Author

I observe the same type of behavior as @Nancy-Salpepi and @KatieWoe . However, I would qualify this as minor in comparison to the issue in 1.1.0-rc.1 where one could not bring the cards back to the carousel. Personally I would think that the drag behavior is much improved in 1.1.0-rc.2 and we can now bring cards back into the carousel.

@veillette veillette removed their assignment Apr 13, 2023
@chrisklus
Copy link
Contributor

Thanks all! I discussed with @amanda-phet. We are not concerned about #223 (comment). I have an idea for making #223 (comment) better and think it is worth 10-20 minutes to try, but not blocking.

chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 13, 2023
chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 13, 2023
@chrisklus
Copy link
Contributor

Committed above. Note that it was also possible to get #223 (comment) to happen by grabbing the right edge of a symbol card. These should both be fixed, @Nancy-Salpepi can you please confirm on phettest? thanks!

@Nancy-Salpepi
Copy link

This looks good to me @chrisklus!

chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 13, 2023
chrisklus added a commit to phetsims/number-suite-common that referenced this issue Apr 13, 2023
@chrisklus
Copy link
Contributor

Thanks @Nancy-Salpepi! This is looking great in https://phet-dev.colorado.edu/html/number-play/1.1.0-rc.3/phet/number-play_all_phet.html, I think we are set to close here.

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

No branches or pull requests

5 participants