Skip to content

Commit

Permalink
fix(overlay): proper backdrop stacking with multiple overlays
Browse files Browse the repository at this point in the history 
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 angular#2272.
  • Loading branch information
@crisbeto
crisbeto committed Dec 19, 2016
1 parent 2168d7c commit b203c28
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 27 deletions.
19 changes: 10 additions & 9 deletions src/lib/core/overlay/overlay-ref.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,23 @@ export class OverlayRef implements PortalHost {

/** Attaches a backdrop for this overlay. */
private _attachBackdrop() {
this._backdropElement = document.createElement('div');
this._backdropElement.classList.add('md-overlay-backdrop');
this._backdropElement.classList.add(this._state.backdropClass);
let backdrop = this._backdropElement = document.createElement('div');

this._pane.parentElement.appendChild(this._backdropElement);
backdrop.classList.add('md-overlay-backdrop');
backdrop.classList.add(this._state.backdropClass);

// Insert the backdrop before the pane in the DOM order,
// in order to handle stacked overlays properly.
this._pane.parentElement.insertBefore(backdrop, 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);
});
backdrop.addEventListener('click', () => this._backdropClick.next(null));

// Add class to fade-in the backdrop after one frame.
requestAnimationFrame(() => {
if (this._backdropElement) {
this._backdropElement.classList.add('md-overlay-backdrop-showing');
if (backdrop) {
backdrop.classList.add('md-overlay-backdrop-showing');
}
});
}
Expand Down
16 changes: 16 additions & 0 deletions src/lib/core/overlay/overlay.spec.ts
View file
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('.md-overlay-backdrop');
let pane = overlayContainerElement.querySelector('.md-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
2 changes: 1 addition & 1 deletion src/lib/core/style/_variables.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ $z-index-drawer: 100 !default;
// stacking context for all overlays.
$md-z-index-overlay-container: 1000;
$md-z-index-overlay: 1000;
$md-z-index-overlay-backdrop: 1;
$md-z-index-overlay-backdrop: 1000;


// Global constants
Expand Down
4 changes: 2 additions & 2 deletions src/lib/menu/menu.spec.ts
View file
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('.md-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('.md-overlay-pane') as HTMLElement;
pane.style.position = 'absolute';
return pane;
}
Expand Down
42 changes: 27 additions & 15 deletions src/lib/select/select.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ describe('MdSelect', () => {
fixture.whenStable().then(() => {
trigger.click();
fixture.detectChanges();
const pane = overlayContainerElement.children[0] as HTMLElement;
const pane = overlayContainerElement.querySelector('.md-overlay-pane') as HTMLElement;
expect(pane.style.minWidth).toBe('200px');
});
}));
Expand Down Expand Up @@ -559,7 +559,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('.md-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 @@ -597,7 +597,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -614,7 +615,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -631,7 +633,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -652,7 +655,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -685,7 +689,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -709,7 +714,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

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

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

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -766,7 +773,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -855,7 +863,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('.md-overlay-pane') as HTMLElement;
overlayPane.style.position = 'absolute';

const triggerBottom = trigger.getBoundingClientRect().bottom;
Expand All @@ -882,7 +891,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('.md-overlay-pane') as HTMLElement;
overlayPane.style.position = 'absolute';

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

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -927,7 +938,8 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

const overlayPane = overlayContainerElement.children[0] as HTMLElement;
const overlayPane =
overlayContainerElement.querySelector('.md-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 @@ -1168,7 +1180,7 @@ describe('MdSelect', () => {
trigger.click();
fixture.detectChanges();

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

expect(fixture.componentInstance.select.panelOpen).toBe(true);
Expand Down

0 comments on commit b203c28

Please sign in to comment.