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

Improving accessibility on Image Block (deselected) #794

Closed
wants to merge 13 commits into from

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Mar 29, 2019

Part of #476

This PR improves accessibility (screen reader) for the Image Block. In particular when it's deselected.
gutenberg side PR: WordPress/gutenberg#14713

The current behavior is:
Empty image block:

  • Label: "Image block. Empty."
  • Hint: "Double tap to select an image."

Image block with image:

  • Label: "Image block. {alt}."
  • Hint: ""Double tap to edit the image."

cc @rachelmcr

When the block is not selected, it's captured by the screen reader as one whole element, but when it is selected, it opens the inner controls, including the inner tool bar.

To test (iOS):

To test (Android):

Common test steps:

  • Select an empty Image Block.
  • Check that it reads as described before, including label and hint. (Hint takes a couple of seconds on iOS).
  • Activate the selected block.
  • Check that it shows the image selector.
  • Select an Image block with content.
  • Check that it reads as described before.
  • If it doesn't have alt, add one (recommended to deactivate VoiceOver since the bottom-sheet is not scree reader friendly yet)
  • Check that VoiceOver reads "Image block. {alt}." on the image block with alt.
  • Double tap on the block to activate it.
  • Check that the block is now active, and the inner elements are selectable.

@etoledom etoledom added this to the v1.2 milestone Mar 29, 2019
@etoledom etoledom self-assigned this Mar 29, 2019
@etoledom etoledom requested a review from Tug March 29, 2019 18:22
@etoledom etoledom marked this pull request as ready for review March 29, 2019 18:31
Copy link
Contributor

@Tug Tug left a comment

Choose a reason for hiding this comment

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

Works great on Android 🚢 !!

@etoledom etoledom mentioned this pull request Apr 3, 2019
4 tasks
@etoledom etoledom modified the milestones: v1.2, v1.3 Apr 5, 2019
@etoledom etoledom requested review from SergioEstevao and hypest and removed request for SergioEstevao April 8, 2019 10:03
Copy link
Contributor

@JavonDavis JavonDavis left a comment

Choose a reason for hiding this comment

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

@etoledom This also works great on iOS. Tested on iPad 12.1 👍🏽

@hypest hypest removed their request for review April 11, 2019 07:52
@hypest
Copy link
Contributor

hypest commented Apr 19, 2019

Some more work seems to be needed in this one (related to TalkBack as per @etoledom over Slack) so, moving the milestone to not block today's cut.

@hypest hypest modified the milestones: v1.3, v1.4 Apr 19, 2019
@Tug
Copy link
Contributor

Tug commented Apr 22, 2019

Tested on iOS and I found it really hard to make things work in general (not specific to this PR I think)

  • Once an image block selected, and you move to another item, it's almost impossible to go back to the selected block, it jumps from the previous block to the caption inside our block, we should probably synchronize block selection with voiceover selection (if it's possible).
  • Double taping on a selected image block does not trigger any action
  • The hide keyboard button did not respond to double tap

So regarding this PR, I think the following test step is failing:

  • Check that the block is now active, and the inner elements are selectable.

Copy link
Contributor

@Tug Tug left a comment

Choose a reason for hiding this comment

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

After discussing it with you on slack I understood that the inner elements cannot be selectable if the parent one is (I found it weird because it's not the case on android, and talkback say "block container" in this case when the whole block is selected).
Anyway, we can think about a solution to improve this in another PR, this one is already a good step forward, let's merge it 👍

@etoledom
Copy link
Contributor Author

Thank you @Tug !

I found it weird because it's not the case on android, and talkback say "block container" in this case

I think that the fact that Android's TalkBack reads block container is a bug. It is there for UITests and that component is marked with accessibility={ false }. It also happen with block list.

I'm even thinking that it might be a React-Native bug, but I haven't found any tickets about this. If it's expected behavior, it will be super bad for us :(

Double taping on a selected image block does not trigger any action

True! There's a ticket to handle the Image Block on selected status. This one was meant for the Image block when it's not selected (mainly).

The hide keyboard button did not respond to double tap

Also true! This is a bug even without Screen Reader active.

Copy link
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

LGTM!

@etoledom
Copy link
Contributor Author

Closing since the Gutenberg ref was already updated by #920

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants