Skip to content

Commit

Permalink
fix(drag-drop): disabled state not synced on data binding changes
Browse files Browse the repository at this point in the history
Usually we only sync up the drop list ref with the directive right before dragging starts, however if the disabled state is set, it can propagate to the child directives and prevent the user from ever starting a search again. These changes make it so whenever the disable state is set it'll get sync immediately.

Also changes a misleading method name.

Fixes angular#17325.
  • Loading branch information
crisbeto committed Oct 8, 2019
1 parent 45a4300 commit fb8d4dc
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 4 deletions.
33 changes: 32 additions & 1 deletion src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3299,6 +3299,36 @@ describe('CdkDrag', () => {
cleanup();
}));

it('should be able to re-enable a disabled drop list', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
const dragItems = fixture.componentInstance.dragItems;
const tryDrag = () => {
const firstItem = dragItems.first;
const thirdItemRect = dragItems.toArray()[2].element.nativeElement.getBoundingClientRect();
dragElementViaMouse(fixture, firstItem.element.nativeElement,
thirdItemRect.left + 1, thirdItemRect.top + 1);
flush();
fixture.detectChanges();
};

expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim()))
.toEqual(['Zero', 'One', 'Two', 'Three']);

fixture.componentInstance.dropInstance.disabled = true;
fixture.detectChanges();
tryDrag();

expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim()))
.toEqual(['Zero', 'One', 'Two', 'Three']);

fixture.componentInstance.dropInstance.disabled = false;
fixture.detectChanges();
tryDrag();

expect(dragItems.map(drag => drag.element.nativeElement.textContent!.trim()))
.toEqual(['One', 'Two', 'Zero', 'Three']);
}));
});

describe('in a connected drop container', () => {
Expand Down Expand Up @@ -4379,7 +4409,8 @@ const DROP_ZONE_FIXTURE_TEMPLATE = `
[id]="dropZoneId"
[cdkDropListData]="items"
(cdkDropListSorted)="sortedSpy($event)"
(cdkDropListDropped)="droppedSpy($event)">
(cdkDropListDropped)="droppedSpy($event)"
[cdkDropListDisabled]="listDisabled">
<div
*ngFor="let item of items"
cdkDrag
Expand Down
10 changes: 7 additions & 3 deletions src/cdk/drag-drop/directives/drop-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,11 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
return this._disabled || (!!this._group && this._group.disabled);
}
set disabled(value: boolean) {
this._disabled = coerceBooleanProperty(value);
// Usually we sync the directive and ref state right before dragging starts, in order to have
// a single point of failure and to avoid having to use setters for everything. `disabled` is
// a special case, because it can prevent the `beforeStarted` event from firing, which can lock
// the user in a disabled state, so we also need to sync it as it's being set.
this._dropListRef.disabled = this._disabled = coerceBooleanProperty(value);
}
private _disabled = false;

Expand Down Expand Up @@ -159,7 +163,7 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
return this.enterPredicate(drag.data, drop.data);
};

this._syncInputs(this._dropListRef);
this._setupInputSyncSubscription(this._dropListRef);
this._handleEvents(this._dropListRef);
CdkDropList._dropLists.push(this);

Expand Down Expand Up @@ -251,7 +255,7 @@ export class CdkDropList<T = any> implements AfterContentInit, OnDestroy {
}

/** Syncs the inputs of the CdkDropList with the options of the underlying DropListRef. */
private _syncInputs(ref: DropListRef<CdkDropList>) {
private _setupInputSyncSubscription(ref: DropListRef<CdkDropList>) {
if (this._dir) {
this._dir.change
.pipe(startWith(this._dir.value), takeUntil(this._destroyed))
Expand Down

0 comments on commit fb8d4dc

Please sign in to comment.