Skip to content

[Mobile]Improve accessibility on Heading block#15144

Merged
pinarol merged 1 commit intomasterfrom
rnmobile/heading-block-accessibility
Apr 24, 2019
Merged

[Mobile]Improve accessibility on Heading block#15144
pinarol merged 1 commit intomasterfrom
rnmobile/heading-block-accessibility

Conversation

@pinarol
Copy link
Copy Markdown
Contributor

@pinarol pinarol commented Apr 24, 2019 •
edited
Loading

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 mentioned this pull request Apr 24, 2019
Merged
@pinarol pinarol requested review from Tug and etoledom April 24, 2019 13:08
etoledom
etoledom approved these changes Apr 24, 2019
View reviewed changes
Copy link
Copy Markdown
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.

Tested via wordpress-mobile/gutenberg-mobile#925 (review)

Great job!

@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
Copy Markdown
Member

swissspidy commented Apr 24, 2019

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

@swissspidy swissspidy mentioned this pull request Apr 24, 2019
Merged
5 tasks
@swissspidy
Copy link
Copy Markdown
Member

swissspidy commented Apr 24, 2019

Opened #15161 for this.

@pinarol
Copy link
Copy Markdown
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
Copy Markdown
Member

swissspidy commented Apr 25, 2019

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.

This was referenced Apr 29, 2019
Merged
Merged
hypest pushed a commit that referenced this pull request May 2, 2019
@pinarol @hypest
Set accessibility props for Heading (#15144)
b824a73
@youknowriad youknowriad added this to the 5.6 (Gutenberg) milestone May 13, 2019
This was referenced Sep 12, 2024
Open
Open
Open
@AKJUS AKJUS mentioned this pull request Sep 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@etoledom etoledom etoledom approved these changes

@Tug Tug Awaiting requested review from Tug

Assignees

@pinarol pinarol

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

Milestone

5.6 (Gutenberg)

Development

Successfully merging this pull request may close these issues.

Improve Screen Reader on Heading block.

4 participants

@pinarol @swissspidy @etoledom @youknowriad