Skip to content

Commit

Permalink
Accessibility audit (#1385)
Browse files Browse the repository at this point in the history
Accessibility audit
  • Loading branch information
alexreardon authored Jul 4, 2019
2 parents 3d70ffc + 06774cc commit 85f1b6e
Show file tree
Hide file tree
Showing 27 changed files with 804 additions and 68 deletions.
28 changes: 28 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,31 @@ jobs:
path: cypress/videos
- store_artifacts:
path: cypress/screenshots
test-a11y:
docker:
- image: circleci/node:10.15.3-browsers
working_directory: ~/repo
steps:
- checkout

- restore_cache:
keys:
- v7-dependencies-{{ checksum "yarn.lock" }}

# PR's from forks cannot use the dependency cache for performance reasons
- run:
name: 'Forked PR dependency install'
command: yarn

- run:
name: Accessibility Audit
command: node browser-test-harness.js yarn test:accessibility

- store_artifacts:
path: test-reports/lighthouse

- store_test_results:
path: test-reports/lighthouse
workflows:
version: 2
build:
Expand All @@ -150,3 +175,6 @@ workflows:
- test-browser:
requires:
- install
- test-a11y:
requires:
- install
22 changes: 11 additions & 11 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
{
"dist/react-beautiful-dnd.js": {
"bundled": 388572,
"minified": 146854,
"gzipped": 41757
"bundled": 392250,
"minified": 147969,
"gzipped": 41952
},
"dist/react-beautiful-dnd.min.js": {
"bundled": 327181,
"minified": 119986,
"gzipped": 35006
"bundled": 324263,
"minified": 118379,
"gzipped": 34043
},
"dist/react-beautiful-dnd.esm.js": {
"bundled": 226086,
"minified": 116450,
"gzipped": 31754,
"bundled": 227419,
"minified": 117230,
"gzipped": 31968,
"treeshaked": {
"rollup": {
"code": 21132,
"code": 21115,
"import_statements": 879
},
"webpack": {
"code": 24238
"code": 24221
}
}
}
Expand Down
1 change: 0 additions & 1 deletion .storybook/decorator/global-styles.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { colors } from '@atlaskit/theme';
import { grid } from '../../stories/src/constants';

const GlobalStyles = styled.div`
xbackground-color: ${colors.N0};
min-height: 100vh;
color: ${colors.N900};
`;
Expand Down
9 changes: 9 additions & 0 deletions .storybook/preview-head.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!--
Used to ensure there is a lang set on the html attribute for storybook
This is important for accessibility scores
-->
<script>
document.documentElement.setAttribute('lang', 'en');
</script>
23 changes: 23 additions & 0 deletions a11y-audit-parse.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
try {
const a11yReport = require('./test-reports/lighthouse/a11y.report.json');
const a11yScore = a11yReport.categories.accessibility.score;
const a11yScoreFormatted = `${a11yScore ? a11yScore * 100 : 0}%`;

console.log('*************************');
console.log('a11y score: ', a11yScoreFormatted);
console.log('*************************');

if (a11yScore === 1) {
// success!
process.exit(0);
} else {
// fail build
console.log(
'\nNOTE: Lighthouse accessibility audit score must be 100% to pass this build step.\n\n',
);
process.exit(1);
}
} catch (e) {
console.error(e);
process.exit(1);
}
4 changes: 2 additions & 2 deletions docs/api/draggable.md
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ type DragHandleProps = {|
// What DragDropContext the drag handle is in
'data-rbd-drag-handle-context-id': ContextId,

// Aria role (nicer screen reader text)
'aria-roledescription': string,
// Id of hidden element that contains the lift instruction (nicer screen reader text)
'aria-labelledby': ElementId,

// Allow tabbing to this element
tabIndex: number,
Expand Down
9 changes: 7 additions & 2 deletions docs/guides/screen-reader.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,24 @@ All of our built in screen reader messages use `id`'s to identify `<Draggable />

### Step 1: Introduce draggable item

When a user `tabs` to a `<Draggable />`, we need to tell them how to start a drag. We do this by using the `aria-roledescription` property on a _drag handle_.
When a user `tabs` to a _drag handle_, we need to tell them how to start a drag. We do this by using the `liftInstruction` property on a `<DragDropContext />`. All _drag handles_ share the same lift announcement message.

**Default message**: "Draggable item. Press space bar to lift"
**Default message**: "Draggable item. Ensure your screen reader is not in browse mode and then press spacebar to lift."

We tell the user the following:

- The item is draggable
- To disable _browse mode_
- How to start a drag

You don't need to give all the drag movement instructions at this point, let's wait until the user decides to start a drag.

Think about substituting the word "item" for a noun that matches your problem domain, for example, "task" or "issue". You might also want to drop the word "item" altogether.

#### Disabling browse mode

Screen readers can run in [various modes](https://www.accessibility-developer-guide.com/knowledge/desktop-screen-readers/browse-focus-modes/). In order for the keyboard shortcuts to work correctly the user needs to leave the _browse mode_ as it remaps a lot of keyboard shortcuts. Alternatively you could use `aria-role="application"` on the `<body>` element, but this can wreck the standard screen reader usage of your page.

### Step 2: Start drag

When a user lifts a `<Draggable />` by using the `spacebar` we want to tell them a number of things.
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"prettier_target": "*.{js,jsx,md,json} src/**/*.{js,jsx,md,json} test/**/*.{js,jsx,md,json} docs/**/*.{js,jsx,md,json} stories/**/*.{js,jsx,md,json} cypress/**/*.{js,jsx,md,json}"
},
"scripts": {
"test:accessibility": "lighthouse http://localhost:9002/iframe.html?id=single-vertical-list--basic --chrome-flags='--headless' --output=json --output=html --output-path=./test-reports/lighthouse/a11y.json && node a11y-audit-parse.js",
"test": "jest --config ./jest.config.js",
"test:ci": "jest test --maxWorkers=2",
"test:browser": "cypress open",
Expand Down Expand Up @@ -109,6 +110,7 @@
"jest": "^24.8.0",
"jest-junit": "^6.4.0",
"jest-watch-typeahead": "^0.3.1",
"lighthouse": "^4.3.1",
"markdown-it": "^8.4.2",
"prettier": "^1.18.2",
"raf-stub": "^2.0.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ import type {
DropResult,
DraggableLocation,
Combine,
} from '../../../types';
} from './types';

export type MessagePreset = {|
liftInstruction: string,
onDragStart: (start: DragStart) => string,
onDragUpdate: (update: DragUpdate) => string,
onDragEnd: (result: DropResult) => string,
|};

const liftInstruction: string = `Draggable item. Ensure your screen reader is not in browse mode and then press spacebar to lift.`;

const position = (index: number): number => index + 1;

// We cannot list what index the Droppable is in automatically as we are not sure how
Expand Down Expand Up @@ -119,6 +122,7 @@ const onDragEnd = (result: DropResult): string => {
};

const preset: MessagePreset = {
liftInstruction,
onDragStart,
onDragUpdate,
onDragEnd,
Expand Down
2 changes: 1 addition & 1 deletion src/state/middleware/responders/publisher.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import invariant from 'tiny-invariant';
import messagePreset from '../util/screen-reader-message-preset';
import messagePreset from '../../../screen-reader-message-preset';
import * as timings from '../../../debug/timings';
import getExpiringAnnounce, {
type ExpiringAnnounce,
Expand Down
1 change: 1 addition & 0 deletions src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export type DraggableId = Id;
export type DroppableId = Id;
export type TypeId = Id;
export type ContextId = Id;
export type ElementId = Id;

export type DroppableMode = 'STANDARD' | 'VIRTUAL';
export type DroppableDescriptor = {|
Expand Down
3 changes: 2 additions & 1 deletion src/view/context/app-context.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import React from 'react';
import type { DraggableId, ContextId } from '../../types';
import type { DraggableId, ContextId, ElementId } from '../../types';
import type { DimensionMarshal } from '../../state/dimension-marshal/dimension-marshal-types';
import type { FocusMarshal } from '../use-focus-marshal/focus-marshal-types';

Expand All @@ -10,6 +10,7 @@ export type AppContextValue = {|
contextId: ContextId,
canLift: (id: DraggableId) => boolean,
isMovementAllowed: () => boolean,
liftInstructionId: ElementId,
|};

export default React.createContext<?AppContextValue>(null);
18 changes: 14 additions & 4 deletions src/view/drag-drop-context/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import type {
Responders,
Announce,
Sensor,
ElementId,
} from '../../types';
import type { Store, Action } from '../../state/store-types';
import StoreContext from '../context/store-context';
Expand All @@ -38,6 +39,7 @@ import {
} from '../../state/action-creators';
import isMovementAllowed from '../../state/is-movement-allowed';
import useAnnouncer from '../use-announcer';
import useDragHandleDescription from '../use-lift-instruction';
import AppContext, { type AppContextValue } from '../context/app-context';
import useStartupValidation from './use-startup-validation';
import usePrevious from '../use-previous-ref';
Expand All @@ -54,6 +56,7 @@ type Props = {|
// sensors
sensors?: Sensor[],
enableDefaultSensors?: ?boolean,
liftInstruction: string,
|};

const createResponders = (props: Props): Responders => ({
Expand All @@ -73,7 +76,7 @@ function getStore(lazyRef: LazyStoreRef): Store {
}

export default function App(props: Props) {
const { contextId, setOnError, sensors } = props;
const { contextId, setOnError, sensors, liftInstruction } = props;
const lazyStoreRef: LazyStoreRef = useRef<?Store>(null);

useStartupValidation();
Expand All @@ -86,6 +89,11 @@ export default function App(props: Props) {
}, [lastPropsRef]);

const announce: Announce = useAnnouncer(contextId);

const liftInstructionId: ElementId = useDragHandleDescription(
contextId,
liftInstruction,
);
const styleMarshal: StyleMarshal = useStyleMarshal(contextId);

const lazyDispatch: Action => void = useCallback((action: Action): void => {
Expand Down Expand Up @@ -133,12 +141,12 @@ export default function App(props: Props) {
const store: Store = useMemo<Store>(
() =>
createStore({
dimensionMarshal,
focusMarshal,
styleMarshal,
announce,
autoScroller,
dimensionMarshal,
focusMarshal,
getResponders,
styleMarshal,
}),
[
announce,
Expand Down Expand Up @@ -189,13 +197,15 @@ export default function App(props: Props) {
contextId,
canLift: getCanLift,
isMovementAllowed: getIsMovementAllowed,
liftInstructionId,
}),
[
contextId,
dimensionMarshal,
focusMarshal,
getCanLift,
getIsMovementAllowed,
liftInstructionId,
],
);

Expand Down
11 changes: 10 additions & 1 deletion src/view/drag-drop-context/drag-drop-context.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, { type Node } from 'react';
import { useMemo } from 'use-memo-one';
import type { Responders, ContextId, Sensor } from '../../types';
import ErrorBoundary from '../error-boundary';
import preset from '../../screen-reader-message-preset';
import App from './app';

type Props = {|
Expand All @@ -12,6 +13,7 @@ type Props = {|

sensors?: Sensor[],
enableDefaultSensors?: ?boolean,
liftInstruction?: string,
|};

let instanceCount: number = 0;
Expand All @@ -23,13 +25,20 @@ export function resetServerContext() {

export default function DragDropContext(props: Props) {
const contextId: ContextId = useMemo(() => `${instanceCount++}`, []);
const liftInstruction: string =
props.liftInstruction || preset.liftInstruction;

// We need the error boundary to be on the outside of App
// so that it can catch any errors caused by App
return (
<ErrorBoundary>
{setOnError => (
<App setOnError={setOnError} contextId={contextId} {...props}>
<App
setOnError={setOnError}
contextId={contextId}
liftInstruction={liftInstruction}
{...props}
>
{props.children}
</App>
)}
Expand Down
5 changes: 3 additions & 2 deletions src/view/draggable/draggable-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
State,
MovementMode,
ContextId,
ElementId,
} from '../../types';
import { dropAnimationFinished } from '../../state/action-creators';

Expand Down Expand Up @@ -66,8 +67,8 @@ export type DragHandleProps = {|
// What DragDropContext the drag handle is in
'data-rbd-drag-handle-context-id': ContextId,

// Aria role (nicer screen reader text)
'aria-roledescription': string,
// id of drag handle aria description for screen readers
'aria-labelledby': ElementId,

// Allow tabbing to this element
tabIndex: number,
Expand Down
7 changes: 3 additions & 4 deletions src/view/draggable/draggable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export default function Draggable(props: Props) {
const getRef = useCallback((): ?HTMLElement => ref.current, []);

// context
const { contextId } = useRequiredContext(AppContext);
const { contextId, liftInstructionId } = useRequiredContext(AppContext);

// props
const {
Expand Down Expand Up @@ -80,14 +80,13 @@ export default function Draggable(props: Props) {
tabIndex: 0,
'data-rbd-drag-handle-draggable-id': draggableId,
'data-rbd-drag-handle-context-id': contextId,
// English default. Consumers are welcome to add their own start instruction
'aria-roledescription': 'Draggable item. Press space bar to lift',
'aria-labelledby': liftInstructionId,
// Opting out of html5 drag and drops
draggable: false,
onDragStart: preventHtml5Dnd,
}
: null,
[contextId, draggableId, isEnabled],
[contextId, draggableId, isEnabled, liftInstructionId],
);

const onMoveEnd = useCallback(
Expand Down
Loading

0 comments on commit 85f1b6e

Please sign in to comment.