fix(menu,toolbar): avoid potential server-side rendering errors #9423
Conversation
googlebot
added
the
cla: yes
| @@ -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; | |||
There was a problem hiding this comment.
Make constants for node types until _document is always available?
There was a problem hiding this comment.
I'm inclined not to, because we'll end up sprinkling them everywhere and it'll make it more likely to forget about removing them.
jelbourn
added
pr: lgtm
action: merge
|
@crisbeto passes presubmit, just needs rebease |
Avoids a couple of potential server-side rendering errors due to the menu item and toolbar components referring to the `Node` global variable directly.
crisbeto
force-pushed
the
menu-toolbar-global-variables
branch
from
8f85a26 to
b88f0ae
Compare
|
Rebased. |
| constructor( | ||
| elementRef: ElementRef, | ||
| private _platform: Platform, | ||
| @Inject(DOCUMENT) document?: any) { |
There was a problem hiding this comment.
@crisbeto I know this PR has already been merged, but shouldn't this have @deletion-target x.y.z as in others constructor parameters?
There was a problem hiding this comment.
I don't think we had the tooling around @deletion-target when I first did this PR.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Avoids a couple of potential server-side rendering errors due to the menu item and toolbar components referring to the
Nodeglobal variable directly.