Skip to content

Commit

Permalink
Consistent space and enter keystrokes between buttons and anchor butt…
Browse files Browse the repository at this point in the history
…ons (palantir#459)

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

* create `AbstractButton` class that implementations extend so code is easily shared.
  • Loading branch information
leebyp authored and giladgray committed Jan 13, 2017
1 parent fc0e8d4 commit 6a410c5
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 24 deletions.
61 changes: 61 additions & 0 deletions packages/core/src/components/button/abstractButton.tsx
Original file line number Diff line number Diff line change
@@ -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();
}
}
}
39 changes: 15 additions & 24 deletions packages/core/src/components/button/buttons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand All @@ -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() {
Expand All @@ -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)}
Expand All @@ -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,
},
Expand Down
14 changes: 14 additions & 0 deletions packages/core/test/buttons/buttonTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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:", () => {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6a410c5

Please sign in to comment.