From 068f4166cf1bf2f3c30c4ecaf20947b8f79ae798 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 25 Mar 2017 12:36:33 +0100 Subject: [PATCH 1/2] feat(theming): log a warning if core theme isn't loaded * Checks the user's loaded stylesheets and logs a warning if the Material core theme isn't loaded. * Fixes a wrong `typeof` check when determining the doctype. Fixes #2828. Note: I originally went with looping through the `document.styleSheets` to check whether the selector is defined, however I had to switch back to `getComputedStyle`, because browsers don't expose the `document.styleSheets`, if the CSS file is being loaded from another domain. This would've caused the warning to be logged if the user loads over a CDN. --- src/lib/core/_core.scss | 5 ++++ src/lib/core/compatibility/compatibility.ts | 33 +++++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/lib/core/_core.scss b/src/lib/core/_core.scss index d7ef849bae98..2eeb5405d386 100644 --- a/src/lib/core/_core.scss +++ b/src/lib/core/_core.scss @@ -36,4 +36,9 @@ $background: map-get($theme, background); background-color: mat-color($background, background); } + + // Marker that is used to determine whether the user has added a theme to their page. + .mat-theme-loaded-marker { + display: none; + } } diff --git a/src/lib/core/compatibility/compatibility.ts b/src/lib/core/compatibility/compatibility.ts index 7bb8ee9eea1d..cd671afa6c3a 100644 --- a/src/lib/core/compatibility/compatibility.ts +++ b/src/lib/core/compatibility/compatibility.ts @@ -9,6 +9,8 @@ import { } from '@angular/core'; import {DOCUMENT} from '@angular/platform-browser'; +/** Flag for whether we've checked that the theme is loaded. */ +let hasCheckedThemePresence = false; export const MATERIAL_COMPATIBILITY_MODE = new OpaqueToken('md-compatibility-mode'); @@ -170,14 +172,41 @@ export class CompatibilityModule { }; } - constructor(@Optional() @Inject(DOCUMENT) document: any) { - if (isDevMode() && typeof document && !document.doctype) { + constructor(@Optional() @Inject(DOCUMENT) private _document: any) { + this._checkDoctype(); + this._checkTheme(); + } + + private _checkDoctype(): void { + if (isDevMode() && this._document && !this._document.doctype) { console.warn( 'Current document does not have a doctype. This may cause ' + 'some Angular Material components not to behave as expected.' ); } } + + private _checkTheme(): void { + if (hasCheckedThemePresence || !this._document || !isDevMode()) { + return; + } + + let testElement = this._document.createElement('div'); + + testElement.classList.add('mat-theme-loaded-marker'); + this._document.body.appendChild(testElement); + + if (getComputedStyle(testElement).display !== 'none') { + console.warn( + 'Could not find Angular Material core theme. Most Material ' + + 'components may not work as expected. For more info refer ' + + 'to the theming guide: https://github.com/angular/material2/blob/master/guides/theming.md' + ); + } + + this._document.body.removeChild(testElement); + hasCheckedThemePresence = true; + } } From 0dd4416317e2ebe45934a8cb42747a28158a89c6 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 28 Mar 2017 20:06:47 +0200 Subject: [PATCH 2/2] refactor: address feedback --- src/lib/core/compatibility/compatibility.ts | 40 ++++++++++----------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/lib/core/compatibility/compatibility.ts b/src/lib/core/compatibility/compatibility.ts index cd671afa6c3a..0e8bbdb97f04 100644 --- a/src/lib/core/compatibility/compatibility.ts +++ b/src/lib/core/compatibility/compatibility.ts @@ -9,8 +9,8 @@ import { } from '@angular/core'; import {DOCUMENT} from '@angular/platform-browser'; -/** Flag for whether we've checked that the theme is loaded. */ -let hasCheckedThemePresence = false; +/** Whether we've done the global sanity checks (e.g. a theme is loaded, there is a doctype). */ +let hasDoneGlobalChecks = false; export const MATERIAL_COMPATIBILITY_MODE = new OpaqueToken('md-compatibility-mode'); @@ -173,12 +173,15 @@ export class CompatibilityModule { } constructor(@Optional() @Inject(DOCUMENT) private _document: any) { - this._checkDoctype(); - this._checkTheme(); + if (!hasDoneGlobalChecks && isDevMode()) { + this._checkDoctype(); + this._checkTheme(); + hasDoneGlobalChecks = true; + } } private _checkDoctype(): void { - if (isDevMode() && this._document && !this._document.doctype) { + if (this._document && !this._document.doctype) { console.warn( 'Current document does not have a doctype. This may cause ' + 'some Angular Material components not to behave as expected.' @@ -187,25 +190,22 @@ export class CompatibilityModule { } private _checkTheme(): void { - if (hasCheckedThemePresence || !this._document || !isDevMode()) { - return; - } + if (this._document) { + const testElement = this._document.createElement('div'); - let testElement = this._document.createElement('div'); + testElement.classList.add('mat-theme-loaded-marker'); + this._document.body.appendChild(testElement); - testElement.classList.add('mat-theme-loaded-marker'); - this._document.body.appendChild(testElement); + if (getComputedStyle(testElement).display !== 'none') { + console.warn( + 'Could not find Angular Material core theme. Most Material ' + + 'components may not work as expected. For more info refer ' + + 'to the theming guide: https://material.angular.io/guide/theming' + ); + } - if (getComputedStyle(testElement).display !== 'none') { - console.warn( - 'Could not find Angular Material core theme. Most Material ' + - 'components may not work as expected. For more info refer ' + - 'to the theming guide: https://github.com/angular/material2/blob/master/guides/theming.md' - ); + this._document.body.removeChild(testElement); } - - this._document.body.removeChild(testElement); - hasCheckedThemePresence = true; } }