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

[Mobile]Improve accessibility on Heading block #15144

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

pinarol
Copy link
Contributor

@pinarol pinarol commented Apr 24, 2019

Description

This PR improves accessibility on Heading block.
Fixes wordpress-mobile/gutenberg-mobile#908

To test:
Please refer to the gutenberg-mobile side PR.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@pinarol pinarol added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Apr 24, 2019
@pinarol pinarol self-assigned this Apr 24, 2019
@pinarol pinarol requested review from Tug and etoledom April 24, 2019 13:08
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

@pinarol pinarol merged commit af0b01d into master Apr 24, 2019
@pinarol pinarol deleted the rnmobile/heading-block-accessibility branch April 24, 2019 15:47
@swissspidy
Copy link
Member

@etoledom @pinarol Why exactly is this not using sprintf()? I am afraid these strings are totally not translatable this way.

@swissspidy
Copy link
Member

Opened #15161 for this.

@pinarol
Copy link
Contributor Author

pinarol commented Apr 25, 2019

@etoledom @pinarol Why exactly is this not using sprintf()? I am afraid these strings are totally not translatable this way.

Thanks for the heads-up @swissspidy These are the strings to be read out loudly by screen readers(as a part of device accessibility), we just wanted to put a pause between the words and it is done by dot+space. So we didn't want to derive new strings just to put '. ' after them. BTW I totally agree that 'level' should be fixed as 'level %s', that's sth we missed. So, if deriving new strings is the more preferred way by translators that's perfectly ok for us, and it is easier for us to code too!

@swissspidy
Copy link
Member

Thanks for the additional context. Yes, I would say in this case new strings is better. String concatenation does not work for every language there is, and a complete sentence is much easier to comprehend for translators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Screen Reader on Heading block.
4 participants