From 6a410c5129dee74ab4567d5caf1a7c305e9f61e3 Mon Sep 17 00:00:00 2001
From: Benjamin Lee <leebyp@gmail.com>
Date: Fri, 13 Jan 2017 10:54:05 -0800
Subject: [PATCH] Consistent space and enter keystrokes between buttons and
 anchor buttons (#459)

* have consistent actions with space and enter between buttons and anchorbuttons

* create `AbstractButton` class that implementations extend so code is easily shared.
---
 .../src/components/button/abstractButton.tsx  | 61 +++++++++++++++++++
 .../core/src/components/button/buttons.tsx    | 39 +++++-------
 packages/core/test/buttons/buttonTests.tsx    | 14 +++++
 3 files changed, 90 insertions(+), 24 deletions(-)
 create mode 100644 packages/core/src/components/button/abstractButton.tsx

diff --git a/packages/core/src/components/button/abstractButton.tsx b/packages/core/src/components/button/abstractButton.tsx
new file mode 100644
index 0000000000..65cb256aff
--- /dev/null
+++ b/packages/core/src/components/button/abstractButton.tsx
@@ -0,0 +1,61 @@
+import * as React from "react";
+
+import * as Keys from "../../common/keys";
+import { IActionProps } from "../../common/props";
+import { safeInvoke } from "../../common/utils";
+
+export interface IButtonProps extends IActionProps {
+    /** A ref handler that receives the native HTML element backing this component. */
+    elementRef?: (ref: HTMLElement) => any;
+
+    /** Name of icon (the part after `pt-icon-`) to add to button. */
+    rightIconName?: string;
+
+    /**
+     * If set to true, the button will display a centered loading spinner instead of its contents.
+     * The width of the button is not affected by the value of this prop.
+     * @default false
+     */
+    loading?: boolean;
+}
+
+export interface IButtonState {
+    isActive: boolean;
+}
+
+export abstract class AbstractButton<T> extends React.Component<React.HTMLProps<T> & IButtonProps, IButtonState> {
+    public state = {
+        isActive: false,
+    };
+
+    protected buttonRef: HTMLElement;
+    protected refHandlers = {
+        button: (ref: HTMLElement) => {
+            this.buttonRef = ref;
+            safeInvoke(this.props.elementRef, ref);
+        },
+    };
+
+    public abstract render(): JSX.Element;
+
+    protected onKeyDown = (e: React.KeyboardEvent<HTMLElement>) => {
+        switch (e.which) {
+            case Keys.SPACE:
+                e.preventDefault();
+                this.setState({ isActive: true });
+                break;
+            case Keys.ENTER:
+                this.buttonRef.click();
+                break;
+            default:
+                break;
+        }
+    }
+
+    protected onKeyUp = (e: React.KeyboardEvent<HTMLElement>) => {
+        if (e.which === Keys.SPACE) {
+            this.setState({ isActive: false });
+            this.buttonRef.click();
+        }
+    }
+}
diff --git a/packages/core/src/components/button/buttons.tsx b/packages/core/src/components/button/buttons.tsx
index ce470bd5c1..54eaa57de7 100644
--- a/packages/core/src/components/button/buttons.tsx
+++ b/packages/core/src/components/button/buttons.tsx
@@ -12,38 +12,26 @@ import * as classNames from "classnames";
 import * as React from "react";
 
 import * as Classes from "../../common/classes";
-import { IActionProps, removeNonHTMLProps } from "../../common/props";
+import { removeNonHTMLProps } from "../../common/props";
 import { Spinner } from "../spinner/spinner";
+import { AbstractButton, IButtonProps } from "./abstractButton";
 
-export interface IButtonProps extends IActionProps {
-    /** A ref handler that receives the native HTML element backing this component. */
-    elementRef?: (ref: HTMLElement) => any;
-
-    /** Name of icon (the part after `pt-icon-`) to add to button. */
-    rightIconName?: string;
-
-    /**
-     * If set to true, the button will display a centered loading spinner instead of its contents.
-     * The width of the button is not affected by the value of this prop.
-     * @default false
-     */
-    loading?: boolean;
-}
-
-export class Button extends React.Component<React.HTMLProps<HTMLButtonElement> & IButtonProps, {}> {
+export class Button extends AbstractButton<HTMLButtonElement> {
     public static displayName = "Blueprint.Button";
 
     public render() {
-        const { children, elementRef, loading, onClick, rightIconName, text } = this.props;
+        const { children, loading, onClick, rightIconName, text } = this.props;
         const disabled = isButtonDisabled(this.props);
 
         return (
             <button
                 type="button"
                 {...removeNonHTMLProps(this.props)}
-                className={getButtonClasses(this.props)}
+                className={getButtonClasses(this.props, this.state.isActive)}
                 onClick={disabled ? undefined : onClick}
-                ref={elementRef}
+                onKeyDown={this.onKeyDown}
+                onKeyUp={this.onKeyUp}
+                ref={this.refHandlers.button}
             >
                 {maybeRenderSpinner(loading)}
                 {maybeRenderText(text)}
@@ -56,7 +44,7 @@ export class Button extends React.Component<React.HTMLProps<HTMLButtonElement> &
 
 export const ButtonFactory = React.createFactory(Button);
 
-export class AnchorButton extends React.Component<React.HTMLProps<HTMLAnchorElement> & IButtonProps, {}> {
+export class AnchorButton extends AbstractButton<HTMLButtonElement> {
     public static displayName = "Blueprint.AnchorButton";
 
     public render() {
@@ -67,10 +55,12 @@ export class AnchorButton extends React.Component<React.HTMLProps<HTMLAnchorElem
             <a
                 role="button"
                 {...removeNonHTMLProps(this.props)}
-                className={getButtonClasses(this.props)}
+                className={getButtonClasses(this.props, this.state.isActive)}
                 href={disabled ? undefined : href}
                 onClick={disabled ? undefined : onClick}
-                ref={this.props.elementRef}
+                onKeyDown={this.onKeyDown}
+                onKeyUp={this.onKeyUp}
+                ref={this.refHandlers.button}
                 tabIndex={disabled ? undefined : tabIndex}
             >
                 {maybeRenderSpinner(loading)}
@@ -84,9 +74,10 @@ export class AnchorButton extends React.Component<React.HTMLProps<HTMLAnchorElem
 
 export const AnchorButtonFactory = React.createFactory(AnchorButton);
 
-function getButtonClasses(props: IButtonProps) {
+function getButtonClasses(props: IButtonProps, isActive = false) {
     return classNames(
         Classes.BUTTON, {
+            [Classes.ACTIVE]: isActive,
             [Classes.DISABLED]: isButtonDisabled(props),
             [Classes.LOADING]: props.loading,
         },
diff --git a/packages/core/test/buttons/buttonTests.tsx b/packages/core/test/buttons/buttonTests.tsx
index d6c1988031..a944998b71 100644
--- a/packages/core/test/buttons/buttonTests.tsx
+++ b/packages/core/test/buttons/buttonTests.tsx
@@ -9,6 +9,7 @@ import { assert } from "chai";
 import { mount, shallow } from "enzyme";
 import * as React from "react";
 
+import * as Keys from "../../src/common/keys";
 import { AnchorButton, Button, Classes, IButtonProps, Spinner } from "../../src/index";
 
 describe("Buttons:", () => {
@@ -58,6 +59,19 @@ function buttonTestSuite(component: React.ComponentClass<any>, tagName: string)
             assert.equal(onClick.callCount, 0);
         });
 
+        it("calls onClick when enter key pressed", () => {
+            const onClick = sinon.spy();
+            button({ onClick }, true).simulate("keydown", { which: Keys.ENTER });
+            // wait for the whole lifecycle to run
+            setTimeout(() => assert.equal(onClick.callCount, 1), 0);
+        });
+
+        it("calls onClick when space key released", () => {
+            const onClick = sinon.spy();
+            button({ onClick }, true).simulate("keyup", { which: Keys.SPACE });
+            setTimeout(() => assert.equal(onClick.callCount, 1), 0);
+        });
+
         it("elementRef receives reference to HTML element", () => {
             const elementRef = sinon.spy();
             // full DOM rendering here so the ref handler is invoked