Skip to content

Commit dfa68db

Browse files
crisbetojelbourn
authored andcommitted
fix(menu,toolbar): avoid potential server-side rendering errors (#9423)
Avoids a couple of potential server-side rendering errors due to the menu item and toolbar components referring to the `Node` global variable directly.
1 parent a1db4e4 commit dfa68db

File tree

2 files changed

+23
-5
lines changed

2 files changed

+23
-5
lines changed

src/lib/menu/menu-item.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
ElementRef,
1414
OnDestroy,
1515
ViewEncapsulation,
16+
Inject,
1617
} from '@angular/core';
1718
import {
1819
CanDisable,
@@ -21,6 +22,7 @@ import {
2122
mixinDisableRipple
2223
} from '@angular/material/core';
2324
import {Subject} from 'rxjs/Subject';
25+
import {DOCUMENT} from '@angular/common';
2426

2527
// Boilerplate for applying mixins to MatMenuItem.
2628
/** @docs-private */
@@ -55,6 +57,8 @@ export const _MatMenuItemMixinBase = mixinDisableRipple(mixinDisabled(MatMenuIte
5557
export class MatMenuItem extends _MatMenuItemMixinBase
5658
implements FocusableOption, CanDisable, CanDisableRipple, OnDestroy {
5759

60+
private _document: Document;
61+
5862
/** Stream that emits when the menu item is hovered. */
5963
_hovered: Subject<MatMenuItem> = new Subject<MatMenuItem>();
6064

@@ -66,8 +70,10 @@ export class MatMenuItem extends _MatMenuItemMixinBase
6670

6771
constructor(
6872
private _elementRef: ElementRef,
69-
// TODO(crisbeto): switch to a required param when doing breaking changes.
73+
@Inject(DOCUMENT) document?: any,
7074
private _focusMonitor?: FocusMonitor) {
75+
76+
// @deletion-target 6.0.0 make `_focusMonitor` and `document` required params.
7177
super();
7278

7379
if (_focusMonitor) {
@@ -76,6 +82,8 @@ export class MatMenuItem extends _MatMenuItemMixinBase
7682
// mouse or touch interaction.
7783
_focusMonitor.monitor(this._getHostElement(), false);
7884
}
85+
86+
this._document = document;
7987
}
8088

8189
/** Focuses the menu item. */
@@ -123,6 +131,7 @@ export class MatMenuItem extends _MatMenuItemMixinBase
123131
/** Gets the label to be used when determining whether the option should be focused. */
124132
getLabel(): string {
125133
const element: HTMLElement = this._elementRef.nativeElement;
134+
const textNodeType = this._document ? this._document.TEXT_NODE : 3;
126135
let output = '';
127136

128137
if (element.childNodes) {
@@ -132,7 +141,7 @@ export class MatMenuItem extends _MatMenuItemMixinBase
132141
// We skip anything that's not a text node to prevent the text from
133142
// being thrown off by something like an icon.
134143
for (let i = 0; i < length; i++) {
135-
if (element.childNodes[i].nodeType === Node.TEXT_NODE) {
144+
if (element.childNodes[i].nodeType === textNodeType) {
136145
output += element.childNodes[i].textContent;
137146
}
138147
}

src/lib/toolbar/toolbar.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ import {
1515
ElementRef,
1616
isDevMode,
1717
QueryList,
18-
ViewEncapsulation
18+
ViewEncapsulation,
19+
Inject,
1920
} from '@angular/core';
2021
import {CanColor, mixinColor} from '@angular/material/core';
2122
import {Platform} from '@angular/cdk/platform';
23+
import {DOCUMENT} from '@angular/common';
2224

2325
// Boilerplate for applying mixins to MatToolbar.
2426
/** @docs-private */
@@ -51,12 +53,19 @@ export class MatToolbarRow {}
5153
preserveWhitespaces: false,
5254
})
5355
export class MatToolbar extends _MatToolbarMixinBase implements CanColor, AfterViewInit {
56+
private _document: Document;
5457

5558
/** Reference to all toolbar row elements that have been projected. */
5659
@ContentChildren(MatToolbarRow) _toolbarRows: QueryList<MatToolbarRow>;
5760

58-
constructor(elementRef: ElementRef, private _platform: Platform) {
61+
constructor(
62+
elementRef: ElementRef,
63+
private _platform: Platform,
64+
@Inject(DOCUMENT) document?: any) {
5965
super(elementRef);
66+
67+
// TODO: make the document a required param when doing breaking changes.
68+
this._document = document;
6069
}
6170

6271
ngAfterViewInit() {
@@ -80,7 +89,7 @@ export class MatToolbar extends _MatToolbarMixinBase implements CanColor, AfterV
8089
// a <mat-toolbar-row> element.
8190
const isCombinedUsage = [].slice.call(this._elementRef.nativeElement.childNodes)
8291
.filter(node => !(node.classList && node.classList.contains('mat-toolbar-row')))
83-
.filter(node => node.nodeType !== Node.COMMENT_NODE)
92+
.filter(node => node.nodeType !== (this._document ? this._document.COMMENT_NODE : 8))
8493
.some(node => node.textContent.trim());
8594

8695
if (isCombinedUsage) {

0 commit comments

Comments
 (0)