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

Hammer.js overrides native drag&drop #1457

Closed
taffeldt opened this issue Oct 9, 2016 · 12 comments
Closed

Hammer.js overrides native drag&drop #1457

taffeldt opened this issue Oct 9, 2016 · 12 comments
Assignees
Labels
P2 The issue is important to a large percentage of users, with a workaround

Comments

@taffeldt
Copy link
Contributor

taffeldt commented Oct 9, 2016

Bug, feature request, or proposal:

Probably a bug. Might be a proposal. Not entirely sure.

What is the expected behavior?

You should be able to use native HTML5 drag&drop, at least on non-Material elements.

What is the current behavior?

The dragstart event gets overriden by Hammer.js, so you can't access required event properties like dataTransfer.

What are the steps to reproduce?

With Hammer.js installed, bind the dragstart event to a function and try to access dataTransfer. It will be undefined.
Alternatively uninstall Hammer.js and do the dame steps. You will get the error message: "Hammer.js is not loaded, can not bind dragstart event".
For the first case I set up a plunker right here: http://plnkr.co/edit/CMzTNi0HKVxI9xI3ztsc?p=preview Make sure to check console output once you start dragging the first div.

What is the use-case or motivation for changing an existing behavior?

Angular Material 2 should not prevent you from using native APIs on other tags.

Which versions of Angular, Material, OS, browsers are affected?

Tested with Angular 2.0.1, Material 2.0.0-alpha.9-3 and Hammerjs 2.0.8
All browsers should be affected. Tested with Firefox 49.0.1 and Chrome 53.0.2785.143

Is there anything else we should know?

Overriding dragstart should not be necessary during Hammerjs initilisation. The drag event contains all necessary information.
You can read the discussion on Reddit for additional information: https://www.reddit.com/r/Angular2/comments/56kh4g/html5_drag_drop/

@devversion
Copy link
Member

This is because the MdGestureConfig is registering the drag event in Angular's event manager.

It would be probably better to just have the slide event for now, because the drag event is not used by any component yet (only by the gestures demo)

@jimitndiaye
Copy link
Contributor

jimitndiaye commented Oct 11, 2016

@devversion I think the drag event should be sufficient for gestures, if needed. dragstart and dragend* are required for HTML5 drag and drop and should not be overridden.
MdGestureConfig#L11 should probably be modified to exclude them.

@jelbourn
Copy link
Member

I agree that we shouldn't be clobbering the native drag events.

@jelbourn jelbourn added the P2 The issue is important to a large percentage of users, with a workaround label Oct 11, 2016
@mina-skunk
Copy link
Contributor

mina-skunk commented Oct 18, 2016

In the mean time, I found, you can add the following to app.module to revert to native drag events.

@Injectable()
export class AppGestureConfig extends HammerGestureConfig { }

@NgModule({
...
  providers: [{ provide: HAMMER_GESTURE_CONFIG, useClass: AppGestureConfig }],
...
})
export class AppModule {}

@ratscrew
Copy link

#1025 is the same issue

@devversion
Copy link
Member

@jimitndiaye I did not mean to remove the drag events, I just said that we could use the slide recognizer, because it's based on the drag recognizer (just with a smaller threshold) and does not interfere with the native DOM events.

Anyways it seems that dragend and dragstart are now removed in #1458

Personally I would have removed all drag aliases for now, because those are not used by any component yet.

@jelbourn
Copy link
Member

Closed via #1458

@kgiroux
Copy link

kgiroux commented Nov 2, 2016

When this correction will be available ?

Sincerly

@mwamufiya
Copy link

@kgiroux in waiting for this change, I've manually updated the material.umd.js to remove the 2 events in conforming with the change that is pending as shown here merge

not ideal, but hopefully it helps if this is a blocker for you as it was for me.

@kgiroux
Copy link

kgiroux commented Nov 2, 2016

@mwamufiya, I follow the advice from @gatimus. But it is just a temporary fix. It is just for having a date of release. Thank for your quick reply =)

@rajjaiswalsaumya
Copy link

rajjaiswalsaumya commented Nov 14, 2017

Is this released ? Can we drag and drop rows in cdktables ?

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Development

No branches or pull requests

10 participants