From e9c89e06c3823db1aef1580bc47d3d518a12cdef Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 19:13:24 -0500 Subject: [PATCH 01/11] Conditionally load DOM4 polyfill (load only in the browser) --- packages/core/package.json | 1 + packages/core/src/components/index.ts | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/core/package.json b/packages/core/package.json index 008317f458..c2bf29d24f 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -6,6 +6,7 @@ "typings": "dist/index.d.ts", "style": "dist/blueprint.css", "dependencies": { + "@types/dom4": "^1.5.20", "classnames": "^2.2", "dom4": "^1.8", "normalize.css": "4.1.1", diff --git a/packages/core/src/components/index.ts b/packages/core/src/components/index.ts index 9e64014f6b..dc64d21588 100644 --- a/packages/core/src/components/index.ts +++ b/packages/core/src/components/index.ts @@ -5,7 +5,11 @@ * and https://github.com/palantir/blueprint/blob/master/PATENTS */ -import "dom4"; +declare function require(moduleName: string): any; //declare node.js 'require' so that we can conditionally import +if (typeof window != 'undefined' && typeof document != 'undefined') { //we're in browser + require("dom4"); //only import actual dom4 if we're in the browser (not server-compatible) + //we'll still need dom4 types for the TypeScript to compile, these are included in package.json +} import * as contextMenu from "./context-menu/contextMenu"; export const ContextMenu = contextMenu; From f281672a0826049d6d9cf69fa2599c737daa91a3 Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 21:05:15 -0500 Subject: [PATCH 02/11] Check for presence of 'document' before attempting to use it. Safely default to null to fix server-side rendering. --- packages/core/src/accessibility/focusStyleManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/accessibility/focusStyleManager.ts b/packages/core/src/accessibility/focusStyleManager.ts index e4b9dd2101..fdca23035e 100644 --- a/packages/core/src/accessibility/focusStyleManager.ts +++ b/packages/core/src/accessibility/focusStyleManager.ts @@ -9,7 +9,7 @@ import { InteractionModeEngine } from "../common/interactionMode"; export const FOCUS_DISABLED_CLASS = "pt-focus-disabled"; -const focusEngine = new InteractionModeEngine(document.documentElement, FOCUS_DISABLED_CLASS); +const focusEngine = new InteractionModeEngine(typeof document != 'undefined' ? document.documentElement : null, FOCUS_DISABLED_CLASS); // this is basically meaningless to unit test; it requires manual UI testing /* istanbul ignore next */ From 32dda358d7a1b73e2dfbc5cccba9c9f4cd52888c Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 21:07:59 -0500 Subject: [PATCH 03/11] Check for presence of 'navigator' before attempting to use it. Safely default to "ctrl" (as on all non-Apple platforms) to fix server-side rendering. --- packages/core/src/components/hotkeys/hotkeyParser.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/components/hotkeys/hotkeyParser.ts b/packages/core/src/components/hotkeys/hotkeyParser.ts index eb3019e453..31fb50fb2a 100644 --- a/packages/core/src/components/hotkeys/hotkeyParser.ts +++ b/packages/core/src/components/hotkeys/hotkeyParser.ts @@ -112,7 +112,7 @@ export const Aliases = { command: "meta", escape: "esc", minus: "-", - mod: /Mac|iPod|iPhone|iPad/.test(navigator.platform) ? "meta" : "ctrl", + mod: ((typeof navigator != 'undefined') && /Mac|iPod|iPhone|iPad/.test(navigator.platform)) ? "meta" : "ctrl", option: "alt", plus: "+", return: "enter", From 345e1fa88d275ff94e984deb0f98d1c01e170258 Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 19:40:56 -0500 Subject: [PATCH 04/11] Store the "show dialog" key rather than the key combo in a const. Fixes server-side rendering. --- packages/core/src/components/hotkeys/hotkeysEvents.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeysEvents.ts b/packages/core/src/components/hotkeys/hotkeysEvents.ts index da1d3bf1ba..f695913219 100644 --- a/packages/core/src/components/hotkeys/hotkeysEvents.ts +++ b/packages/core/src/components/hotkeys/hotkeysEvents.ts @@ -13,7 +13,7 @@ import { comboMatches, getKeyCombo, IKeyCombo, parseKeyCombo } from "./hotkeyPar import { IHotkeysProps } from "./hotkeys"; import { isHotkeysDialogShowing, showHotkeysDialog } from "./hotkeysDialog"; -const SHOW_DIALOG_KEY_COMBO = parseKeyCombo("?"); +const SHOW_DIALOG_KEY = "?"; export enum HotkeyScope { LOCAL, @@ -59,7 +59,7 @@ export class HotkeysEvents { const combo = getKeyCombo(e); - if (comboMatches(SHOW_DIALOG_KEY_COMBO, combo)) { + if (comboMatches(parseKeyCombo(SHOW_DIALOG_KEY), combo)) { showHotkeysDialog(this.actions.map((action) => action.props)); return; } From 69568c31c5a9ed7448903a29143214ee9f3f68d2 Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 21:11:26 -0500 Subject: [PATCH 05/11] Check that 'document.documentElement.clientWidth' exists and is a number before attempting to use it. Fixes server-side rendering. --- packages/core/src/components/menu/menuItem.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/menu/menuItem.tsx b/packages/core/src/components/menu/menuItem.tsx index 8da41b9b68..a3beff2639 100644 --- a/packages/core/src/components/menu/menuItem.tsx +++ b/packages/core/src/components/menu/menuItem.tsx @@ -176,7 +176,13 @@ export class MenuItem extends AbstractComponent } let { left = 0, right = 0 } = this.props.submenuViewportMargin; - right = document.documentElement.clientWidth - right; + if (typeof document != 'undefined' + && typeof document.documentElement != 'undefined' + && Number(document.documentElement.clientWidth)) { + // we're in a browser context and the clientWidth is available, + // use it to set calculate 'right' + right = document.documentElement.clientWidth - right; + } // uses context to prioritize the previous positioning let alignLeft = this.context.alignLeft || false; if (alignLeft) { From f77bd1a30669157b7df96103ba6677ea55b116b4 Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 20:23:57 -0500 Subject: [PATCH 06/11] Move event bindings to 'componentDidMount', which fixes server-side rendering, since 'componentDidMount' never gets called on the server ('componentWillMount' does). --- .../core/src/components/hotkeys/hotkeysTarget.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeysTarget.tsx b/packages/core/src/components/hotkeys/hotkeysTarget.tsx index 3d98ff1cd8..14fbc146dc 100644 --- a/packages/core/src/components/hotkeys/hotkeysTarget.tsx +++ b/packages/core/src/components/hotkeys/hotkeysTarget.tsx @@ -26,7 +26,7 @@ export interface IHotkeysTarget extends React.Component, React.Compone } export function HotkeysTarget(constructor: T) { - const { componentWillMount, componentWillUnmount, render, renderHotkeys } = constructor.prototype; + const { componentWillMount, componentDidMount, componentWillUnmount, render, renderHotkeys } = constructor.prototype; if (!isFunction(renderHotkeys)) { throw new Error(`@HotkeysTarget-decorated class must implement \`renderHotkeys\`. ${constructor}`); @@ -37,12 +37,19 @@ export function HotkeysTarget(construct this.localHotkeysEvents = new HotkeysEvents(HotkeyScope.LOCAL); this.globalHotkeysEvents = new HotkeysEvents(HotkeyScope.GLOBAL); + if (componentWillMount != null) { + componentWillMount.call(this); + } + }; + + // tslint:disable no-invalid-this only-arrow-functions + constructor.prototype.componentDidMount = function() { // attach global key event listeners document.addEventListener("keydown", this.globalHotkeysEvents.handleKeyDown); document.addEventListener("keyup", this.globalHotkeysEvents.handleKeyUp); - if (componentWillMount != null) { - componentWillMount.call(this); + if (componentDidMount != null) { + componentDidMount.call(this); } }; From dd490e943ed731d6b758544a6159a4e682306105 Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 22:47:44 -0500 Subject: [PATCH 07/11] Return an void/noop Manager in a server context. Makes FocusStyleManager more useable by not exploding when it called during the server lifecycle. --- packages/core/src/accessibility/focusStyleManager.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/core/src/accessibility/focusStyleManager.ts b/packages/core/src/accessibility/focusStyleManager.ts index fdca23035e..6fd55abcb9 100644 --- a/packages/core/src/accessibility/focusStyleManager.ts +++ b/packages/core/src/accessibility/focusStyleManager.ts @@ -9,7 +9,14 @@ import { InteractionModeEngine } from "../common/interactionMode"; export const FOCUS_DISABLED_CLASS = "pt-focus-disabled"; -const focusEngine = new InteractionModeEngine(typeof document != 'undefined' ? document.documentElement : null, FOCUS_DISABLED_CLASS); +const fakeFocusEngine = { + stop: () => true, + isActive: () => true, + start: () => true +}; +const focusEngine = typeof document != 'undefined' + ? new InteractionModeEngine(document.documentElement, FOCUS_DISABLED_CLASS) + : fakeFocusEngine; // this is basically meaningless to unit test; it requires manual UI testing /* istanbul ignore next */ From 828acbee23d82aabc49a98d5d186bc91bb292ea9 Mon Sep 17 00:00:00 2001 From: Code Bling Date: Mon, 12 Dec 2016 23:51:54 -0500 Subject: [PATCH 08/11] Fix lint --- packages/core/src/accessibility/focusStyleManager.ts | 6 +++--- packages/core/src/components/hotkeys/hotkeyParser.ts | 2 +- packages/core/src/components/hotkeys/hotkeysTarget.tsx | 8 +++++++- packages/core/src/components/index.ts | 9 +++++---- packages/core/src/components/menu/menuItem.tsx | 4 ++-- 5 files changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/core/src/accessibility/focusStyleManager.ts b/packages/core/src/accessibility/focusStyleManager.ts index 6fd55abcb9..c5f4119169 100644 --- a/packages/core/src/accessibility/focusStyleManager.ts +++ b/packages/core/src/accessibility/focusStyleManager.ts @@ -10,11 +10,11 @@ import { InteractionModeEngine } from "../common/interactionMode"; export const FOCUS_DISABLED_CLASS = "pt-focus-disabled"; const fakeFocusEngine = { - stop: () => true, isActive: () => true, - start: () => true + start: () => true, + stop: () => true, }; -const focusEngine = typeof document != 'undefined' +const focusEngine = typeof document !== "undefined" ? new InteractionModeEngine(document.documentElement, FOCUS_DISABLED_CLASS) : fakeFocusEngine; diff --git a/packages/core/src/components/hotkeys/hotkeyParser.ts b/packages/core/src/components/hotkeys/hotkeyParser.ts index 31fb50fb2a..488999d892 100644 --- a/packages/core/src/components/hotkeys/hotkeyParser.ts +++ b/packages/core/src/components/hotkeys/hotkeyParser.ts @@ -112,7 +112,7 @@ export const Aliases = { command: "meta", escape: "esc", minus: "-", - mod: ((typeof navigator != 'undefined') && /Mac|iPod|iPhone|iPad/.test(navigator.platform)) ? "meta" : "ctrl", + mod: ((typeof navigator !== "undefined") && /Mac|iPod|iPhone|iPad/.test(navigator.platform)) ? "meta" : "ctrl", option: "alt", plus: "+", return: "enter", diff --git a/packages/core/src/components/hotkeys/hotkeysTarget.tsx b/packages/core/src/components/hotkeys/hotkeysTarget.tsx index 14fbc146dc..4135659570 100644 --- a/packages/core/src/components/hotkeys/hotkeysTarget.tsx +++ b/packages/core/src/components/hotkeys/hotkeysTarget.tsx @@ -26,7 +26,13 @@ export interface IHotkeysTarget extends React.Component, React.Compone } export function HotkeysTarget(constructor: T) { - const { componentWillMount, componentDidMount, componentWillUnmount, render, renderHotkeys } = constructor.prototype; + const { + componentWillMount, + componentDidMount, + componentWillUnmount, + render, + renderHotkeys, + } = constructor.prototype; if (!isFunction(renderHotkeys)) { throw new Error(`@HotkeysTarget-decorated class must implement \`renderHotkeys\`. ${constructor}`); diff --git a/packages/core/src/components/index.ts b/packages/core/src/components/index.ts index dc64d21588..2a7aa9ea61 100644 --- a/packages/core/src/components/index.ts +++ b/packages/core/src/components/index.ts @@ -5,10 +5,11 @@ * and https://github.com/palantir/blueprint/blob/master/PATENTS */ -declare function require(moduleName: string): any; //declare node.js 'require' so that we can conditionally import -if (typeof window != 'undefined' && typeof document != 'undefined') { //we're in browser - require("dom4"); //only import actual dom4 if we're in the browser (not server-compatible) - //we'll still need dom4 types for the TypeScript to compile, these are included in package.json +declare function require(moduleName: string): any; // declare node.js "require" so that we can conditionally import +if (typeof window !== "undefined" && typeof document !== "undefined") { // we're in browser + // tslint:disable-next-line:no-var-requires + require("dom4"); // only import actual dom4 if we're in the browser (not server-compatible) + // we'll still need dom4 types for the TypeScript to compile, these are included in package.json } import * as contextMenu from "./context-menu/contextMenu"; diff --git a/packages/core/src/components/menu/menuItem.tsx b/packages/core/src/components/menu/menuItem.tsx index a3beff2639..c1bbdd66c9 100644 --- a/packages/core/src/components/menu/menuItem.tsx +++ b/packages/core/src/components/menu/menuItem.tsx @@ -176,8 +176,8 @@ export class MenuItem extends AbstractComponent } let { left = 0, right = 0 } = this.props.submenuViewportMargin; - if (typeof document != 'undefined' - && typeof document.documentElement != 'undefined' + if (typeof document !== "undefined" + && typeof document.documentElement !== "undefined" && Number(document.documentElement.clientWidth)) { // we're in a browser context and the clientWidth is available, // use it to set calculate 'right' From df12c62b116e60bac2e08141212853b691fb00fe Mon Sep 17 00:00:00 2001 From: Code Bling Date: Tue, 13 Dec 2016 00:16:33 -0500 Subject: [PATCH 09/11] Don't calculate code-coverage for this file, since functionality is so trivial as to render extended testing useless. --- packages/core/src/accessibility/focusStyleManager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/accessibility/focusStyleManager.ts b/packages/core/src/accessibility/focusStyleManager.ts index c5f4119169..77191a2186 100644 --- a/packages/core/src/accessibility/focusStyleManager.ts +++ b/packages/core/src/accessibility/focusStyleManager.ts @@ -4,6 +4,7 @@ * of the license at https://github.com/palantir/blueprint/blob/master/LICENSE * and https://github.com/palantir/blueprint/blob/master/PATENTS */ +/* istanbul ignore next */ import { InteractionModeEngine } from "../common/interactionMode"; From 6a53b13b6e8309da0de9e53725c49faaa89aa72a Mon Sep 17 00:00:00 2001 From: Code Bling Date: Tue, 13 Dec 2016 00:23:49 -0500 Subject: [PATCH 10/11] Remove useless tslint hint --- packages/core/src/components/hotkeys/hotkeysTarget.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/components/hotkeys/hotkeysTarget.tsx b/packages/core/src/components/hotkeys/hotkeysTarget.tsx index 4135659570..cf3932a882 100644 --- a/packages/core/src/components/hotkeys/hotkeysTarget.tsx +++ b/packages/core/src/components/hotkeys/hotkeysTarget.tsx @@ -48,7 +48,6 @@ export function HotkeysTarget(construct } }; - // tslint:disable no-invalid-this only-arrow-functions constructor.prototype.componentDidMount = function() { // attach global key event listeners document.addEventListener("keydown", this.globalHotkeysEvents.handleKeyDown); From 97c0800ee04797dec96ea63f264936e8d94fe622 Mon Sep 17 00:00:00 2001 From: Code Bling Date: Tue, 13 Dec 2016 00:35:29 -0500 Subject: [PATCH 11/11] Ignore coverage checks on code that can't be meaningfully tested. --- packages/core/src/accessibility/focusStyleManager.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/accessibility/focusStyleManager.ts b/packages/core/src/accessibility/focusStyleManager.ts index 77191a2186..7a636781c0 100644 --- a/packages/core/src/accessibility/focusStyleManager.ts +++ b/packages/core/src/accessibility/focusStyleManager.ts @@ -4,17 +4,19 @@ * of the license at https://github.com/palantir/blueprint/blob/master/LICENSE * and https://github.com/palantir/blueprint/blob/master/PATENTS */ -/* istanbul ignore next */ import { InteractionModeEngine } from "../common/interactionMode"; export const FOCUS_DISABLED_CLASS = "pt-focus-disabled"; +/* istanbul ignore next */ const fakeFocusEngine = { isActive: () => true, start: () => true, stop: () => true, }; + +/* istanbul ignore next */ const focusEngine = typeof document !== "undefined" ? new InteractionModeEngine(document.documentElement, FOCUS_DISABLED_CLASS) : fakeFocusEngine;