Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trying to fix lift announcing #1716

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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": 379875,
"minified": 139571,
"gzipped": 41966
"bundled": 380487,
"minified": 139848,
"gzipped": 42029
},
"dist/react-beautiful-dnd.min.js": {
"bundled": 321489,
"minified": 115006,
"gzipped": 34084
"bundled": 322101,
"minified": 115283,
"gzipped": 34154
},
"dist/react-beautiful-dnd.esm.js": {
"bundled": 241086,
"minified": 125714,
"gzipped": 32875,
"bundled": 241664,
"minified": 125997,
"gzipped": 32952,
"treeshaked": {
"rollup": {
"code": 21306,
"code": 21383,
"import_statements": 788
},
"webpack": {
"code": 24963
"code": 25040
}
}
}
Expand Down
18 changes: 17 additions & 1 deletion docs/guides/screen-reader.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

> Because great features should be accessible for everyone

`react-beautiful-dnd` ships with great screen reader support, in English, out of the box. If you just want to get started, then there's nothing you have to do. But if it's tailored messaging you're after, you have total control of that too.
`react-beautiful-dnd` ships with great screen reader support out of the box with basic English text. If you just want to get started, then there's nothing you have to do. But if it's tailored messaging you're after, you have total control of that too.

This guide is here to help you create messaging that supports and delights your users. The screen reader experience is focused on keyboard interactions, but it's possible for a screen reader user to use any input type (for example mouse and keyboard).

Expand Down Expand Up @@ -43,6 +43,22 @@ All of our built in screen reader messages use `id`'s to identify `<Draggable />

### Step 1: Introduce draggable item

Each element has a number of accessibility properties associated with it:

- name: a way of identifying the element
- role: type of element (eg "button")
- role description: a more specific description of the role
- description: additional information including [instructions](https://www.w3.org/TR/WCAG20-TECHS/ARIA1.html) for the element

A combination of these properties will control what a screen reader reads out when element is focused on.

Here is how we populate the accessibility properties for _drag handles_ to impact what is read out

- **name**: (We do not control this). By default the content of the element will be used. Otherwise you can use `aria-label` or `aria-labelledby`.
- **role**: (Default: we add `role="button"` to a drag handle). This is done so that the element is marked as *interactive* and will have information read out. `role="button"` might not be the right role for your element, or it might already be an interactive element. In which case you are welcome to remove or override the `role`.
- **role description**: (We do not control this). Role description controlled by adding `aria-roledescription`. Ideally we would add `aria-roledescription="Draggable item"` by default. However, [Google lighthouse](https://developers.google.com/web/tools/lighthouse) marks this as a problem even though it is fine according to [axe and the spec](https://twitter.com/alexandereardon/status/1220563555797229568). So you are welcome to add a `aria-roledescription` but keep in mind that [Google lighthouse](https://developers.google.com/web/tools/lighthouse) will mark it as a accessibility problem
- **description**: (Default: `"Press space bar to lift"`). We control this by using `aria-describedby="${elementId}"`. We create a hidden element for each `<DragDropContext>` and populate the element with the `liftInstruction` text.

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` prop on a `<DragDropContext />`. All _drag handles_ share the same lift announcement message.

**Default message**: "Draggable item. Ensure your screen reader is not in browse mode and then press spacebar to lift."
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
"fs-extra": "^8.0.1",
"globby": "^10.0.1",
"jest": "^24.9.0",
"jest-axe": "^3.2.0",
"jest-axe": "^3.3.0",
"jest-junit": "^9.0.0",
"jest-watch-typeahead": "^0.4.0",
"lighthouse": "^5.6.0",
Expand Down
2 changes: 1 addition & 1 deletion src/view/context/app-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export type AppContextValue = {|
contextId: ContextId,
canLift: (id: DraggableId) => boolean,
isMovementAllowed: () => boolean,
liftInstructionId: ElementId,
dragHandleInstructionId: ElementId,
marshal: DimensionMarshal,
registry: Registry,
|};
Expand Down
14 changes: 8 additions & 6 deletions src/view/drag-drop-context/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import {
} from '../../state/action-creators';
import isMovementAllowed from '../../state/is-movement-allowed';
import useAnnouncer from '../use-announcer';
import useLiftInstruction from '../use-lift-instruction';
import useHiddenElement from '../use-hidden-element';
import AppContext, { type AppContextValue } from '../context/app-context';
import useStartupValidation from './use-startup-validation';
import usePrevious from '../use-previous-ref';
Expand Down Expand Up @@ -97,10 +97,12 @@ export default function App(props: Props) {

const announce: Announce = useAnnouncer(contextId);

const liftInstructionId: ElementId = useLiftInstruction(
const dragHandleInstructionId: ElementId = useHiddenElement({
contextId,
liftInstruction,
);
uniqueKey: 'drag-handle-usage',
text: liftInstruction,
});

const styleMarshal: StyleMarshal = useStyleMarshal(contextId, nonce);

const lazyDispatch: Action => void = useCallback((action: Action): void => {
Expand Down Expand Up @@ -219,16 +221,16 @@ export default function App(props: Props) {
contextId,
canLift: getCanLift,
isMovementAllowed: getIsMovementAllowed,
liftInstructionId,
dragHandleInstructionId,
registry,
}),
[
contextId,
dimensionMarshal,
dragHandleInstructionId,
focusMarshal,
getCanLift,
getIsMovementAllowed,
liftInstructionId,
registry,
],
);
Expand Down
18 changes: 16 additions & 2 deletions src/view/draggable/draggable-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,22 @@ export type DragHandleProps = {|
// What DragDropContext the drag handle is in
'data-rbd-drag-handle-context-id': ContextId,

// id of drag handle aria description for screen readers
'aria-labelledby': ElementId,
// Accessibility property: role (type of the element)
// interactive element required for adding custom names / descriptions to be read out
// https://www.w3.org/TR/using-aria/#label-support
// Consumers should change the role or remove it if they want to use the
// elements default role or another role.
// We will be using role="button" initially
role: string,

// Accessibility property: role description
// Sadly not adding this as it does not play well with lighthouse
// https://twitter.com/alexandereardon/status/1220563555797229568
// 'aria-roledescription': string,

// Accessibility property: description (using instructions)
// id of element containing screen reader usage instructions
'aria-describedby': ElementId,

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

// context
const { contextId, liftInstructionId, registry } = useRequiredContext(
const { contextId, dragHandleInstructionId, registry } = useRequiredContext(
AppContext,
);
const { type, droppableId } = useRequiredContext(DroppableContext);
Expand Down Expand Up @@ -103,13 +103,23 @@ export default function Draggable(props: Props) {
tabIndex: 0,
'data-rbd-drag-handle-draggable-id': draggableId,
'data-rbd-drag-handle-context-id': contextId,
'aria-labelledby': liftInstructionId,
// need to force the item to be interative
// consumers are welcome to remove or change this
role: 'button',

// Sadly not giving a nicer role description
// Lighthouse currently complains about this as a a11y violation
// 'aria-roledescription': 'Draggable item',

// Adding the lift instruction as a description
'aria-describedby': dragHandleInstructionId,

// Opting out of html5 drag and drops
draggable: false,
onDragStart: preventHtml5Dnd,
}
: null,
[contextId, draggableId, isEnabled, liftInstructionId],
[contextId, dragHandleInstructionId, draggableId, isEnabled],
);

const onMoveEnd = useCallback(
Expand Down
2 changes: 2 additions & 0 deletions src/view/use-hidden-element/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// @flow
export { default } from './use-hidden-element';
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,24 @@ import { useMemo } from 'use-memo-one';
import type { ContextId, ElementId } from '../../types';
import getBodyElement from '../get-body-element';

export const getId = (contextId: ContextId): string =>
`rbd-lift-instruction-${contextId}`;
export const getId = (uniqueKey: string, contextId: ContextId): string =>
`rbd-hidden-${uniqueKey}-${contextId}`;

export default function useLiftInstruction(
type Args = {|
contextId: ContextId,
liftInstruction: string,
): ElementId {
const id: string = useMemo(() => getId(contextId), [contextId]);
uniqueKey: string,
text: string,
|};

export default function useHiddenElement({
contextId,
text,
uniqueKey,
}: Args): ElementId {
const id: string = useMemo(() => getId(uniqueKey, contextId), [
contextId,
uniqueKey,
]);

useEffect(
function mount() {
Expand All @@ -21,7 +31,7 @@ export default function useLiftInstruction(
el.id = id;

// add the description text
el.textContent = liftInstruction;
el.textContent = text;

// Using `display: none` prevent screen readers from reading this element in the document flow
// This element is used as a `aria-labelledby` reference for *other elements* and will be read out for those
Expand All @@ -35,7 +45,7 @@ export default function useLiftInstruction(
getBodyElement().removeChild(el);
};
},
[id, liftInstruction],
[id, text],
);

return id;
Expand Down
2 changes: 0 additions & 2 deletions src/view/use-lift-instruction/index.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`should support rendering to a string 1`] = `"<div data-rbd-droppable-id=\\"droppable\\" data-rbd-droppable-context-id=\\"0\\"><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"0\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"0\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"0\\">item: <!-- -->0</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"1\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"1\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"1\\">item: <!-- -->1</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"2\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"2\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"2\\">item: <!-- -->2</div></div>"`;
exports[`should support rendering to a string 1`] = `"<div data-rbd-droppable-id=\\"droppable\\" data-rbd-droppable-context-id=\\"0\\"><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"0\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"0\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" role=\\"button\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"0\\">item: <!-- -->0</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"1\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"1\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" role=\\"button\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"1\\">item: <!-- -->1</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"2\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"2\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" role=\\"button\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"2\\">item: <!-- -->2</div></div>"`;

exports[`should support rendering to static markup 1`] = `"<div data-rbd-droppable-id=\\"droppable\\" data-rbd-droppable-context-id=\\"0\\"><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"0\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"0\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"0\\">item: 0</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"1\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"1\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"1\\">item: 1</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"2\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"2\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"2\\">item: 2</div></div>"`;
exports[`should support rendering to static markup 1`] = `"<div data-rbd-droppable-id=\\"droppable\\" data-rbd-droppable-context-id=\\"0\\"><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"0\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"0\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" role=\\"button\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"0\\">item: 0</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"1\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"1\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" role=\\"button\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"1\\">item: 1</div><div data-rbd-draggable-context-id=\\"0\\" data-rbd-draggable-id=\\"2\\" tabindex=\\"0\\" data-rbd-drag-handle-draggable-id=\\"2\\" data-rbd-drag-handle-context-id=\\"0\\" aria-labelledby=\\"rbd-lift-instruction-0\\" role=\\"button\\" draggable=\\"false\\" data-is-dragging=\\"false\\" data-is-drop-animating=\\"false\\" data-is-combining=\\"false\\" data-is-combine-target=\\"false\\" data-is-clone=\\"false\\" data-testid=\\"2\\">item: 2</div></div>"`;
2 changes: 1 addition & 1 deletion test/unit/view/droppable/util/mount.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function App(props: AppProps) {
contextId,
canLift: () => true,
isMovementAllowed,
liftInstructionId: 'fake-id',
dragHandleInstructionId: 'fake-id',
marshal: getMarshalStub(),
registry: createRegistry(),
}),
Expand Down
2 changes: 1 addition & 1 deletion test/unit/view/use-droppable-publisher/util/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export function WithAppContext(props: WithAppContextProps) {
contextId: 'fake',
canLift: () => true,
isMovementAllowed: () => true,
liftInstructionId: '1',
dragHandleInstructionId: '1',
marshal: props.marshal || getMarshalStub(),
registry: props.registry,
}),
Expand Down
18 changes: 9 additions & 9 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2738,10 +2738,10 @@ [email protected]:
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-3.3.0.tgz#3b32d7e54390d89ff4891b20394d33ad7a192776"
integrity sha512-54XaTd2VB7A6iBnXMUG2LnBOI7aRbnrVxC5Tz+rVUwYl9MX/cIJc/Ll32YUoFIE/e9UKWMZoQenQu9dFrQyZCg==

axe-core@3.3.1:
version "3.3.1"
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-3.3.1.tgz#3d1fa78cca8ead1b78c350581501e4e37b97b826"
integrity sha512-gw1T0JptHPF4AdLLqE8yQq3Z7YvsYkpFmFWd84r6hnq/QoKRr8icYHFumhE7wYl5TVIHgVlchMyJsAYh0CfwCQ==
axe-core@3.4.1:
version "3.4.1"
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-3.4.1.tgz#e42623918bb85b5ef674633852cb9029db0309c5"
integrity sha512-+EhIdwR0hF6aeMx46gFDUy6qyCfsL0DmBrV3Z+LxYbsOd8e1zBaPHa3f9Rbjsz2dEwSBkLw6TwML/CAIIAqRpw==

axobject-query@^2.0.2:
version "2.0.2"
Expand Down Expand Up @@ -7129,12 +7129,12 @@ istanbul-reports@^2.2.6:
dependencies:
handlebars "^4.1.2"

jest-axe@^3.2.0:
version "3.2.0"
resolved "https://registry.yarnpkg.com/jest-axe/-/jest-axe-3.2.0.tgz#0f7a0132565289432936421cf38b7b8504690835"
integrity sha512-QSQwSwG72/cpmhJU0fBsaUUvu9mb2uAqhccGQVG6JbIV8sK+aIXh8hYl7vxraMF/I6soQod1aqSdD/j7LjpVFQ==
jest-axe@^3.3.0:
version "3.3.0"
resolved "https://registry.yarnpkg.com/jest-axe/-/jest-axe-3.3.0.tgz#ae37c25e26dfdadb9caf6122798a11d681015c03"
integrity sha512-wGCwX8/R6Orp7rY3G9SLOhoS5jvxyViac0HBVlRxq6diufypinxvKDgZZJRjgeZ7x6v8l1sZ55EDgsTCBsBbxQ==
dependencies:
axe-core "3.3.1"
axe-core "3.4.1"
chalk "2.4.2"
jest-matcher-utils "24.8.0"
lodash.merge "4.6.2"
Expand Down