Skip to content

Commit 2b2cb66

Browse files
committed
refactor(cdk/overlay): keep global overlays inside the container
Keeps global overlays inside the overlay container since it can be breaking for users to move them out. (cherry picked from commit 8e1890b)
1 parent 1f196ee commit 2b2cb66

File tree

6 files changed

+18
-70
lines changed

6 files changed

+18
-70
lines changed

goldens/cdk/overlay/index.api.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ export function createCloseScrollStrategy(injector: Injector, config?: CloseScro
267267
export function createFlexibleConnectedPositionStrategy(injector: Injector, origin: FlexibleConnectedPositionStrategyOrigin): FlexibleConnectedPositionStrategy;
268268

269269
// @public
270-
export function createGlobalPositionStrategy(injector: Injector): GlobalPositionStrategy;
270+
export function createGlobalPositionStrategy(_injector: Injector): GlobalPositionStrategy;
271271

272272
// @public
273273
export function createNoopScrollStrategy(): NoopScrollStrategy;
@@ -327,7 +327,6 @@ export class FullscreenOverlayContainer extends OverlayContainer implements OnDe
327327

328328
// @public
329329
export class GlobalPositionStrategy implements PositionStrategy {
330-
constructor(injector?: Injector);
331330
apply(): void;
332331
// (undocumented)
333332
attach(overlayRef: OverlayRef): void;
@@ -336,7 +335,6 @@ export class GlobalPositionStrategy implements PositionStrategy {
336335
centerVertically(offset?: string): this;
337336
dispose(): void;
338337
end(value?: string): this;
339-
getPopoverInsertionPoint(): Element;
340338
// @deprecated
341339
height(value?: string): this;
342340
left(value?: string): this;
@@ -525,7 +523,7 @@ export interface PositionStrategy {
525523
attach(overlayRef: OverlayRef): void;
526524
detach?(): void;
527525
dispose(): void;
528-
getPopoverInsertionPoint?(): Element;
526+
getPopoverInsertionPoint?(): Element | null;
529527
}
530528

531529
// @public

src/cdk/overlay/overlay-ref.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,8 +405,12 @@ export class OverlayRef implements PortalOutlet {
405405

406406
private _attachHost() {
407407
if (!this._host.parentElement) {
408-
if (this._config.usePopover && this._positionStrategy?.getPopoverInsertionPoint) {
409-
this._positionStrategy.getPopoverInsertionPoint().after(this._host);
408+
const customInsertionPoint = this._config.usePopover
409+
? this._positionStrategy?.getPopoverInsertionPoint?.()
410+
: null;
411+
412+
if (customInsertionPoint) {
413+
customInsertionPoint.after(this._host);
410414
} else {
411415
this._previousHostParent?.appendChild(this._host);
412416
}

src/cdk/overlay/overlay.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,12 @@ export function createOverlayRef(injector: Injector, config?: OverlayConfig): Ov
6767
host.classList.add('cdk-overlay-popover');
6868
}
6969

70-
if (overlayConfig.usePopover && overlayConfig.positionStrategy?.getPopoverInsertionPoint) {
71-
overlayConfig.positionStrategy.getPopoverInsertionPoint().after(host);
70+
const customInsertionPoint = overlayConfig.usePopover
71+
? overlayConfig.positionStrategy?.getPopoverInsertionPoint?.()
72+
: null;
73+
74+
if (customInsertionPoint) {
75+
customInsertionPoint.after(host);
7276
} else {
7377
overlayContainer.getContainerElement().appendChild(host);
7478
}

src/cdk/overlay/position/global-position-strategy.spec.ts

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ import {
77
OverlayRef,
88
createOverlayRef,
99
createGlobalPositionStrategy,
10-
GlobalPositionStrategy,
11-
OverlayContainer,
1210
} from '../index';
1311

1412
describe('GlobalPositonStrategy', () => {
@@ -472,50 +470,6 @@ describe('GlobalPositonStrategy', () => {
472470
expect(elementStyle.marginRight).toBe('');
473471
expect(parentStyle.justifyContent).toBe('flex-end');
474472
});
475-
476-
describe('DOM location', () => {
477-
let positionStrategy: GlobalPositionStrategy;
478-
let containerElement: HTMLElement;
479-
480-
beforeEach(() => {
481-
containerElement = TestBed.inject(OverlayContainer).getContainerElement();
482-
positionStrategy = createGlobalPositionStrategy(injector);
483-
});
484-
485-
it('should place the overlay inside the overlay container by default', () => {
486-
attachOverlay({positionStrategy, usePopover: false});
487-
expect(containerElement.contains(overlayRef.hostElement)).toBe(true);
488-
expect(overlayRef.hostElement.getAttribute('popover')).toBeFalsy();
489-
});
490-
491-
it('should be able to opt into placing the overlay inside a popover element', () => {
492-
if (!('showPopover' in document.body)) {
493-
return;
494-
}
495-
496-
attachOverlay({positionStrategy, usePopover: true});
497-
498-
expect(containerElement.contains(overlayRef.hostElement)).toBe(false);
499-
expect(document.body.lastChild).toBe(overlayRef.hostElement);
500-
expect(overlayRef.hostElement.getAttribute('popover')).toBe('manual');
501-
});
502-
503-
it('should re-attach the popover at the end of the body', () => {
504-
if (!('showPopover' in document.body)) {
505-
return;
506-
}
507-
508-
attachOverlay({positionStrategy, usePopover: true});
509-
expect(document.body.lastChild).toBe(overlayRef.hostElement);
510-
511-
overlayRef.detach();
512-
TestBed.inject(ApplicationRef).tick();
513-
expect(overlayRef.hostElement.parentNode).toBeFalsy();
514-
515-
overlayRef.attach(portal);
516-
expect(document.body.lastChild).toBe(overlayRef.hostElement);
517-
});
518-
});
519473
});
520474

521475
@Component({

src/cdk/overlay/position/global-position-strategy.ts

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {DOCUMENT, Injector} from '@angular/core';
9+
import {Injector} from '@angular/core';
1010
import {OverlayRef} from '../overlay-ref';
1111
import {PositionStrategy} from './position-strategy';
1212

@@ -17,8 +17,8 @@ const wrapperClass = 'cdk-global-overlay-wrapper';
1717
* Creates a global position strategy.
1818
* @param injector Injector used to resolve dependencies for the strategy.
1919
*/
20-
export function createGlobalPositionStrategy(injector: Injector): GlobalPositionStrategy {
21-
return new GlobalPositionStrategy(injector);
20+
export function createGlobalPositionStrategy(_injector: Injector): GlobalPositionStrategy {
21+
return new GlobalPositionStrategy();
2222
}
2323

2424
/**
@@ -39,13 +39,6 @@ export class GlobalPositionStrategy implements PositionStrategy {
3939
private _width = '';
4040
private _height = '';
4141
private _isDisposed = false;
42-
private _document: Document;
43-
44-
constructor(injector?: Injector) {
45-
// TODO(crisbeto): injector should be required, but some internal apps
46-
// don't go through `createGlobalPositionStrategy` so they don't provide it.
47-
this._document = injector?.get(DOCUMENT) || document;
48-
}
4942

5043
attach(overlayRef: OverlayRef): void {
5144
const config = overlayRef.getConfig();
@@ -274,9 +267,4 @@ export class GlobalPositionStrategy implements PositionStrategy {
274267
this._overlayRef = null!;
275268
this._isDisposed = true;
276269
}
277-
278-
/** @docs-private */
279-
getPopoverInsertionPoint(): Element {
280-
return this._document.body.lastChild as Element;
281-
}
282270
}

src/cdk/overlay/position/position-strategy.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,5 @@ export interface PositionStrategy {
2626
* Gets the element in the DOM after which to insert
2727
* the overlay when it is rendered out as a popover.
2828
*/
29-
getPopoverInsertionPoint?(): Element;
29+
getPopoverInsertionPoint?(): Element | null;
3030
}

0 commit comments

Comments
 (0)