Skip to content

Commit

Permalink
fix(global-position-strategy): error if disposed before applied
Browse files Browse the repository at this point in the history
Fixes an error that is thrown by the `GlobalPositionStrategy` if its connected `OverlayRef` is disposed before the strategy is applied. This could happen, because the `OverlayRef` applies the position strategy asynchronously.

Fixes angular#8758.
  • Loading branch information
crisbeto committed Dec 1, 2017
1 parent 53c94c7 commit b83fe9e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/cdk/overlay/position/global-position-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {OverlayModule, Overlay, OverlayRef, GlobalPositionStrategy} from '../ind
describe('GlobalPositonStrategy', () => {
let element: HTMLElement;
let strategy: GlobalPositionStrategy;
let hasOverlayAttached: boolean;

beforeEach(() => {
TestBed.configureTestingModule({imports: [OverlayModule]});
Expand All @@ -15,11 +16,18 @@ describe('GlobalPositonStrategy', () => {

element = document.createElement('div');
document.body.appendChild(element);
strategy.attach({overlayElement: element} as OverlayRef);
hasOverlayAttached = true;
strategy.attach({
overlayElement: element,
hasAttached: () => hasOverlayAttached
} as OverlayRef);
});

afterEach(() => {
element.parentNode!.removeChild(element);
if (element.parentNode) {
element.parentNode.removeChild(element);
}

strategy.dispose();
});

Expand Down Expand Up @@ -151,4 +159,12 @@ describe('GlobalPositonStrategy', () => {
expect(element.style.marginTop).toBe('0px');
expect((element.parentNode as HTMLElement).style.alignItems).toBe('flex-start');
});

it('should not throw when attempting to apply after the overlay has been disposed', () => {
strategy.dispose();
element.parentNode!.removeChild(element);
hasOverlayAttached = false;

expect(() => strategy.apply()).not.toThrow();
});
});
7 changes: 7 additions & 0 deletions src/cdk/overlay/position/global-position-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,13 @@ export class GlobalPositionStrategy implements PositionStrategy {
* @returns Resolved when the styles have been applied.
*/
apply(): void {
// Since the overlay ref applies the strategy asynchronously, it could
// have been disposed before it ends up being applied. If that is the
// case, we shouldn't do anything.
if (!this._overlayRef.hasAttached()) {
return;
}

const element = this._overlayRef.overlayElement;

if (!this._wrapper && element.parentNode) {
Expand Down

0 comments on commit b83fe9e

Please sign in to comment.