Skip to content

Commit be56a67

Browse files
committed
fix(overlay): hide pane element if detached
* Currently the overlay element is still visible if the overlay has been detached. This is problematic if the overlay pane element has an `height | wídth` set from the state, because now the overlay element may block clicks on the page and overlap the actual content. * The issue isn't noticable for the `select` and other overlay components, because those don't specify a `height | width` in their state. Nevertheless the pane elements are still visible, but just have no height and width set. Fixes #2739
1 parent 592f33f commit be56a67

File tree

4 files changed

+48
-1
lines changed

4 files changed

+48
-1
lines changed

src/lib/core/overlay/overlay-directives.spec.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,25 @@ describe('Overlay directives', () => {
2929
fixture.detectChanges();
3030
});
3131

32+
/** Returns the current open overlay pane element. */
33+
function getPaneElement() {
34+
return overlayContainerElement.firstElementChild as HTMLElement;
35+
}
36+
3237
it(`should attach the overlay based on the open property`, () => {
3338
fixture.componentInstance.isOpen = true;
3439
fixture.detectChanges();
3540

3641
expect(overlayContainerElement.textContent).toContain('Menu content');
42+
expect(getPaneElement().style.visibility)
43+
.toBeFalsy('Expected the overlay pane to be visible when attached.');
3744

3845
fixture.componentInstance.isOpen = false;
3946
fixture.detectChanges();
4047

4148
expect(overlayContainerElement.textContent).toBe('');
49+
expect(getPaneElement().style.visibility)
50+
.toBe('hidden', 'Expected the overlay pane to be hidden when detached.');
4251
});
4352

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

4958
expect(overlayContainerElement.textContent.trim()).toBe('');
59+
expect(getPaneElement())
60+
.toBeFalsy('Expected the overlay pane element to be removed when disposed.');
5061
});
5162

5263
it('should use a connected position strategy with a default set of positions', () => {

src/lib/core/overlay/overlay-ref.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,15 @@ export class OverlayRef implements PortalHost {
3535
}
3636

3737
let attachResult = this._portalHost.attach(portal);
38+
39+
// Update the pane element with the given state configuration.
3840
this.updateSize();
3941
this.updateDirection();
4042
this.updatePosition();
4143

44+
// Make the pane element visible when overlay is attached.
45+
this._toggleVisibility(true);
46+
4247
return attachResult;
4348
}
4449

@@ -48,6 +53,12 @@ export class OverlayRef implements PortalHost {
4853
*/
4954
detach(): Promise<any> {
5055
this._detachBackdrop();
56+
57+
// When the overlay is detached, the pane element should be invisible.
58+
// This is necessary because otherwise the pane element can cover the page and disable
59+
// pointer events. Depends on the position strategy and the applied pane boundaries.
60+
this._toggleVisibility(false);
61+
5162
return this._portalHost.detach();
5263
}
5364

@@ -115,6 +126,11 @@ export class OverlayRef implements PortalHost {
115126
}
116127
}
117128

129+
/** Toggles the visibility of the overlay pane element. */
130+
private _toggleVisibility(isVisible: boolean) {
131+
this._pane.style.visibility = isVisible ? null : 'hidden';
132+
}
133+
118134
/** Attaches a backdrop for this overlay. */
119135
private _attachBackdrop() {
120136
this._backdropElement = document.createElement('div');

src/lib/core/overlay/overlay.spec.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,23 @@ describe('Overlay', () => {
6262
expect(overlayContainerElement.textContent).toBe('');
6363
});
6464

65+
it('should hide the pane element if the overlay is detached', () => {
66+
let overlayRef = overlay.create();
67+
let paneElement = overlayRef.overlayElement;
68+
69+
overlayRef.attach(componentPortal);
70+
71+
expect(paneElement.childNodes.length).not.toBe(0);
72+
expect(paneElement.style.visibility)
73+
.toBeFalsy('Expected the overlay pane to be visible when attached.');
74+
75+
overlayRef.detach();
76+
77+
expect(paneElement.childNodes.length).toBe(0);
78+
expect(paneElement.style.visibility)
79+
.toBe('hidden', 'Expected the overlay pane to be hidden when detached.');
80+
});
81+
6582
it('should open multiple overlays', () => {
6683
let pizzaOverlayRef = overlay.create();
6784
pizzaOverlayRef.attach(componentPortal);

src/lib/core/portal/dom-portal-host.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,14 @@ export class DomPortalHost extends BasePortalHost {
9393
let viewContainer = portal.viewContainerRef;
9494
let viewRef = viewContainer.createEmbeddedView(portal.templateRef);
9595

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

98101
this.setDisposeFn((() => {
99102
let index = viewContainer.indexOf(viewRef);
100-
if (index != -1) {
103+
if (index !== -1) {
101104
viewContainer.remove(index);
102105
}
103106
}));

0 commit comments

Comments
 (0)