search

phetsims / number-compare

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

Shouldn't there be a limit to the number of 10 cards that can be used? #25

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip

Operating System 13.2.1

Browser Safari

Problem description For https://github.com/phetsims/qa/issues/917, on the lab screen, there doesn't seem to be a limit to the number of 10 cards I can put in the play area. Shouldn't there be one?

Visuals

Screenshot 2023-03-15 at 2 55 34 PM
chrisklus commented 1 year ago

Thanks @Nancy-Salpepi, nice catch! @amanda-phet could you please comment how many ten frames we should allow? Based on the space available above, I'm thinking 10-12 maybe? After commenting, please reassign to @marlitas and @pixelzoom to implement at their convenience, whoever's up for it.

marlitas commented 1 year ago

Oooh mine!

amanda-phet commented 1 year ago

I agree, 10-12 seems the most reasonable for dev bounds (and I can't imagine a good pedagogical use beyond 10). Let's say 10.

Over to @marlitas to implement this change.

marlitas commented 1 year ago

Addressed in the above commits. I started by just eliminating pickability when we reached 10 Ten Frames. However this behavior seemed a bit confusing and remembered some other use cases where the draggable icon no longer appeared as though to indicate the stack ran out. I created a commit for that option as well.

Below are two videos showing both options. If the first option is preferred it would be very easy to revert the second commit where we toggle the icon's visibility.

Option 1: Limit pickability

https://user-images.githubusercontent.com/42476338/226421227-92146e44-79a7-4a1c-933e-f173dc80ea08.mov

Option 2: Limit visiblity

https://user-images.githubusercontent.com/42476338/226421056-8f21eef2-4cab-4848-b468-16525e7330db.mov

@amanda-phet & @chrisklus let me know your preference.

chrisklus commented 1 year ago

@marlitas and I paired on this. We decided to keep the visibility implementation because the rest of the items on this screen follow the same pattern. We also refactored TenFrameNode.getTenFramePath to take PathOptions so we could simplify the implementation by passing in a visibleProperty option that is a DerivedProperty. We think this is working as expected.

@Nancy-Salpepi can you please confirm on phettest? Feel free to close if looking good.

Nancy-Salpepi commented 1 year ago

Looks good! There are now 10 Ten Frames. The toolbox looks empty when all ten are in the play area and the icon reappears when one is returned.

Closing!