Skip to content

Commit

Permalink
fix(overlay): proper backdrop stacking with multiple overlays (#2276)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
crisbeto authored and jelbourn committed Dec 20, 2016
1 parent 75cd73b commit b16031a
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 23 deletions.
8 changes: 4 additions & 4 deletions src/lib/core/overlay/overlay-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
16 changes: 16 additions & 0 deletions src/lib/core/overlay/overlay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

});
});

Expand Down
3 changes: 1 addition & 2 deletions src/lib/core/style/_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand Down Expand Up @@ -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;
}
Expand Down
42 changes: 27 additions & 15 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
}));
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b16031a

Please sign in to comment.