Skip to content

Commit 539c6a9

Browse files
committed
fix(overlay): only clear duplicate containers from different platform
Currently we clear all overlay containers when we create a new one as a way to avoid duplicate content coming in from the server. Our current approach is a little too aggressive, because it can pick up containers from different apps. These changes add an extra attribute to the container so that we can determine which platform it's coming from. Fixes #16851.
1 parent 7f6972f commit 539c6a9

File tree

5 files changed

+57
-16
lines changed

5 files changed

+57
-16
lines changed

src/cdk/overlay/fullscreen-overlay-container.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {Injectable, Inject, OnDestroy} from '@angular/core';
1010
import {OverlayContainer} from './overlay-container';
1111
import {DOCUMENT} from '@angular/common';
12+
import {Platform} from '@angular/cdk/platform';
1213

1314

1415
/**
@@ -23,8 +24,14 @@ export class FullscreenOverlayContainer extends OverlayContainer implements OnDe
2324
private _fullScreenEventName: string | undefined;
2425
private _fullScreenListener: () => void;
2526

26-
constructor(@Inject(DOCUMENT) _document: any) {
27-
super(_document);
27+
constructor(
28+
@Inject(DOCUMENT) _document: any,
29+
/**
30+
* @deprecated `platform` parameter to become required.
31+
* @breaking-change 10.0.0
32+
*/
33+
platform?: Platform) {
34+
super(_document, platform);
2835
}
2936

3037
ngOnDestroy() {

src/cdk/overlay/overlay-container.spec.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,29 @@ describe('OverlayContainer', () => {
5353
.toBe(false, 'Expected the overlay container not to have class "commander-shepard"');
5454
});
5555

56-
it('should ensure that there is only one overlay container on the page', () => {
56+
it('should remove overlay containers from the server when on the browser', () => {
5757
const extraContainer = document.createElement('div');
5858
extraContainer.classList.add('cdk-overlay-container');
59+
extraContainer.setAttribute('platform', 'server');
5960
document.body.appendChild(extraContainer);
6061

6162
overlayContainer.getContainerElement();
6263

6364
expect(document.querySelectorAll('.cdk-overlay-container').length).toBe(1);
6465
});
66+
67+
it('should not remove extra containers that were created on the browser', () => {
68+
const extraContainer = document.createElement('div');
69+
extraContainer.classList.add('cdk-overlay-container');
70+
document.body.appendChild(extraContainer);
71+
72+
overlayContainer.getContainerElement();
73+
74+
expect(document.querySelectorAll('.cdk-overlay-container').length).toBe(2);
75+
76+
extraContainer.parentNode!.removeChild(extraContainer);
77+
});
78+
6579
});
6680

6781
/** Test-bed component that contains a TempatePortal and an ElementRef. */

src/cdk/overlay/overlay-container.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
Optional,
1616
SkipSelf,
1717
} from '@angular/core';
18+
import {Platform} from '@angular/cdk/platform';
1819

1920

2021
/** Container inside which all overlays will render. */
@@ -23,13 +24,21 @@ export class OverlayContainer implements OnDestroy {
2324
protected _containerElement: HTMLElement;
2425
protected _document: Document;
2526

26-
constructor(@Inject(DOCUMENT) document: any) {
27+
constructor(
28+
@Inject(DOCUMENT) document: any,
29+
/**
30+
* @deprecated `platform` parameter to become required.
31+
* @breaking-change 10.0.0
32+
*/
33+
protected _platform?: Platform) {
2734
this._document = document;
2835
}
2936

3037
ngOnDestroy() {
31-
if (this._containerElement && this._containerElement.parentNode) {
32-
this._containerElement.parentNode.removeChild(this._containerElement);
38+
const container = this._containerElement;
39+
40+
if (container && container.parentNode) {
41+
container.parentNode.removeChild(container);
3342
}
3443
}
3544

@@ -52,16 +61,28 @@ export class OverlayContainer implements OnDestroy {
5261
* with the 'cdk-overlay-container' class on the document body.
5362
*/
5463
protected _createContainer(): void {
64+
// @breaking-change 10.0.0 Remove null check for `_platform`.
65+
const isBrowser = this._platform ? this._platform.isBrowser : typeof window !== 'undefined';
5566
const containerClass = 'cdk-overlay-container';
56-
const previousContainers = this._document.getElementsByClassName(containerClass);
5767

58-
// Remove any old containers. This can happen when transitioning from the server to the client.
59-
for (let i = 0; i < previousContainers.length; i++) {
60-
previousContainers[i].parentNode!.removeChild(previousContainers[i]);
68+
if (isBrowser) {
69+
const oppositePlatformContainers =
70+
this._document.querySelectorAll(`.${containerClass}[platform="server"]`);
71+
72+
// Remove any old containers from the opposite platform.
73+
// This can happen when transitioning from the server to the client.
74+
for (let i = 0; i < oppositePlatformContainers.length; i++) {
75+
oppositePlatformContainers[i].parentNode!.removeChild(oppositePlatformContainers[i]);
76+
}
6177
}
6278

6379
const container = this._document.createElement('div');
6480
container.classList.add(containerClass);
81+
82+
if (!isBrowser) {
83+
container.setAttribute('platform', 'server');
84+
}
85+
6586
this._document.body.appendChild(container);
6687
this._containerElement = container;
6788
}

src/material-experimental/select/testing/shared.spec.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,11 @@ function getActiveElementId() {
222222
<mat-option *ngFor="let state of states" [value]="state.code">{{ state.name }}</mat-option>
223223
</mat-select>
224224
</mat-form-field>
225-
226225
<mat-form-field>
227226
<mat-select multiple id="multiple-selection">
228227
<mat-option *ngFor="let state of states" [value]="state.code">{{ state.name }}</mat-option>
229228
</mat-select>
230229
</mat-form-field>
231-
232230
<mat-form-field>
233231
<mat-select id="grouped">
234232
<mat-optgroup *ngFor="let group of stateGroups" [label]="group.name">
@@ -238,7 +236,6 @@ function getActiveElementId() {
238236
</mat-optgroup>
239237
</mat-select>
240238
</mat-form-field>
241-
242239
<mat-form-field>
243240
<mat-select [formControl]="formControl" id="with-form-control">
244241
<mat-option *ngFor="let state of states" [value]="state.code">{{ state.name }}</mat-option>
@@ -279,4 +276,3 @@ class SelectHarnessTest {
279276
}
280277
];
281278
}
282-

tools/public_api_guard/cdk/overlay.d.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ export declare type FlexibleConnectedPositionStrategyOrigin = ElementRef | HTMLE
134134
};
135135

136136
export declare class FullscreenOverlayContainer extends OverlayContainer implements OnDestroy {
137-
constructor(_document: any);
137+
constructor(_document: any,
138+
platform?: Platform);
138139
protected _createContainer(): void;
139140
getFullscreenElement(): Element;
140141
ngOnDestroy(): void;
@@ -202,7 +203,9 @@ export interface OverlayConnectionPosition {
202203
export declare class OverlayContainer implements OnDestroy {
203204
protected _containerElement: HTMLElement;
204205
protected _document: Document;
205-
constructor(document: any);
206+
protected _platform?: Platform | undefined;
207+
constructor(document: any,
208+
_platform?: Platform | undefined);
206209
protected _createContainer(): void;
207210
getContainerElement(): HTMLElement;
208211
ngOnDestroy(): void;

0 commit comments

Comments
 (0)