Skip to content

Commit

Permalink
fix(draggable): disable pointer events on the element when dragging
Browse files Browse the repository at this point in the history
  • Loading branch information
Matt Lewis committed Dec 9, 2016
1 parent baef877 commit f29b424
Showing 1 changed file with 7 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/draggable.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ export class Draggable implements OnInit, OnDestroy {

this.dragStart.next({x: 0, y: 0});

if (this.ghostDragEnabled) {
this.renderer.setElementStyle(this.element.nativeElement, 'pointerEvents', 'none');
}

const currentDrag: Subject<any> = new Subject();

this.draggableHelper.currentDrag.next(currentDrag);
Expand Down Expand Up @@ -93,6 +97,9 @@ export class Draggable implements OnInit, OnDestroy {
this.dragEnd.next({x, y});
currentDrag.complete();
this.setCssTransform('');
if (this.ghostDragEnabled) {
this.renderer.setElementStyle(this.element.nativeElement, 'pointerEvents', 'auto');
}
});

return mouseMove;
Expand Down

6 comments on commit f29b424

@dwknippers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this added? It seems weird to change the cursor while dragging

@mattlewis92
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't change the cursor, it suppresses click events from interfering with the element while it's dragged: https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events

@dwknippers
Copy link
Contributor

@dwknippers dwknippers commented on f29b424 May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thank you for the clarification!

In the documention you linked, it states
"in addition to indicating that the element is not the target of mouse events, the value none instructs the mouse event to go "through" the element and target whatever is "underneath" that element instead.".

This means that the cursor property of the actual element is then ignored, and the cursor reverts to its default value. This doesn't make for such nice dragging.

What was the problem leading to this change?

@mattlewis92
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine it was to prevent the native browser click event from firing when the user presses down on the item. The logic was copied from here: https://github.com/mattlewis92/angular-bootstrap-calendar/blob/master/src/directives/mwlDraggable.js#L41

@dwknippers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm it would be quite interesting how that would happen. It seems like an edge case, I'm unable to reproduce it so far... maybe it would be better as to remove it, as the user experience would probably be better in my opinion. But hey, maybe I'm overseeing the necessity :)

@dwknippers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some digging, it was in the older code since even the first commit, thus probably wasn't linked to any issues. Only change didn't change it, but can't see issue related since the issue tracker is down.
No noticeable impact so far, OK for PR to remove it?

Please sign in to comment.