Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
11 changes: 11 additions & 0 deletions src/lib/core/overlay/overlay-directives.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,25 @@ describe('Overlay directives', () => {
fixture.detectChanges();
});

/** Returns the current open overlay pane element. */
function getPaneElement() {
Copy link
Member

Choose a reason for hiding this comment

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

It may be better to use querySelector here, otherwise the test will break if we changed the DOM order.

Copy link
Member

Choose a reason for hiding this comment

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

As long as there's a unit test for the behavior it should be okay

Copy link
Member Author

@devversion devversion Jan 31, 2017

Choose a reason for hiding this comment

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

Yeah it shouldn't be necessary. Nevertheless I just changed it (Testing the dismiss review button)

return overlayContainerElement.firstElementChild as HTMLElement;
}

it(`should attach the overlay based on the open property`, () => {
fixture.componentInstance.isOpen = true;
fixture.detectChanges();

expect(overlayContainerElement.textContent).toContain('Menu content');
expect(getPaneElement().style.pointerEvents)
.toBeFalsy('Expected the overlay pane to enable pointerEvents when attached.');

fixture.componentInstance.isOpen = false;
fixture.detectChanges();

expect(overlayContainerElement.textContent).toBe('');
expect(getPaneElement().style.pointerEvents)
.toBe('none', 'Expected the overlay pane to disable pointerEvents when detached.');
});

it('should destroy the overlay when the directive is destroyed', () => {
Expand All @@ -47,6 +56,8 @@ describe('Overlay directives', () => {
fixture.destroy();

expect(overlayContainerElement.textContent.trim()).toBe('');
expect(getPaneElement())
.toBeFalsy('Expected the overlay pane element to be removed when disposed.');
});

it('should use a connected position strategy with a default set of positions', () => {
Expand Down
16 changes: 16 additions & 0 deletions src/lib/core/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,15 @@ export class OverlayRef implements PortalHost {
}

let attachResult = this._portalHost.attach(portal);

// Update the pane element with the given state configuration.
this.updateSize();
this.updateDirection();
this.updatePosition();

// Enable pointer events for the overlay pane element.
this._togglePointerEvents(true);
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these enabled by default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah they are enabled by default. But developers can always call attach after detach has been called. This is necessary to revert the togglePointerEvents(false) from the detach method.


return attachResult;
}

Expand All @@ -48,6 +53,12 @@ export class OverlayRef implements PortalHost {
*/
detach(): Promise<any> {
this._detachBackdrop();

// When the overlay is detached, the pane element should disable pointer events.
// This is necessary because otherwise the pane element will cover the page and disable
// pointer events therefore. Depends on the position strategy and the applied pane boundaries.
this._togglePointerEvents(false);

return this._portalHost.detach();
}

Expand Down Expand Up @@ -115,6 +126,11 @@ export class OverlayRef implements PortalHost {
}
}

/** Toggles the pointer events for the overlay pane element. */
private _togglePointerEvents(enablePointer: boolean) {
this._pane.style.pointerEvents = enablePointer ? null : 'none';
}

/** Attaches a backdrop for this overlay. */
private _attachBackdrop() {
this._backdropElement = document.createElement('div');
Expand Down
17 changes: 17 additions & 0 deletions src/lib/core/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,23 @@ describe('Overlay', () => {
expect(overlayContainerElement.textContent).toBe('');
});

it('should disable pointer events of the pane element if detached', () => {
let overlayRef = overlay.create();
let paneElement = overlayRef.overlayElement;

overlayRef.attach(componentPortal);

expect(paneElement.childNodes.length).not.toBe(0);
expect(paneElement.style.pointerEvents)
.toBeFalsy('Expected the overlay pane to enable pointerEvents when attached.');

overlayRef.detach();

expect(paneElement.childNodes.length).toBe(0);
expect(paneElement.style.pointerEvents)
.toBe('none', 'Expected the overlay pane to disable pointerEvents when detached.');
});

it('should open multiple overlays', () => {
let pizzaOverlayRef = overlay.create();
pizzaOverlayRef.attach(componentPortal);
Expand Down
5 changes: 4 additions & 1 deletion src/lib/core/portal/dom-portal-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,14 @@ export class DomPortalHost extends BasePortalHost {
let viewContainer = portal.viewContainerRef;
let viewRef = viewContainer.createEmbeddedView(portal.templateRef);

// The method `createEmbeddedView` will add the view as a child of the viewContainer.
// But for the DomPortalHost the view can be added everywhere in the DOM (e.g Overlay Container)
// To move the view to the specified host element. We just re-append the existing root nodes.
viewRef.rootNodes.forEach(rootNode => this._hostDomElement.appendChild(rootNode));

this.setDisposeFn((() => {
let index = viewContainer.indexOf(viewRef);
if (index != -1) {
if (index !== -1) {
viewContainer.remove(index);
}
}));
Expand Down