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

Use offset for touch dragging #197

Closed
zepumph opened this issue Sep 18, 2023 · 14 comments
Closed

Use offset for touch dragging #197

zepumph opened this issue Sep 18, 2023 · 14 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 18, 2023

While working on issues from dev testing, I noticed that my finger completely covers up the particle view while dragging. In other spots in code we use an offset to still be able to see the dragging node, like AbstractCoinTermNode. I think we should play around with this a bit.

Note the code is in shred so I'll just make an option like "touchOffset"

@zepumph zepumph self-assigned this Sep 18, 2023
zepumph added a commit to phetsims/shred that referenced this issue Sep 18, 2023
@zepumph
Copy link
Member Author

zepumph commented Sep 18, 2023

This was simple enough to do that I decided to implement it as the design conversation. @ariel-phet, do you like the offset applied on main? We can make it less or more, or get rid of it. I personally feel like it is an improvement.

@zepumph
Copy link
Member Author

zepumph commented Sep 18, 2023

@Nancy-Salpepi FYI in case you have thoughts.

@Nancy-Salpepi
Copy link

@zepumph I think this is great. It is more user friendly for sure.

@Nancy-Salpepi
Copy link

Wondering if we need this much offset @zepumph? I see that in Expression Exchange it is a lot less and doesn't look so jumpy.

@ariel-phet
Copy link
Contributor

@zepumph @Nancy-Salpepi I don't have access to a great touch device currently.

I think the idea of an offset is appropriate, and in this case I think @Nancy-Salpepi is well qualified to make the call of what looks best. I would say fine tune with her!

@zepumph
Copy link
Member Author

zepumph commented Sep 20, 2023

To find the perfect number, I made a query parameter ?particleTouchOffset=20. @Nancy-Salpepi you tested with a value of 30 and thought it was a bit too much. The current default is 20. Let me know what you want the value to be.

Wondering if we need this much offset @zepumph? I see that in Expression Exchange it is a lot less and doesn't look so jumpy.

In expression exchange we have a bit of an easier time because the object is bigger, so you don't have to move it as much to make it visible above your finger. The particles are pretty tiny.

I personally don't mind a jump too much if it means that the particle is clearly visible as I drag. I tried a value of 15 and couldn't really see the particle still (but it also didn't jump)! That said I have fat fingers.

I don't really have an opinion here, but wanted to give you the data I found while working on this. Let me know what you like and we should do that.

@zepumph zepumph removed their assignment Sep 20, 2023
@Nancy-Salpepi
Copy link

@zepumph the behavior I see today with ?particleTouchOffset=30 is not the same as the other day. The particle is much closer to my fingertip and not jumpy. Was there another change that may have cause this?

@zepumph
Copy link
Member Author

zepumph commented Sep 20, 2023

Hmmm, not that I know of. Weird!

@Nancy-Salpepi
Copy link

Is the behavior the same for you today as it was the other day @zepumph?

@Nancy-Salpepi
Copy link

The current behavior I am seeing with ?particleTouchOffset=30 looks good to me. What I initially saw and commented on in #197 (comment) was behavior similar to ?particleTouchOffset=70. Not sure why there was this inconsistency.

zepumph added a commit that referenced this issue Sep 21, 2023
zepumph added a commit that referenced this issue Sep 21, 2023
@zepumph
Copy link
Member Author

zepumph commented Sep 21, 2023

Is the behavior the same for you today as it was the other day @zepumph?

Ahh shoot. I totally gas-lit you. I thought that I originally set it to 30, but I actually set it to 60 before. So sorry. It is 30 now and the query parameter has been removed. Do you like it?

@zepumph zepumph assigned Nancy-Salpepi and unassigned zepumph Sep 21, 2023
zepumph added a commit that referenced this issue Sep 22, 2023
zepumph added a commit that referenced this issue Sep 22, 2023
@zepumph
Copy link
Member Author

zepumph commented Sep 22, 2023

After discussing with @Nancy-Salpepi, we brought back the query parameter to play with more. Note that this really depends on the size of the window. A small sim will definitely hide at 30, but 30 in a full screen sim seems really nice on my screen. @Nancy-Salpepi let me know what you like!

@Nancy-Salpepi
Copy link

I tested on both the iPad and iPhone. Once I zoom in a bit, my finger doesn't hide the particle at 30 on the phone. Let's stick with 30.
I think I decided what house to buy and planned my entire wedding faster than I made this decision 😂.

@Luisav1
Copy link
Contributor

Luisav1 commented Sep 25, 2023

@zepumph and I removed the touchOffset query parameter and made it permanently 30. Closing.

@Luisav1 Luisav1 closed this as completed Sep 25, 2023
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

4 participants