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

Screen reader support #311

Merged
merged 17 commits into from
Feb 12, 2018
Merged

Screen reader support #311

merged 17 commits into from
Feb 12, 2018

Conversation

alexreardon
Copy link
Collaborator

@alexreardon alexreardon commented Feb 8, 2018

Adding proper screen reader support

Closes #7

Partially completes #298 (adds onDragUpdate)

@@ -564,6 +566,29 @@ Here are a few poor user experiences that can occur if you change things *during
- If you remove the node that the user is dragging, then the drag will instantly end
- If you change the dimension of the dragging node, then other things will not move out of the way at the correct time.

### Accessibility ❤️
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: fill out

@@ -1,143 +0,0 @@
// @flow
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to new file

const previousPhase = previous.phase;

// Dragging in progress
if (currentPhase === 'DRAGGING' && previousPhase === 'DRAGGING') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

processing for a new hook - onDragUpdate 👍

src/types.js Outdated
onDragStart?: (start: DragStart) => void,
onDragEnd: (result: DropResult) => void,
onDragStart?: (start: DragStart, announce: Announce) => void,
onDragUpdate?: (update: DragUpdate, announce: Announce) => void,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new hook!

src/types.js Outdated
export type Hooks = {|
onDragStart?: (start: DragStart) => void,
onDragEnd: (result: DropResult) => void,
onDragStart?: (start: DragStart, announce: Announce) => void,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all hooks now provided with a second argument: announce

This is used to announce text to screen readers

@@ -7,7 +7,8 @@ import { quotes, getQuotes } from './src/data';
import { grid } from './src/constants';

const data = {
small: quotes,
// small: quotes,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • change back

tasks: Task[]
|}

const getPosition = (index: number, length: number): string => `
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wording feels a little dry

@@ -15,7 +15,7 @@ type Props = {

type HTMLElement = any;

const Container = styled.a`
const Container = styled.div`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • change back

@@ -40,7 +40,11 @@ export default class QuoteApp extends Component<Props, State> {
quotes: this.props.initial,
};

onDragStart = (initial: DragStart) => {
onDragStart = (initial: DragStart, announce: Announce) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove the announcement stuff from this example and leave it in the new story

@alexreardon
Copy link
Collaborator Author

This build will never go green as auto-scroller is also red. Once we are happy with this we can merge it into that branch

@alexreardon alexreardon merged commit 6f01b78 into auto-scroll Feb 12, 2018
@alexreardon alexreardon deleted the accessibility branch March 23, 2018 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant