Skip to content

Commit 1b30154

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 c8ceae5 commit 1b30154

File tree

6 files changed

+63
-17
lines changed

6 files changed

+63
-17
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: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,30 @@ 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+
extraContainer.setAttribute('platform', 'browser');
71+
document.body.appendChild(extraContainer);
72+
73+
overlayContainer.getContainerElement();
74+
75+
expect(document.querySelectorAll('.cdk-overlay-container').length).toBe(2);
76+
77+
extraContainer.parentNode!.removeChild(extraContainer);
78+
});
79+
6580
});
6681

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

src/cdk/overlay/overlay-container.ts

Lines changed: 21 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,21 @@ 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);
67+
const oppositePlatformContainers = this._document.querySelectorAll(
68+
`.${containerClass}[platform="${isBrowser ? 'server' : 'browser'}"]`);
5769

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]);
70+
// Remove any old containers from the opposite platform.
71+
// This can happen when transitioning from the server to the client.
72+
for (let i = 0; i < oppositePlatformContainers.length; i++) {
73+
oppositePlatformContainers[i].parentNode!.removeChild(oppositePlatformContainers[i]);
6174
}
6275

6376
const container = this._document.createElement('div');
6477
container.classList.add(containerClass);
78+
container.setAttribute('platform', isBrowser ? 'browser' : 'server');
6579
this._document.body.appendChild(container);
6680
this._containerElement = container;
6781
}

src/material-experimental/mdc-select/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ ng_test_library(
6666
":mdc-select",
6767
"//src/cdk-experimental/testing",
6868
"//src/cdk-experimental/testing/testbed",
69+
"//src/cdk/overlay",
6970
"//src/cdk/platform",
7071
"//src/cdk/testing",
7172
"//src/material/form-field",

src/material-experimental/mdc-select/harness/select-harness.spec.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import {HarnessLoader} from '@angular/cdk-experimental/testing';
22
import {TestbedHarnessEnvironment} from '@angular/cdk-experimental/testing/testbed';
33
import {Component} from '@angular/core';
4-
import {ComponentFixture, TestBed} from '@angular/core/testing';
4+
import {ComponentFixture, TestBed, inject} from '@angular/core/testing';
55
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
66
import {ReactiveFormsModule, FormControl, Validators} from '@angular/forms';
7+
import {OverlayContainer} from '@angular/cdk/overlay';
78
import {MatFormFieldModule} from '@angular/material/form-field';
89
import {MatSelectModule} from '@angular/material/select';
910
import {MatSelectModule as MatMdcSelectModule} from '../index';
@@ -13,6 +14,7 @@ import {MatSelectHarness as MatMdcSelectHarness} from './mdc-select-harness';
1314
let fixture: ComponentFixture<SelectHarnessTest>;
1415
let loader: HarnessLoader;
1516
let harness: typeof MatSelectHarness;
17+
let overlayContainer: OverlayContainer;
1618

1719
describe('MatSelectHarness', () => {
1820
describe('non-MDC-based', () => {
@@ -31,6 +33,9 @@ describe('MatSelectHarness', () => {
3133
fixture.detectChanges();
3234
loader = TestbedHarnessEnvironment.loader(fixture);
3335
harness = MatSelectHarness;
36+
inject([OverlayContainer], (oc: OverlayContainer) => {
37+
overlayContainer = oc;
38+
})();
3439
});
3540

3641
runTests();
@@ -63,6 +68,11 @@ describe('MatSelectHarness', () => {
6368

6469
/** Shared tests to run on both the original and MDC-based select. */
6570
function runTests() {
71+
afterEach(() => {
72+
// Angular won't call this for us so we need to do it ourselves to avoid leaks.
73+
overlayContainer.ngOnDestroy();
74+
});
75+
6676
it('should load all select harnesses', async () => {
6777
const selects = await loader.getAllHarnesses(harness);
6878
expect(selects.length).toBe(4);
@@ -244,13 +254,11 @@ function getActiveElementId() {
244254
<mat-option *ngFor="let state of states" [value]="state.code">{{ state.name }}</mat-option>
245255
</mat-select>
246256
</mat-form-field>
247-
248257
<mat-form-field>
249258
<mat-select multiple id="multiple-selection">
250259
<mat-option *ngFor="let state of states" [value]="state.code">{{ state.name }}</mat-option>
251260
</mat-select>
252261
</mat-form-field>
253-
254262
<mat-form-field>
255263
<mat-select id="grouped">
256264
<mat-optgroup *ngFor="let group of stateGroups" [label]="group.name">
@@ -260,7 +268,6 @@ function getActiveElementId() {
260268
</mat-optgroup>
261269
</mat-select>
262270
</mat-form-field>
263-
264271
<mat-form-field>
265272
<mat-select [formControl]="formControl" id="with-form-control">
266273
<mat-option *ngFor="let state of states" [value]="state.code">{{ state.name }}</mat-option>
@@ -301,4 +308,3 @@ class SelectHarnessTest {
301308
}
302309
];
303310
}
304-

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)