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

"incorrect" objects can't always automatically move off ten frame #30

Closed
Nancy-Salpepi opened this issue Mar 17, 2023 · 9 comments
Closed
Assignees
Labels
status:on-hold type:bug Something isn't working

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air M1 chip and iPad 9th generation

Operating System
13.2.1 and 16.3.1

Browser
Safari

Problem description
For phetsims/qa#917, on the Lab Screen:
If I try to add an object that "doesn't belong" to a partially filled ten frame located at the top or side of a play area, that object can't move up any further or to the side any further so it stays where I placed it.

Steps to reproduce

  1. On the Lab Screen, place a ten frame in the top left of the play area
  2. Add an apple
  3. Move a dog to the square right of the apple and let go.
  4. Move a dog to the square underneath the apple and let go.

Visuals
Screenshot 2023-03-17 at 8 48 47 AM
Screenshot 2023-03-17 at 8 49 32 AM

@Nancy-Salpepi Nancy-Salpepi added the type:bug Something isn't working label Mar 17, 2023
@amanda-phet
Copy link
Contributor

This is indeed a bad user experience. Discussed with @chrisklus and the current logic is to look for the nearest spot outside the ten frame, but the code doesn't check for whether that spot is in bounds or not. Chris will spend 30 minutes exploring how doable this is, and if it's going to be too much developer time and/or introduce other weird behavior, we need to not fix this until a client asks for this behavior.

@Nancy-Salpepi
Copy link
Author

also noting something similar when ten frames partially overlap:

butterfly.mp4

@zepumph
Copy link
Member

zepumph commented Mar 28, 2023

Part of this has been fixed by #29 where the potential for return button bounds narrowed the left draggable dimension by at least the width of a CountingObject. I will still need to look into the other kinds of cases. I think that #30 (comment) may be excusable since it is such a specific case (should still check with @amanda-phet though), but the top case should be detected for. Looks like the bottom isn't an issue because the tenframe will just be returned to the toolbox.

@zepumph zepumph self-assigned this Mar 28, 2023
@chrisklus
Copy link
Contributor

@zepumph or @chrisklus can take this, see TenFrame.pushAwayCountingObject

@zepumph
Copy link
Member

zepumph commented Mar 28, 2023

@chrisklus and I got this working. @Nancy-Salpepi can you please provide us with your testing prowess and feel free to close is all is well.

@Nancy-Salpepi
Copy link
Author

What I noted in my original comment is fixed! The objects can now move off the ten frame! They do tend to end up on top of one another though. I didn't know if that was OK or not. Sending it back to you guys for a final decision.

applesStack.mp4

@zepumph
Copy link
Member

zepumph commented Mar 29, 2023

I'm not too worried about this, but perhaps it would be good to come back to this problem after we have sorted out phetsims/number-play#172 (today), that is about a very similar issue that we may want to apply here as well.

@amanda-phet
Copy link
Contributor

This is unfortunate, but the Lab screen here mimics the Fractions Lab screen, and users can place things right on top of others and it seems to be acceptable behavior for this kind of screen. I'm ok allowing objects to stack in this way.

@chrisklus
Copy link
Contributor

Thanks for weighing in @amanda-phet. I think that means we are good to close here. Thanks for all of your time figuring out these cases @Nancy-Salpepi!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:on-hold type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants