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

Add an aria-label to the editor blocks. #1618

Merged
merged 2 commits into from
Jul 3, 2017
Merged

Conversation

afercia
Copy link
Contributor

@afercia afercia commented Jun 30, 2017

First pass to try to add an aria-label to the Editor blocks. Code and wording could be improved, any feedback welcome.

All the editor blocks need to be labeled to allow screen reader users to get what a block is.

Screenshots:

screen shot 2017-06-30 at 17 55 40

screen shot 2017-06-30 at 17 57 09

Re: VoiceOver in the screenshot above:

  • "group" is added by VoiceOver because blocks have no semantic role, so it announces them as generic "group"
  • also the number of items in the group is added by VoiceOver

Fixes #562

@@ -236,6 +237,7 @@ class VisualEditorBlock extends Component {
render() {
const { block, multiSelectedBlockUids } = this.props;
const blockType = getBlockType( block.name );
const blockLabel = sprintf( __( 'Editor block: %s' ), blockType.title );
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'd just use Block: %s but I'm fine either way

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@afercia
Copy link
Contributor Author

afercia commented Jul 2, 2017

I've simplified the aria label to be just Block: %s. Also, added a translators comment, using the same one used for the block mover label. That could be probably improved a bit. Worth noting these translators comments should preferably be inside sprintf(). I see this has already been considered and there are thoughts to improve a bit the tooling, see #984 (comment)
Not sure if there's already a specific GH issue.

Edit: seems it has already been addressed? /cc @aduth

@afercia afercia merged commit 9ef4572 into master Jul 3, 2017
@afercia afercia deleted the try/editor-blocks-aria-label branch July 3, 2017 13:37
@aduth
Copy link
Member

aduth commented Jul 3, 2017

Edit: seems it has already been addressed? /cc @aduth

Yes, should be improved as of #1068

@afercia afercia mentioned this pull request Oct 19, 2017
3 tasks
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.

3 participants