Skip to content

fix(cdk/drag-drop): don't disable native dragging on inactive handles #20991

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,37 @@ describe('CdkDrag', () => {
expect(styles.touchAction || (styles as any).webkitUserDrag).toBe('none');
});

it('should enable native drag interactions on the drag handle if dragging is disabled', () => {
const fixture = createComponent(StandaloneDraggableWithHandle);
fixture.detectChanges();
fixture.componentInstance.draggingDisabled = true;
fixture.detectChanges();
const styles = fixture.componentInstance.handleElement.nativeElement.style;
expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy();
});

it('should enable native drag interactions on the drag handle if dragging is disabled ' +
'on init', () => {
const fixture = createComponent(StandaloneDraggableWithHandle);
fixture.componentInstance.draggingDisabled = true;
fixture.detectChanges();
const styles = fixture.componentInstance.handleElement.nativeElement.style;
expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy();
});

it('should toggle native drag interactions based on whether the handle is disabled', () => {
const fixture = createComponent(StandaloneDraggableWithHandle);
fixture.detectChanges();
fixture.componentInstance.handleInstance.disabled = true;
fixture.detectChanges();
const styles = fixture.componentInstance.handleElement.nativeElement.style;
expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy();

fixture.componentInstance.handleInstance.disabled = false;
fixture.detectChanges();
expect(styles.touchAction || (styles as any).webkitUserDrag).toBe('none');
});

it('should be able to reset a freely-dragged item to its initial position', fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();
Expand Down Expand Up @@ -5558,7 +5589,7 @@ class StandaloneDraggableSvgWithViewBox {

@Component({
template: `
<div #dragElement cdkDrag
<div #dragElement cdkDrag [cdkDragDisabled]="draggingDisabled"
style="width: 100px; height: 100px; background: red; position: relative">
<div #handleElement cdkDragHandle style="width: 10px; height: 10px; background: green;"></div>
</div>
Expand All @@ -5569,6 +5600,7 @@ class StandaloneDraggableWithHandle {
@ViewChild('handleElement') handleElement: ElementRef<HTMLElement>;
@ViewChild(CdkDrag) dragInstance: CdkDrag;
@ViewChild(CdkDragHandle) handleInstance: CdkDragHandle;
draggingDisabled = false;
}

@Component({
Expand Down
11 changes: 8 additions & 3 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ export class DragRef<T = any> {
if (newValue !== this._disabled) {
this._disabled = newValue;
this._toggleNativeDragInteractions();
this._handles.forEach(handle => toggleNativeDragInteractions(handle, newValue));
}
}
private _disabled = false;
Expand Down Expand Up @@ -342,7 +343,7 @@ export class DragRef<T = any> {
/** Registers the handles that can be used to drag the element. */
withHandles(handles: (HTMLElement | ElementRef<HTMLElement>)[]): this {
this._handles = handles.map(handle => coerceElement(handle));
this._handles.forEach(handle => toggleNativeDragInteractions(handle, false));
this._handles.forEach(handle => toggleNativeDragInteractions(handle, this.disabled));
this._toggleNativeDragInteractions();
return this;
}
Expand Down Expand Up @@ -458,8 +459,9 @@ export class DragRef<T = any> {
* @param handle Handle element that should be disabled.
*/
disableHandle(handle: HTMLElement) {
if (this._handles.indexOf(handle) > -1) {
if (!this._disabledHandles.has(handle) && this._handles.indexOf(handle) > -1) {
this._disabledHandles.add(handle);
toggleNativeDragInteractions(handle, true);
}
}

Expand All @@ -468,7 +470,10 @@ export class DragRef<T = any> {
* @param handle Handle element to be enabled.
*/
enableHandle(handle: HTMLElement) {
this._disabledHandles.delete(handle);
if (this._disabledHandles.has(handle)) {
this._disabledHandles.delete(handle);
toggleNativeDragInteractions(handle, this.disabled);
Copy link
Contributor

Choose a reason for hiding this comment

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

should this say false instead of this.disabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the handle should stay disabled if it is enabled while the drag item is disabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then should it also use this.disabled in disableHandle? seems odd that the methods aren't mirrored

Copy link
Member Author

Choose a reason for hiding this comment

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

No since disableHandle only disables the handle, not the entire draggable item. A handle is considered "inactive" if either it is disabled or the item is disabled. This might be slightly confusing since toggleNativeDragInteractions(true) means that interactions should be enabled which is true when dragging is disabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I think I understand it, just a little confusing at first sight

}
}

/** Sets the layout direction of the draggable item. */
Expand Down