From 6bbb57cb00d233f019aa7fd46228c75f02da532f Mon Sep 17 00:00:00 2001 From: Jon Espen Kvisler Date: Wed, 29 Nov 2017 22:44:55 +0100 Subject: [PATCH] proper aria handling --- .storybook/styled.js | 49 ++++++++++++++----------- .storybook/table.js | 20 ++++++++--- .storybook/with-sorting.js | 2 +- src/index.js | 54 +++++++++++++++++----------- src/types.js | 28 +++++++++------ test.js | 74 +++++++++++++++++++++----------------- 6 files changed, 136 insertions(+), 91 deletions(-) diff --git a/.storybook/styled.js b/.storybook/styled.js index 9a65aaa..c7b8b3a 100644 --- a/.storybook/styled.js +++ b/.storybook/styled.js @@ -12,27 +12,35 @@ const Table = styled.table` `; const Tr = styled.tr``; const Td = styled.td` - padding: 0.75rem; + padding: ${props => (props.sortable ? 0 : "0.75rem")}; border-top: 1px solid #eee; `; const Th = Td.extend` text-align: left; - &:focus, - &:hover { - cursor: ${props => (props.sortable ? "pointer" : "default")}; - background-color: ${props => (props.sortable ? "#ffc" : "inherit")}; - } `.withComponent("th"); function getSortDirection(props) { - if (props.direction && props.visible) { - return props.direction === "asc" ? "↓" : "↑"; + switch (props.direction) { + case "ascending": + return "↓"; + case "descending": + return "↑"; + default: + return "↓↑"; } } + const Sort = styled.span` + display: block; + padding: 0.75rem; + &:focus, + &:hover { + cursor: pointer; + background-color: #ffc; + } &::after { content: "${getSortDirection}"; - width: .25rem; + width: .75rem; font-size: .75rem; color: #666; display: inline-block; @@ -43,23 +51,22 @@ function StyledTable({ rows, ...props }: { rows: Row[] }) { return ( { + render={({ + getTableProps, + getSortProps, + getSortButtonProps, + sortKey, + sortDirection + }) => { return ( - +
@@ -91,6 +98,6 @@ storiesOf("With styled-components", module) rows={initialRows} onSort={action("onSort")} initialSortKey="name" - initialSortDirection="asc" + initialSortDirection="ascending" /> )); diff --git a/.storybook/table.js b/.storybook/table.js index b7efb79..16b8b30 100644 --- a/.storybook/table.js +++ b/.storybook/table.js @@ -30,16 +30,26 @@ export function Table({ rows, ...props }: { rows: Row[] }) { return ( { + render={({ + getTableProps, + getSortProps, + getSortButtonProps, + sortKey, + sortDirection + }) => { return ( -
- Name - + Name - Age + Age Country (not sortable)
+
@@ -69,6 +79,6 @@ storiesOf("Table", module) rows={initialRows} onSort={action("onSort")} initialSortKey="name" - initialSortDirection="asc" + initialSortDirection="ascending" /> )); diff --git a/.storybook/with-sorting.js b/.storybook/with-sorting.js index 5d7e8c9..2062746 100644 --- a/.storybook/with-sorting.js +++ b/.storybook/with-sorting.js @@ -30,7 +30,7 @@ class WithSorting extends React.Component { const rows = this.state.rows.sort(compare(sortKey)); - if (sortDirection === "desc") { + if (sortDirection === "descending") { rows.reverse(); } diff --git a/src/index.js b/src/index.js index add9ce8..f66858b 100644 --- a/src/index.js +++ b/src/index.js @@ -9,18 +9,12 @@ import type { RenderProps } from "./types"; -export const ariaSortMap = { - asc: "ascending", - desc: "descending" -}; - -const sortDirections = [...Object.keys(ariaSortMap), null]; - export default class Unsort extends React.Component { props: Props; static defaultProps = { - onSort: () => {} + onSort: () => {}, + sortDirections: ["none", "ascending", "descending"] }; constructor(props: Props) { @@ -32,47 +26,65 @@ export default class Unsort extends React.Component { }; } - getSortDirection = (key: string): ?SortDirection => { + getNextSortDirection = (key: string): ?SortDirection => { + const { sortDirections } = this.props; + const { sortDirection } = this.state; + if (!sortDirection) { + return sortDirections[1]; + } // if sortKey is changed or not set, return default sortDirections if (this.state.sortKey !== key) { - return sortDirections[0]; + return sortDirections[1]; } - const sortDirectionIndex = sortDirections.indexOf(this.state.sortDirection); + const sortDirectionIndex = sortDirections.indexOf(sortDirection); return sortDirections[(sortDirectionIndex + 1) % sortDirections.length]; }; handleSortChange = (key: string) => { - const sortDirection = this.getSortDirection(key); + const sortDirection = this.getNextSortDirection(key); const sortKey = sortDirection === null ? null : key; const newState = { sortKey, sortDirection }; this.props.onSort(newState); this.setState(newState); }; - getSortProps = (key: string): SortProps => { + getSortDirection = (key: string): SortDirection => { + const { sortDirections } = this.props; const { sortKey, sortDirection } = this.state; + return key === sortKey && sortDirection ? sortDirection : sortDirections[0]; + }; + + getSortProps = (key: string): SortProps => { + return { + ["aria-sort"]: this.getSortDirection(key) + }; + }; - const sortProps: SortProps = { + getTableProps = () => { + return { role: "grid" }; + }; + + getSortButtonProps = (key: string) => { + return { role: "button", tabIndex: 0, onClick: () => this.handleSortChange(key), - onKeyUp: event => { + onKeyUp: (event: SyntheticKeyboardEvent<*>) => { // 13 = enter keyCode if (event.keyCode === 13) { this.handleSortChange(key); } - } + }, + direction: this.getSortDirection(key) }; - if (key === sortKey && sortDirection) { - sortProps["aria-sort"] = ariaSortMap[sortDirection]; - } - return sortProps; }; render() { const renderProps: RenderProps = { ...this.state, - getSortProps: this.getSortProps + getSortProps: this.getSortProps, + getTableProps: this.getTableProps, + getSortButtonProps: this.getSortButtonProps }; return this.props.render(renderProps); } diff --git a/src/types.js b/src/types.js index 90f4ba3..621a292 100644 --- a/src/types.js +++ b/src/types.js @@ -2,24 +2,30 @@ import { type Node as ReactNode } from "react"; -export type SortDirection = "asc" | "desc"; +export type SortDirection = "ascending" | "descending" | "none" | "other"; -type AriaSortDirection = "ascending" | "descending"; - -export type SortProps = {| - role: "button", - tabIndex: 0, - "aria-sort"?: AriaSortDirection, - onClick: (event: SyntheticMouseEvent<*>) => void, - onKeyUp: (event: SyntheticKeyboardEvent<*>) => void -|}; +export type SortProps = { + "aria-sort": SortDirection +}; export type RenderProps = State & { - getSortProps: (key: string) => SortProps + getSortProps: (key: string) => SortProps, + getTableProps: () => {| + role: "grid" + |}, + getSortButtonProps: ( + key: string + ) => { + role: "button", + tabIndex: 0, + onClick: (event: SyntheticMouseEvent<*>) => void, + onKeyUp: (event: SyntheticKeyboardEvent<*>) => void + } }; export type Props = { render: RenderProps => ReactNode, + sortDirections: Array, initialSortKey?: string, initialSortDirection?: SortDirection, onSort: State => void diff --git a/test.js b/test.js index b8daae9..b796b7a 100644 --- a/test.js +++ b/test.js @@ -7,16 +7,12 @@ import * as React from "react"; import renderer from "react-test-renderer"; import { mount } from "enzyme"; -import Unsort, { ariaSortMap } from "./src"; +import Unsort from "./src"; test("exist", () => { expect(typeof Unsort).toBe("function"); }); -test("export ariaSortMap", () => { - expect(ariaSortMap).toBeDefined(); -}); - test("render prop renders content", () => { const json = renderer .create( {}} render={() =>
- Name {sortKey === "name" && sortDirection} + + Name {sortKey === "name" && sortDirection} + - Age {sortKey === "age" && sortDirection} + + Age {sortKey === "age" && sortDirection} + Country (not sortable)
} />) @@ -24,94 +20,108 @@ test("render prop renders content", () => { expect(json).toMatchSnapshot(); }); -describe("getSortProps", () => { +describe("getTableProps", () => { + const { getTableProps } = setup(); + expect(getTableProps()).toHaveProperty("role", "grid"); +}); + +describe("getSortButtonProps", () => { + const { getSortButtonProps } = setup(); + expect(getSortButtonProps()).toHaveProperty("role", "button"); + expect(getSortButtonProps()).toHaveProperty("tabIndex", 0); +}); + +describe("getSortButtonProps", () => { + let getSortButtonProps; let getSortProps; let mockOnSort = jest.fn(); beforeEach(() => { - getSortProps = setup({ + const s = setup({ onSort: mockOnSort, initialSortKey: "foo", - initialSortDirection: "asc" - }).getSortProps; + initialSortDirection: "ascending" + }); + getSortProps = s.getSortProps; + getSortButtonProps = s.getSortButtonProps; }); afterEach(() => { mockOnSort.mockClear(); }); test("sets tabIndex", () => { - expect(getSortProps("foo").tabIndex).toBe(0); + expect(getSortButtonProps("foo").tabIndex).toBe(0); }); test("onClick triggers onSort", () => { const fakeEvent = { target: null }; - const { onClick } = getSortProps("foo"); + const { onClick } = getSortButtonProps("foo"); onClick(fakeEvent); expect(mockOnSort).toHaveBeenCalledTimes(1); expect(mockOnSort).toHaveBeenCalledWith({ sortKey: "foo", - sortDirection: "desc" + sortDirection: "descending" }); }); test("onKeyUp triggers onSort if Enter have been pressed", () => { const fakeEvent = { target: null, keyCode: 13 }; - const { onKeyUp } = getSortProps("foo"); + const { onKeyUp } = getSortButtonProps("foo"); onKeyUp(fakeEvent); expect(mockOnSort).toHaveBeenCalledTimes(1); expect(mockOnSort).toHaveBeenCalledWith({ sortKey: "foo", - sortDirection: "desc" + sortDirection: "descending" }); }); test("onSort sets sortDirection in the right order", () => { const fakeEvent = { target: null }; - const { onClick } = getSortProps("foo"); + const { onClick } = getSortButtonProps("foo"); onClick(fakeEvent); onClick(fakeEvent); onClick(fakeEvent); expect(mockOnSort).toHaveBeenCalledTimes(3); expect(mockOnSort.mock.calls).toEqual([ - [{ sortKey: "foo", sortDirection: "desc" }], - [{ sortKey: null, sortDirection: null }], - [{ sortKey: "foo", sortDirection: "asc" }] + [{ sortKey: "foo", sortDirection: "descending" }], + [{ sortKey: "foo", sortDirection: "none" }], + [{ sortKey: "foo", sortDirection: "ascending" }] ]); }); describe("aria-sort", () => { test("sets aria-sort if key is sorted", () => { expect(getSortProps("foo")["aria-sort"]).toBe("ascending"); }); - test("does not set aria-sort if key is not sorted", () => { - expect(getSortProps("bar")["aria-sort"]).toBeUndefined(); + test("sets aria-sort to none if the sort direction is not set", () => { + expect(getSortProps("bar")["aria-sort"]).toBe("none"); }); }); }); test("onClick should go to next sortDirection", () => { - const { getSortProps, renderSpy } = setup({ + const { getSortButtonProps, renderSpy } = setup({ initialSortKey: "foo", - initialSortDirection: "asc" + initialSortDirection: "ascending" }); - const { onClick } = getSortProps("foo"); + const { onClick } = getSortButtonProps("foo"); onClick(); expect(renderSpy).toHaveBeenLastCalledWith( - expect.objectContaining({ sortDirection: "desc" }) + expect.objectContaining({ sortDirection: "descending" }) ); onClick(); expect(renderSpy).toHaveBeenLastCalledWith( - expect.objectContaining({ sortDirection: null, sortKey: null }) + expect.objectContaining({ sortDirection: "none", sortKey: "foo" }) ); onClick(); expect(renderSpy).toHaveBeenLastCalledWith( - expect.objectContaining({ sortDirection: "asc", sortKey: "foo" }) + expect.objectContaining({ sortDirection: "ascending", sortKey: "foo" }) ); }); test("change sort key should reset sort direction", () => { - const { getSortProps, renderSpy } = setup({ + const { getSortButtonProps, renderSpy } = setup({ initialSortKey: "foo", - initialSortDirection: "asc" + initialSortDirection: "ascending" }); - const { onClick } = getSortProps("bar"); + const { onClick } = getSortButtonProps("bar"); onClick(); expect(renderSpy).toHaveBeenLastCalledWith( - expect.objectContaining({ sortDirection: "asc", sortKey: "bar" }) + expect.objectContaining({ sortDirection: "ascending", sortKey: "bar" }) ); }); @@ -126,9 +136,9 @@ test("sortKey is inherited from initialSortKey", () => { test("sortDirection is inherited from initialSortDirection", () => { const { sortDirection } = setup({ onSort: jest.fn(), - initialSortDirection: "asc" + initialSortDirection: "ascending" }); - expect(sortDirection).toBe("asc"); + expect(sortDirection).toBe("ascending"); }); function setup(