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

fix: Change aria-label depending on content of paragraph block #11653

Merged
merged 2 commits into from
Nov 9, 2018

Conversation

tofumatt
Copy link
Member

@tofumatt tofumatt commented Nov 9, 2018

Fix issue introduced in #11560.

This only shows an aria-label for RichText blocks when they aren't empty. I think it's a start; let me know if that fixes the issue I introduced in #11560
#11560 (comment).

@tofumatt tofumatt added this to the 4.4 milestone Nov 9, 2018
@tofumatt tofumatt requested review from afercia and a team November 9, 2018 02:37
@afercia
Copy link
Contributor

afercia commented Nov 9, 2018

Thanks @tofumatt
This actually renders an aria-label="false" where false is evaluated as a string and used as the attribute value.

screenshot 2018-11-09 at 08 49 10

Additionally, quoting from #1659:

The aria-label attribute works as a <label> element and gives a name to its control. So both when the Editable is empty or already filled up with content, screen readers will announce the Editable with the name given by the aria-label.

[sorry for my broken English]

the aria-label should identify what the Editable is. For a paragraph, it should say: Paragraph content or similar wording

The placeholder values:

are more "suggestions" that should go in a description (see #5981 (comment)). Instead, each editable field should be labelled to communicate what it is, as one would normally do for form fields.

@jasmussen
Copy link
Contributor

I added a followup comment on the phrasing in #11560 (comment) — if that phrasing was intentional, then I won't be the blocker. But if it was a typo or slightly unintentional, do you think you could lump in the phrasing change in this PR? Thanks Matt.

@youknowriad youknowriad modified the milestones: 4.4, 4.3 Nov 9, 2018
@youknowriad
Copy link
Contributor

I added to 4.3 as it's smallish we can get it quickly in.

@tofumatt tofumatt changed the title fix: Only show aria-label when content is empty fix: Change aria-label depending on content of paragraph block Nov 9, 2018
@tofumatt
Copy link
Member Author

tofumatt commented Nov 9, 2018

I've updated the code to reflect the real state of this block; when empty it's functionally an empty block that by default accepts text to become a paragraph or the slash key to open the inserter.

Now the label instructs users what is there; when there is content: the label "Paragraph Block" is read. When there is no content: "Empty block; start writing or type forward slash to choose a block" is read.

I think that's a good solution for now.

I also tweaked the text as per @jasmussen's request.

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

Approving based on content of placeholder. Thanks so much for this, it's way better, and should pair nicely with #11659 which makes the text not wrap on mobile or in columns.

Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

You may want to wait for a second (Joen beat me to it!) 👍, but this looks correct to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants