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

Drag & Drop, resize doesn't work on Touchscreen Laptop #80

Open
ray-kay opened this issue Apr 20, 2023 · 9 comments · May be fixed by #90
Open

Drag & Drop, resize doesn't work on Touchscreen Laptop #80

ray-kay opened this issue Apr 20, 2023 · 9 comments · May be fixed by #90
Labels
bug Something isn't working enhancement Some improvement (performance or just code structure)

Comments

@ray-kay
Copy link

ray-kay commented Apr 20, 2023

Is drag & drop and resize suppose to work on a touchscreen laptop?
E.g. https://katoid.github.io/angular-grid-layout/playground and https://katoid.github.io/angular-grid-layout/real-life-example doesn't work for me. It moves the whole page when trying to drag an item.

Many thanks

@llorenspujol
Copy link
Contributor

Finally! I've been waiting 2 years for someone to open this issue 😆...

You are right, It does NOT work on touchscreen laptops right now. Here is the code that runs nowadays:

/**
 * Emits when a mousedown or touchstart emits. Avoids conflicts between both events.
 * @param element, html element where to  listen the events.
 * @param touchNumber number of the touch to track the event, default to the first one.
 */
export function ktdMouseOrTouchDown(element, touchNumber = 1): Observable<MouseEvent | TouchEvent> {
    return iif(
        () => ktdIsMobileOrTablet(),
        fromEvent<TouchEvent>(element, 'touchstart', passiveEventListenerOptions as AddEventListenerOptions).pipe(
            filter((touchEvent) => touchEvent.touches.length === touchNumber)
        ),
        fromEvent<MouseEvent>(element, 'mousedown', activeEventListenerOptions as AddEventListenerOptions).pipe(
            filter((mouseEvent: MouseEvent) => {
                /**
                 * 0 : Left mouse button
                 * 1 : Wheel button or middle button (if present)
                 * 2 : Right mouse button
                 */
                return mouseEvent.button === 0; // Mouse down to be only fired if is left click
            })
        )
    );
}

If the device is mobile or tablet, I listen only to touch events, and if not, I listen to mouse events. Obviously, this does not work for hybrid devices with mouse and touchscreens.

Solving this issue is not as easy as listen to both events sources (touch and mouse), since there are mouse emulation events that are fired after the touch events in many mobile environments. We should filter those synthetic events out, with that, the issue is solved. But it gets quite tricky... check this issue (Note that we don't want to lose the scroll on touch).

I will try this solution and see how it works. If not solid enough, I would suggest to ignore mouse events for some time after a touchstart event.

I you want to do this issue you are welcome to do it. You have the information to solve it. If not, I will fix it in the near future.

Thanks for reporting!

@llorenspujol llorenspujol added bug Something isn't working enhancement Some improvement (performance or just code structure) labels Apr 21, 2023
@ray-kay
Copy link
Author

ray-kay commented May 5, 2023

Hi @llorenspujol
thanks for your feedback.

I found a quite simple solution (I think).
As I don't have the permission to create a new branch and pr I am attaching the git patch file here:

TL:DR

  • replace 'mouse...' event listeners with 'pointer...' (supported by all major browsers)
  • add touch-action: none; to grid-item host styling (supported by all major browsers)

angular-grid-layout-6732a93-enable drag and resize when using touch on hybrid displays.patch

Let me know what you think.
I tested the demo app on my HP Elitebook (Touch) and both (mouse and touchscreen) are working fine (incl. scrolling).

@llorenspujol
Copy link
Contributor

Thanks @ray-kay !

I have created a Pull Request that just replaces mouse and touch events for pointer events: #90

If you could try it and provide feedback, it would be appreciated. It should be tested on multiple devices before merging to ensure there are no drawbacks present.

I have not added the CSS touch-action: none; for now. It appears to be working well without it. If it is eventually needed, it will have to be removed in cases where custom drag handles are used; otherwise, scrolling will not work when touching that grid item. So, for now, let's try it without and see how it goes.

@ray-kay
Copy link
Author

ray-kay commented May 30, 2023

Hi @llorenspujol,
thanks for providing a PR.
I had a look and it doesn't work at all on my laptop touch screen. You don't a have a hybrid screen to test on, do you?
Did you see the changes in my patch file?
What is missing is the renaming all 'mouse...' events to 'pointer...' e.g. 'mousedown' becomes 'pointerdown'.
Also touch-action: none; to grid-item is essential otherwise the page starts scrolling immediately after dragging starts on the grid item.
Thanks

@llorenspujol
Copy link
Contributor

Hey @ray-kay !

Thanks for checking it!

Yes you're right, there were some issues in my last commit. Now is fixed and should work (all pointer events and touch-action: none). I haves tested it on an hybrid screen an seems to work fine.

Let me know if it is working on your laptop 👌

@ray-kay
Copy link
Author

ray-kay commented Jul 4, 2023

Hey @llorenspujol,
sorry for the delay and thanks working in those changes.
I tested it on my hybrid laptop and both - touch and mouse are working fine 👍

The only thing I wonder is that there are still some 'mouse...' events in code instead of 'pointer...' .

image

If thats intentional and ok for you then please provide thes changes in a new version.
Many thanks 🙌

@llorenspujol
Copy link
Contributor

Thanks for testing @ray-kay !

Yes, old code using mouse and touch events is still used if pointer events are not supported:

if (!ktdSupportsPointerEvents()) {
     // Old mouse and touch events code
} else {
    // New pointer events code
}

Even though pointer events are widely supported across all browsers, for now this is the safest change. To safely remove the old code I need more time to investigate and test it.

Preserving the old code is also important because there might be cases where pointer events don't perform as expected. If that happens, I can add a flag to switch back to the old behavior until it's fixed.

In relation to that, before merging this branch, I'll need to test it on more devices to ensure everything works as expected. This is a core change to the drag functionality, so it needs caution before merging.

But I hope soon it could be done!!

@Duckelekuuk
Copy link

Duckelekuuk commented Feb 21, 2024

Is there any update on this? I tested out the POC and the change works great and I would like to use this fix in the new version of our application.

Devices tested on:
Acer chromebook R11 - Chrome
HP probook - FireFox/Chrome
Dell - Chrome

@llorenspujol
Copy link
Contributor

@Duckelekuuk This fix will be published alongside the Angular Grid Layout update to Angular version 16 #101 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement Some improvement (performance or just code structure)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants