Skip to content

Commit 7114888

Browse files
crisbetocrisbeto
committed
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.
1 parent 2168d7c commit 7114888

File tree

5 files changed

+50
-22
lines changed

5 files changed

+50
-22
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,13 @@ export class OverlayRef implements PortalHost {
101101
this._backdropElement.classList.add('md-overlay-backdrop');
102102
this._backdropElement.classList.add(this._state.backdropClass);
103103

104-
this._pane.parentElement.appendChild(this._backdropElement);
104+
// Insert the backdrop before the pane in the DOM order,
105+
// in order to handle stacked overlays properly.
106+
this._pane.parentElement.insertBefore(this._backdropElement, this._pane);
105107

106108
// Forward backdrop clicks such that the consumer of the overlay can perform whatever
107109
// action desired when such a click occurs (usually closing the overlay).
108-
this._backdropElement.addEventListener('click', () => {
109-
this._backdropClick.next(null);
110-
});
110+
this._backdropElement.addEventListener('click', () => this._backdropClick.next(null));
111111

112112
// Add class to fade-in the backdrop after one frame.
113113
requestAnimationFrame(() => {

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,22 @@ describe('Overlay', () => {
235235
expect(backdrop.style.pointerEvents).toBe('none');
236236
});
237237

238+
it('should insert the backdrop before the overlay pane in the DOM order', () => {
239+
let overlayRef = overlay.create(config);
240+
overlayRef.attach(componentPortal);
241+
242+
viewContainerFixture.detectChanges();
243+
244+
let backdrop = overlayContainerElement.querySelector('.md-overlay-backdrop');
245+
let pane = overlayContainerElement.querySelector('.md-overlay-pane');
246+
let children = Array.prototype.slice.call(overlayContainerElement.children);
247+
248+
expect(children.indexOf(backdrop)).toBeGreaterThan(-1);
249+
expect(children.indexOf(pane)).toBeGreaterThan(-1);
250+
expect(children.indexOf(backdrop))
251+
.toBeLessThan(children.indexOf(pane), 'Expected backdrop to be before the pane in the DOM');
252+
});
253+
238254
});
239255
});
240256

‎src/lib/core/style/_variables.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ $z-index-drawer: 100 !default;
2424
// stacking context for all overlays.
2525
$md-z-index-overlay-container: 1000;
2626
$md-z-index-overlay: 1000;
27-
$md-z-index-overlay-backdrop: 1;
27+
$md-z-index-overlay-backdrop: 1000;
2828

2929

3030
// Global constants

‎src/lib/menu/menu.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ describe('MdMenu', () => {
101101
fixture.componentInstance.trigger.openMenu();
102102
fixture.detectChanges();
103103

104-
const overlayPane = overlayContainerElement.children[0];
104+
const overlayPane = overlayContainerElement.querySelector('.md-overlay-pane');
105105
expect(overlayPane.getAttribute('dir')).toEqual('rtl');
106106
});
107107

@@ -248,7 +248,7 @@ describe('MdMenu', () => {
248248
});
249249

250250
function getOverlayPane(): HTMLElement {
251-
let pane = overlayContainerElement.children[0] as HTMLElement;
251+
let pane = overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
252252
pane.style.position = 'absolute';
253253
return pane;
254254
}

‎src/lib/select/select.spec.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ describe('MdSelect', () => {
102102
fixture.whenStable().then(() => {
103103
trigger.click();
104104
fixture.detectChanges();
105-
const pane = overlayContainerElement.children[0] as HTMLElement;
105+
const pane = overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
106106
expect(pane.style.minWidth).toBe('200px');
107107
});
108108
}));
@@ -559,7 +559,7 @@ describe('MdSelect', () => {
559559
* @param index The index of the option.
560560
*/
561561
function checkTriggerAlignedWithOption(index: number): void {
562-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
562+
const overlayPane = overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
563563

564564
// We need to set the position to absolute, because the top/left positioning won't work
565565
// since the component CSS isn't included in the tests.
@@ -597,7 +597,8 @@ describe('MdSelect', () => {
597597
trigger.click();
598598
fixture.detectChanges();
599599

600-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
600+
const overlayPane =
601+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
601602
const scrollContainer = overlayPane.querySelector('.md-select-panel');
602603

603604
// The panel should be scrolled to 0 because centering the option is not possible.
@@ -614,7 +615,8 @@ describe('MdSelect', () => {
614615
trigger.click();
615616
fixture.detectChanges();
616617

617-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
618+
const overlayPane =
619+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
618620
const scrollContainer = overlayPane.querySelector('.md-select-panel');
619621

620622
// The panel should be scrolled to 0 because centering the option is not possible.
@@ -631,7 +633,8 @@ describe('MdSelect', () => {
631633
trigger.click();
632634
fixture.detectChanges();
633635

634-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
636+
const overlayPane =
637+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
635638
const scrollContainer = overlayPane.querySelector('.md-select-panel');
636639

637640
// The selected option should be scrolled to the center of the panel.
@@ -652,7 +655,8 @@ describe('MdSelect', () => {
652655
trigger.click();
653656
fixture.detectChanges();
654657

655-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
658+
const overlayPane =
659+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
656660
const scrollContainer = overlayPane.querySelector('.md-select-panel');
657661

658662
// The selected option should be scrolled to the max scroll position.
@@ -685,7 +689,8 @@ describe('MdSelect', () => {
685689
trigger.click();
686690
fixture.detectChanges();
687691

688-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
692+
const overlayPane =
693+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
689694
const scrollContainer = overlayPane.querySelector('.md-select-panel');
690695

691696
// Scroll should adjust by the difference between the top space available (85px + 8px
@@ -709,7 +714,8 @@ describe('MdSelect', () => {
709714
trigger.click();
710715
fixture.detectChanges();
711716

712-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
717+
const overlayPane =
718+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
713719
const scrollContainer = overlayPane.querySelector('.md-select-panel');
714720

715721
// Scroll should adjust by the difference between the bottom space available
@@ -734,7 +740,8 @@ describe('MdSelect', () => {
734740
trigger.click();
735741
fixture.detectChanges();
736742

737-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
743+
const overlayPane =
744+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
738745

739746
// We need to set the position to absolute, because the top/left positioning won't work
740747
// since the component CSS isn't included in the tests.
@@ -766,7 +773,8 @@ describe('MdSelect', () => {
766773
trigger.click();
767774
fixture.detectChanges();
768775

769-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
776+
const overlayPane =
777+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
770778

771779
// We need to set the position to absolute, because the top/left positioning won't work
772780
// since the component CSS isn't included in the tests.
@@ -855,7 +863,8 @@ describe('MdSelect', () => {
855863
fixture.detectChanges();
856864

857865
// CSS styles aren't in the tests, so position must be absolute to reflect top/left
858-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
866+
const overlayPane =
867+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
859868
overlayPane.style.position = 'absolute';
860869

861870
const triggerBottom = trigger.getBoundingClientRect().bottom;
@@ -882,7 +891,8 @@ describe('MdSelect', () => {
882891
fixture.detectChanges();
883892

884893
// CSS styles aren't in the tests, so position must be absolute to reflect top/left
885-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
894+
const overlayPane =
895+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
886896
overlayPane.style.position = 'absolute';
887897

888898
const triggerTop = trigger.getBoundingClientRect().top;
@@ -904,7 +914,8 @@ describe('MdSelect', () => {
904914
trigger.click();
905915
fixture.detectChanges();
906916

907-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
917+
const overlayPane =
918+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
908919

909920
// We need to set the position to absolute, because the top/left positioning won't work
910921
// since the component CSS isn't included in the tests.
@@ -927,7 +938,8 @@ describe('MdSelect', () => {
927938
trigger.click();
928939
fixture.detectChanges();
929940

930-
const overlayPane = overlayContainerElement.children[0] as HTMLElement;
941+
const overlayPane =
942+
overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
931943

932944
// We need to set the position to absolute, because the top/left positioning won't work
933945
// since the component CSS isn't included in the tests.
@@ -1168,7 +1180,7 @@ describe('MdSelect', () => {
11681180
trigger.click();
11691181
fixture.detectChanges();
11701182

1171-
const pane = overlayContainerElement.children[0] as HTMLElement;
1183+
const pane = overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
11721184
expect(pane.style.minWidth).toEqual('300px');
11731185

11741186
expect(fixture.componentInstance.select.panelOpen).toBe(true);

0 commit comments

Comments
 (0)