[Mobile]: Improving accessibility on Image Block (deselected) #14713
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
etoledom
added
the
Mobile App - i.e. Android or iOS
etoledom
changed the title
5 tasks
Tug
reviewed
Apr 2, 2019
| accessible={ ! isSelected } | ||
| accessibilityLabel={ __( 'Image block' ) + '.' + alt } | ||
| accessibilityRole={ 'button' } | ||
| accessibilityHint={ __( 'Doulbe tap to edit the image' ) } |
Tug
reviewed
Apr 2, 2019
| @@ -16,7 +16,12 @@ import styles from './styles.scss'; | |||
|
|
|||
| function MediaPlaceholder( props ) { | |||
| return ( | |||
| <TouchableWithoutFeedback onPress={ props.onMediaOptionsPressed }> | |||
| <TouchableWithoutFeedback | |||
| accessibilityLabel={ __( 'Image block. Empty ' ) } | |||
There was a problem hiding this comment.
I guess we can reuse the 'Image block' string here. Image block. Empty as a whole does not make much sense for translators:
accessibilityLabel={ __( 'Image block' ) + '. ' + __( 'Empty' ) }
Tug
reviewed
Apr 2, 2019
| <TouchableWithoutFeedback onPress={ this.onImagePressed } disabled={ ! isSelected }> | ||
| <TouchableWithoutFeedback | ||
| accessible={ ! isSelected } | ||
| accessibilityLabel={ __( 'Image block' ) + '.' + alt } |
There was a problem hiding this comment.
It could use a space after the period:
__( 'Image block' ) + '. ' + alt
…ordPress/gutenberg into rnmobile/accessibility-mark-ii
|
Thank you for the review @Tug ! |
etoledom
requested review from
SergioEstevao and
hypest
and removed request for
SergioEstevao
For some reason, VoiceOver wouldn't scroll down the block list. The breaking change was located at the removal of this css property `font-family: $default-regular-font;` on the nextpage block. Addinb back that property to be able to scroll the blocks list again using VoiceOver.
etoledom
commented
Apr 22, 2019
| // that React Native wasn't able to handle (adding a fallback generic font family). | ||
| const textStyle = { | ||
| ...styles[ 'block-library-nextpage__text' ], | ||
| fontFamily: 'System', |
There was a problem hiding this comment.
This was creating an issue that disabled scrolling with VoiceOver active.
I'm not sure the specific reason, but rolling back this change fixed the issue.
pinarol
reviewed
Apr 22, 2019
| <TouchableWithoutFeedback onPress={ this.onImagePressed } disabled={ ! isSelected }> | ||
| <TouchableWithoutFeedback | ||
| accessible={ ! isSelected } | ||
| accessibilityLabel={ __( 'Image block' ) + '. ' + alt } |
There was a problem hiding this comment.
the dot here can be localized also
pinarol
reviewed
Apr 22, 2019
| @@ -16,7 +16,12 @@ import styles from './styles.scss'; | |||
|
|
|||
| function MediaPlaceholder( props ) { | |||
| return ( | |||
| <TouchableWithoutFeedback onPress={ props.onMediaOptionsPressed }> | |||
| <TouchableWithoutFeedback | |||
| accessibilityLabel={ __( 'Image block' ) + '. ' + __( 'Empty' ) } | |||
There was a problem hiding this comment.
the dot here can be localized also
pinarol
reviewed
Apr 22, 2019
| <View style={ styles[ 'block-library-nextpage__container' ] }> | ||
| <View | ||
| accessible | ||
| accessibilityLabel={ __( 'Page break block' ) + '. ' + accessibilityTitle } |
There was a problem hiding this comment.
the dot here can be localized also
|
I've tried adding accesibility label for Image itself also, I think it felt better to be able to hear the image info independent from its selected state. |
|
Thank you! |
This was referenced Apr 29, 2019
hypest
pushed a commit
that referenced
this pull request
May 2, 2019
* Adding accessibility properties to MediaPlaceholder and ImageEdit * Fix lint issues * Fix typo * Better accessibility labels * Disable lists. (#14844) * Fix scroll with VoiceOver ON. For some reason, VoiceOver wouldn't scroll down the block list. The breaking change was located at the removal of this css property `font-family: $default-regular-font;` on the nextpage block. Addinb back that property to be able to scroll the blocks list again using VoiceOver. * Setting accessibilityStates properly to avoid Android crash * Remove vscode settings file (added by mistake) * Fix lint issues * Update accessibility labels to use sprintf * Fix lint issues
This was referenced Sep 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves accessibility on the Image Block.
To test:
Please refer to the gutenberg-mobile side PR.