Skip to content

Commit

Permalink
fix(menu,toolbar): avoid potential server-side rendering errors
Browse files Browse the repository at this point in the history
Avoids a couple of potential server-side rendering errors due to the menu item and toolbar components referring to the `Node` global variable directly.
  • Loading branch information
crisbeto committed Jan 16, 2018
1 parent 60b0625 commit 8f85a26
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 5 deletions.
12 changes: 10 additions & 2 deletions src/lib/menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
ElementRef,
OnDestroy,
ViewEncapsulation,
Inject,
} from '@angular/core';
import {
CanDisable,
Expand All @@ -21,6 +22,7 @@ import {
mixinDisableRipple
} from '@angular/material/core';
import {Subject} from 'rxjs/Subject';
import {DOCUMENT} from '@angular/common';

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

private _document: Document;

/** Stream that emits when the menu item is hovered. */
_hovered: Subject<MatMenuItem> = new Subject();

Expand All @@ -64,8 +68,11 @@ export class MatMenuItem extends _MatMenuItemMixinBase
/** Whether the menu item acts as a trigger for a sub-menu. */
_triggersSubmenu: boolean = false;

constructor(private _elementRef: ElementRef) {
constructor(private _elementRef: ElementRef, @Inject(DOCUMENT) document?: any) {
super();

// TODO: make the document a required param when doing breaking changes.
this._document = document;
}

/** Focuses the menu item. */
Expand Down Expand Up @@ -105,6 +112,7 @@ export class MatMenuItem extends _MatMenuItemMixinBase
/** Gets the label to be used when determining whether the option should be focused. */
getLabel(): string {
const element: HTMLElement = this._elementRef.nativeElement;
const textNodeType = this._document ? this._document.TEXT_NODE : 3;
let output = '';

if (element.childNodes) {
Expand All @@ -114,7 +122,7 @@ export class MatMenuItem extends _MatMenuItemMixinBase
// We skip anything that's not a text node to prevent the text from
// being thrown off by something like an icon.
for (let i = 0; i < length; i++) {
if (element.childNodes[i].nodeType === Node.TEXT_NODE) {
if (element.childNodes[i].nodeType === textNodeType) {
output += element.childNodes[i].textContent;
}
}
Expand Down
15 changes: 12 additions & 3 deletions src/lib/toolbar/toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ import {
ElementRef,
isDevMode,
QueryList,
ViewEncapsulation
ViewEncapsulation,
Inject,
} from '@angular/core';
import {CanColor, mixinColor} from '@angular/material/core';
import {Platform} from '@angular/cdk/platform';
import {DOCUMENT} from '@angular/common';

// Boilerplate for applying mixins to MatToolbar.
/** @docs-private */
Expand Down Expand Up @@ -51,12 +53,19 @@ export class MatToolbarRow {}
preserveWhitespaces: false,
})
export class MatToolbar extends _MatToolbarMixinBase implements CanColor, AfterViewInit {
private _document: Document;

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

constructor(elementRef: ElementRef, private _platform: Platform) {
constructor(
elementRef: ElementRef,
private _platform: Platform,
@Inject(DOCUMENT) document?: any) {
super(elementRef);

// TODO: make the document a required param when doing breaking changes.
this._document = document;
}

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

if (isCombinedUsage) {
Expand Down

0 comments on commit 8f85a26

Please sign in to comment.