katoid / angular-grid-layout

Responsive grid with draggable and resizable items for Angular applications.
https://katoid.github.io/angular-grid-layout
MIT License
436 stars 60 forks source link

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

Closed ray-kay closed 1 month ago

ray-kay commented 1 year ago

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 commented 1 year ago

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!

ray-kay commented 1 year ago

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

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 commented 1 year ago

Thanks @ray-kay !

I have created a Pull Request that just replaces mouse and touch events for pointer events: https://github.com/katoid/angular-grid-layout/pull/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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 4 months ago

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 commented 4 months ago

@Duckelekuuk This fix will be published alongside the Angular Grid Layout update to Angular version 16 https://github.com/katoid/angular-grid-layout/issues/101#issuecomment-1962622220