From b16031a3a907e54140d168ea0720dbeb1f6e3ff0 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 21 Dec 2016 00:27:47 +0200 Subject: [PATCH] fix(overlay): proper backdrop stacking with multiple overlays (#2276) * fix(overlay): proper backdrop stacking with multiple overlays Currently backdrops get inserted after their corresponding overlays in the DOM. This can lead to situations where another overlay that is technically lower in the stacking order could go above a backdrop (e.g. opening a `select` inside a `dialog`). These changes switch to doing the stacking by having the overlay and backdrop have the same `z-index` and determining the stacking order by the order of the elements in the DOM. Fixes #2272. * Fix wrong selectors after merge with master. --- src/lib/core/overlay/overlay-ref.ts | 8 +++--- src/lib/core/overlay/overlay.spec.ts | 16 +++++++++++ src/lib/core/style/_variables.scss | 3 +- src/lib/menu/menu.spec.ts | 4 +-- src/lib/select/select.spec.ts | 42 ++++++++++++++++++---------- 5 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/lib/core/overlay/overlay-ref.ts b/src/lib/core/overlay/overlay-ref.ts index 6d207b6f1e38..4812a9affa8f 100644 --- a/src/lib/core/overlay/overlay-ref.ts +++ b/src/lib/core/overlay/overlay-ref.ts @@ -101,13 +101,13 @@ export class OverlayRef implements PortalHost { this._backdropElement.classList.add('cdk-overlay-backdrop'); this._backdropElement.classList.add(this._state.backdropClass); - this._pane.parentElement.appendChild(this._backdropElement); + // Insert the backdrop before the pane in the DOM order, + // in order to handle stacked overlays properly. + this._pane.parentElement.insertBefore(this._backdropElement, this._pane); // Forward backdrop clicks such that the consumer of the overlay can perform whatever // action desired when such a click occurs (usually closing the overlay). - this._backdropElement.addEventListener('click', () => { - this._backdropClick.next(null); - }); + this._backdropElement.addEventListener('click', () => this._backdropClick.next(null)); // Add class to fade-in the backdrop after one frame. requestAnimationFrame(() => { diff --git a/src/lib/core/overlay/overlay.spec.ts b/src/lib/core/overlay/overlay.spec.ts index 1cd28e7d0535..4ebbd7245e03 100644 --- a/src/lib/core/overlay/overlay.spec.ts +++ b/src/lib/core/overlay/overlay.spec.ts @@ -235,6 +235,22 @@ describe('Overlay', () => { expect(backdrop.style.pointerEvents).toBe('none'); }); + it('should insert the backdrop before the overlay pane in the DOM order', () => { + let overlayRef = overlay.create(config); + overlayRef.attach(componentPortal); + + viewContainerFixture.detectChanges(); + + let backdrop = overlayContainerElement.querySelector('.cdk-overlay-backdrop'); + let pane = overlayContainerElement.querySelector('.cdk-overlay-pane'); + let children = Array.prototype.slice.call(overlayContainerElement.children); + + expect(children.indexOf(backdrop)).toBeGreaterThan(-1); + expect(children.indexOf(pane)).toBeGreaterThan(-1); + expect(children.indexOf(backdrop)) + .toBeLessThan(children.indexOf(pane), 'Expected backdrop to be before the pane in the DOM'); + }); + }); }); diff --git a/src/lib/core/style/_variables.scss b/src/lib/core/style/_variables.scss index c80127f5cf7f..f577dd853305 100644 --- a/src/lib/core/style/_variables.scss +++ b/src/lib/core/style/_variables.scss @@ -24,12 +24,11 @@ $z-index-drawer: 100 !default; // stacking context for all overlays. $cdk-z-index-overlay-container: 1000; $cdk-z-index-overlay: 1000; -$cdk-z-index-overlay-backdrop: 1; +$cdk-z-index-overlay-backdrop: 1000; // Background color for all of the backdrops $cdk-overlay-dark-backdrop-background: rgba(0, 0, 0, 0.6); - // Global constants $pi: 3.14159265; diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 06184d46335a..bb24ff384152 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -101,7 +101,7 @@ describe('MdMenu', () => { fixture.componentInstance.trigger.openMenu(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0]; + const overlayPane = overlayContainerElement.querySelector('.cdk-overlay-pane'); expect(overlayPane.getAttribute('dir')).toEqual('rtl'); }); @@ -248,7 +248,7 @@ describe('MdMenu', () => { }); function getOverlayPane(): HTMLElement { - let pane = overlayContainerElement.children[0] as HTMLElement; + let pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; pane.style.position = 'absolute'; return pane; } diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index 44935ebd5de3..66db9a6211c6 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -104,7 +104,7 @@ describe('MdSelect', () => { fixture.whenStable().then(() => { trigger.click(); fixture.detectChanges(); - const pane = overlayContainerElement.children[0] as HTMLElement; + const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; expect(pane.style.minWidth).toBe('200px'); }); })); @@ -561,7 +561,7 @@ describe('MdSelect', () => { * @param index The index of the option. */ function checkTriggerAlignedWithOption(index: number): void { - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; // We need to set the position to absolute, because the top/left positioning won't work // since the component CSS isn't included in the tests. @@ -599,7 +599,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; const scrollContainer = overlayPane.querySelector('.md-select-panel'); // The panel should be scrolled to 0 because centering the option is not possible. @@ -616,7 +617,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; const scrollContainer = overlayPane.querySelector('.md-select-panel'); // The panel should be scrolled to 0 because centering the option is not possible. @@ -633,7 +635,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; const scrollContainer = overlayPane.querySelector('.md-select-panel'); // The selected option should be scrolled to the center of the panel. @@ -654,7 +657,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; const scrollContainer = overlayPane.querySelector('.md-select-panel'); // The selected option should be scrolled to the max scroll position. @@ -687,7 +691,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; const scrollContainer = overlayPane.querySelector('.md-select-panel'); // Scroll should adjust by the difference between the top space available (85px + 8px @@ -711,7 +716,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; const scrollContainer = overlayPane.querySelector('.md-select-panel'); // Scroll should adjust by the difference between the bottom space available @@ -736,7 +742,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; // We need to set the position to absolute, because the top/left positioning won't work // since the component CSS isn't included in the tests. @@ -768,7 +775,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; // We need to set the position to absolute, because the top/left positioning won't work // since the component CSS isn't included in the tests. @@ -857,7 +865,8 @@ describe('MdSelect', () => { fixture.detectChanges(); // CSS styles aren't in the tests, so position must be absolute to reflect top/left - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; overlayPane.style.position = 'absolute'; const triggerBottom = trigger.getBoundingClientRect().bottom; @@ -884,7 +893,8 @@ describe('MdSelect', () => { fixture.detectChanges(); // CSS styles aren't in the tests, so position must be absolute to reflect top/left - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; overlayPane.style.position = 'absolute'; const triggerTop = trigger.getBoundingClientRect().top; @@ -906,7 +916,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; // We need to set the position to absolute, because the top/left positioning won't work // since the component CSS isn't included in the tests. @@ -929,7 +940,8 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const overlayPane = overlayContainerElement.children[0] as HTMLElement; + const overlayPane = + overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; // We need to set the position to absolute, because the top/left positioning won't work // since the component CSS isn't included in the tests. @@ -1170,7 +1182,7 @@ describe('MdSelect', () => { trigger.click(); fixture.detectChanges(); - const pane = overlayContainerElement.children[0] as HTMLElement; + const pane = overlayContainerElement.querySelector('.cdk-overlay-pane') as HTMLElement; expect(pane.style.minWidth).toEqual('300px'); expect(fixture.componentInstance.select.panelOpen).toBe(true);