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

Fixes Touch Functionality #4213

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

jivansh77
Copy link
Contributor

@jivansh77 jivansh77 commented Dec 30, 2024

Description

  1. Fixed two-finger scroll direction
  2. Added proper handling of horizontal scrolling based on the toggle
  3. Long-press opens the context menus

Changes made:

  1. Added moveContainer method to handle container scrolling separately from block movement
  2. Modified touch event handlers to properly track two-finger scroll positions
  3. Updated moveBlock and moveBlockRelative and to skip block movement during two-finger scroll

Testing:

  1. Two-finger scrolling with horizontal scrolling enabled and disabled
  2. Two-finger scrolling over blocks vs empty canvas areas
  3. Proper return to normal block interaction after scrolling ends

Video:

IMG_4595.mp4

Related Issue:

Fixes #4096 - Two-finger scrolling touch functionality

@jivansh77
Copy link
Contributor Author

@walterbender Should I also incorporate the zoom in/out using 2 fingers in this PR?

@jivansh77
Copy link
Contributor Author

I am working on the long press to open context menu as well.

@walterbender
Copy link
Member

This works well. But perhaps too well. If you long press on a block, you also get the canvas menu. We need to trigger the block menu instead. (This is related to but not the same as #3986)

@jivansh77
Copy link
Contributor Author

@walterbender Yes I am facing a similar issue, long-pressing on a block is causing an overlap in advanced mode,

IMG_4613.2.mp4

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 31, 2024

On my MacBook as well when right clicking the helpfulwheel appears for some time and this happens in the master branch

Screen.Recording.2024-12-31.at.6.57.40.PM.mov

@jivansh77
Copy link
Contributor Author

@walterbender now right clicking/long-pressing for some time does not cause an overlap and block menu appears after the helpfulwheel but I'm trying to make it so that it does not appear at all.

@walterbender
Copy link
Member

what does closest(."block") do?

@jivansh77
Copy link
Contributor Author

Before and after,

Screen.Recording.2025-01-01.at.8.42.03.PM.mov

@jivansh77
Copy link
Contributor Author

@walterbender I added the closest() method to check if the target is a block so that I can prevent the helpful wheel from appearing but it is still an issue, I will remove that

@jivansh77
Copy link
Contributor Author

The touch devices also dont have the overlap now

IMG_4619.mov

@walterbender
Copy link
Member

On my system (Firefox) I can no longer get the block menu, only the canvas menu (even when I long touch on a block).

@walterbender
Copy link
Member

But right click on a block still gives me both menus.

@jivansh77
Copy link
Contributor Author

@walterbender I think u will have to hold the right click for some time for the block menu to open bcos that is triggering a long press, or just simply right click it.
I’m actually a little confused as to what is the expected behavior if u could just elaborate for both touch and mouse

@walterbender
Copy link
Member

An even longer press does not bring up the block menu.

@jivansh77
Copy link
Contributor Author

jivansh77 commented Jan 2, 2025

@walterbender The coordinate checking logic from #4219 seems to be working for now, I have used the logic for touch also

IMG_4623.mov

@jivansh77 jivansh77 marked this pull request as ready for review January 2, 2025 08:20
@jivansh77
Copy link
Contributor Author

@walterbender I have used the function in blocks.js so long press opens context menus on touch devices while right click is for non-touch devices, pls review

@jivansh77
Copy link
Contributor Author

jivansh77 commented Jan 5, 2025

@walterbender also initially while clicking on the menus on touch devices the blocks would also get triggered so I have added stoppropagation to prevent this from happening. It now works fine

@walterbender
Copy link
Member

Maybe it is just my env. but when I use two-finger touch, it zooms as well as scrolls. We really need to disable the zooming.
Also, long press is still not working for me.

@jivansh77
Copy link
Contributor Author

@walterbender If I am not wrong you can neither see the block menu nor the helpfulwheel right, only the canvas menu. And you have tested using Firefox?

@jivansh77
Copy link
Contributor Author

jivansh77 commented Jan 5, 2025

@walterbender Pls test now if it is still zooming when you scroll bcos I am not able to reproduce this. According to me, zooming is only happening using ctrl + scroll, and pinching on the trackpad, but when using two fingers to scroll on my touch device, it is not zooming

@walterbender
Copy link
Member

walterbender commented Jan 5, 2025

It is better than it was -- long press will bring up the helpful menu.
(zoom is fixed now).

The block help menu is not appearing, however.

@walterbender
Copy link
Member

Also, it would be good to be able to drag a block or stack.

@jivansh77
Copy link
Contributor Author

jivansh77 commented Jan 5, 2025

@walterbender Please check if the block menu is now appearing. Not sure if this fixes the issue, but hold the press for a little longer than you would with the helpfulwheel, block menu should appear

@walterbender
Copy link
Member

Not working for me. Sometimes I get the canvas menu. Sometimes I get nothing. I would think the updated logic from #4224 would help.

@jivansh77
Copy link
Contributor Author

Ok, I will look into it. To confirm you are using a firefox environment right?

@walterbender
Copy link
Member

Yes. I have been testing with Firefox on Fedora.

@jivansh77
Copy link
Contributor Author

jivansh77 commented Jan 6, 2025

@walterbender Pls test it now, most probably this should work

@walterbender
Copy link
Member

Everything works except for events associated with blocks. I cannot drag a block or long-press on a block. The standard block piemenus do work (e.g., the pitch menu).

@jivansh77
Copy link
Contributor Author

jivansh77 commented Jan 8, 2025

@walterbender Not sure if this causes it to work, but for me the dragging of the blocks and the block menu everything is working fine, not sure but I think ur env is causing a mix up between the touch and the mouse events

@walterbender
Copy link
Member

I still cannot long press or drag individual blocks. Everything else seems to be working. Perhaps @pikurasa can test?

@pikurasa
Copy link
Collaborator

pikurasa commented Jan 8, 2025

I still cannot long press or drag individual blocks. Everything else seems to be working. Perhaps @pikurasa can test?

Ok. I'm testing this branch on an X-1 laptop (with touchscreen). I'll let you know.

I won't easily be able to test on mobile, which works a bit differently in my experience.

@pikurasa
Copy link
Collaborator

pikurasa commented Jan 8, 2025

I tested this branch on an X-1 with touch screen, running Trisquel (Debian) and Chromium.

The one good thing about this patch is that accidental zooming in and out when using two fingers seems to be fixed. However, other scrolling doesn't seem to be as performative as it is on master. Also, the main issue that blocks are not draggable on master seems not to be fixed on this PR.

While I was doing this, I tried master (via sugarlabs.github.io/music blocks ) on Android (Pixel 9, running Calyx), and it seems to work pretty well. Here's a video where I test various touch functionality, from scrolling the canvas (two finger), to moving blocks (one finger), to moving the mouse glyph (two finger), to clicking on menus, to testing Snake Maker 5000 (touch for an interactive program within MB). All these things worked well enough. My biggest concern would be discover ability: i.e. how would a user find some of that functionality?

@pikurasa
Copy link
Collaborator

pikurasa commented Jan 8, 2025

Here's the video: https://youtu.be/J7-v7CIDO1k

@pikurasa
Copy link
Collaborator

pikurasa commented Jan 8, 2025

Here's a video of a test of master (sugarlabs.github.io/musicblocks/) on an X-1 laptop with touchscreen for comparison: https://youtu.be/1CZPYjzl7ic

@jivansh77
Copy link
Contributor Author

jivansh77 commented Jan 8, 2025

@walterbender @pikurasa After some logging, I have found out that on laptops with touch screens, a touch is triggering the mousedown event only, I want it to trigger the touchStart event because that is what handles all the touch functionality, this is not the case for mobile devices/tablets which seem to be working

@jivansh77
Copy link
Contributor Author

I will still work on this to make the scrolling smoother and fix the touch events

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

Successfully merging this pull request may close these issues.

Touch functionality not working as expected
3 participants