Improve file block accessibility by adding aria-describedby to download button #28062

allilevine · 2021-01-08T19:42:24Z

Description

This PR adds an id to the file link and adds that id as an aria-describedby on the download button. This announces the Download button in screen readers as "Download, [content from the file link text]".

For example, in this gif with "captions-1" as the link text, it announces the Download button as "Download, captions-1."

To avoid duplicate ids (and the potential for the wrong file name being linked to the download button), I used the clientId method suggested here.

Fixes #22908
Updates #22914

How has this been tested?

Checkout this PR: git fetch origin pull/28062/head:pr/28062 && git checkout pr/28062
Add a new file block to a post:

Upload a file and publish or preview the post.
Navigate the post with a screen reader. The Download button should be announced as "Download," then the file name.

Types of changes

This change fixes an accessibility issue, and requires a deprecation entry and new fixtures.

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.
I've updated all React Native files affected by any refactorings/renamings in this PR.

The text was updated successfully, but these errors were encountered:

eyesofjeremy · 2021-02-23T19:39:21Z

@allilevine this looks great. Have there been any considerations for converting the [Download] button into a span element? I found this PR while reviewing a site for accessibility, and have similar concerns. Adding a label to the button fixes one of them, but we still end up with an extra unnecessary link.

If the Download button were nested inside the link as a span, then you would only have one link. Thoughts?

allilevine · 2021-02-26T20:32:15Z

@allilevine this looks great. Have there been any considerations for converting the [Download] button into a span element? I found this PR while reviewing a site for accessibility, and have similar concerns. Adding a label to the button fixes one of them, but we still end up with an extra unnecessary link.

If the Download button were nested inside the link as a span, then you would only have one link. Thoughts?

I haven't seen any mention of combining the two links, thanks for pointing that out! Is the advantage that only one link would be read by screen readers, instead of two of the same link?

I made the change in save.js and tried it out with VoiceOver:

{ ! RichText.isEmpty( fileName ) && (
	<a
		href={ textLinkHref }
		target={ textLinkTarget }
		rel={ textLinkTarget ? 'noreferrer noopener' : false }
	>
	<RichText.Content value={ fileName } />

	{ showDownloadButton && (
		<span className="wp-block-file__button">
			<RichText.Content
				value={ downloadButtonText }
			/>
		</span>
	) }
	</a>
) }

One potential issue I see is that if there's no fileName, nothing will display, whereas before the Download button would still display. But it does eliminate the need for a unique block id.

allilevine · 2021-02-26T21:51:35Z

@enriquesanchez or @afercia would you mind weighing in on the accessibility of two links vs. one link?

eyesofjeremy · 2021-02-26T23:12:00Z

Is the advantage that only one link would be read by screen readers, instead of two of the same link?

@allilevine yes, that's what I was thinking of. Having a duplicate link means more to wade through for screen readers and just for tabbing (although the effect on tabbing is not quite as annoying).

But the potential issue you bring up is a good one.

Could you do the following, only wrapping the RichText.Content in the conditional for filename? (Note:, haven't tested)

               <a
                    href={ textLinkHref }
                    target={ textLinkTarget }
                    rel={ textLinkTarget ? 'noreferrer noopener' : false }
                >
                    { ! RichText.isEmpty( fileName ) && (
                    <RichText.Content value={ fileName } />
                    ) }

                    { showDownloadButton && (
                        <span className="wp-block-file__button">
                            <RichText.Content
                                value={ downloadButtonText }
                            />
                        </span>
                    ) }
                </a>

allilevine · 2021-03-02T19:52:18Z

Could you do the following, only wrapping the RichText.Content in the conditional for filename? (Note:, haven't tested)

That could work! Then the Download button would still show up without the fileName, which is the current behavior.

That scenario isn't ideal for accessibility though, since we lose the description of the file that the button downloads again. 😕 Maybe it would help to look at why and when that could happen.

tellthemachines · 2021-03-03T23:50:43Z

The two links have different functions: the first opens the file in the browser, and the second downloads it, so we shouldn't try to mash them together.

allilevine

Since 29 files is a lot, I've added some comments to clarify what's happening in the code.

Any changes in packages/e2e-tests/fixtures are required updates to the fixture tests, many of which are auto-generated.

allilevine · 2021-03-17T22:24:17Z

packages/block-library/src/file/block.json

@@ -9,6 +9,12 @@
 		"href": {
 			"type": "string"
 		},
+		"fileId": {


This change adds fileId as a block attribute, so that we can always identify the right file name to announce.

allilevine · 2021-03-17T22:24:17Z

packages/block-library/src/file/deprecated.js

+import { RichText } from '@wordpress/block-editor';
+
+// Version of the file block without PR#28062 accessibility fix.
+const deprecated = [


We're making changes to how the block is edited and saved, so we need to deprecate the previous version of the block. That way, it will still be supported in older posts.

allilevine · 2021-03-17T22:24:17Z

packages/block-library/src/file/edit.js

@@ -49,7 +49,13 @@ function ClipboardToolbarButton( { text, disabled } ) {
 	);
 }

-function FileEdit( { attributes, setAttributes, noticeUI, noticeOperations } ) {
+function FileEdit( {


We're just adding clientId here, to use it in the fileId.

allilevine · 2021-03-17T22:24:17Z

packages/block-library/src/file/edit.js

@@ -93,6 +99,11 @@ function FileEdit( { attributes, setAttributes, noticeUI, noticeOperations } ) {
 		}
 	}, [] );

+	useEffect( () => {


Whenever the clientId changes, the fileId changes. That way it's always unique, even if we duplicate the block.

allilevine · 2021-03-17T22:24:17Z

packages/block-library/src/file/index.js

@@ -7,6 +7,7 @@ import { file as icon } from '@wordpress/icons';
 /**
 * Internal dependencies
 */
+import deprecated from './deprecated';


Here we're telling the block about the deprecated version.

allilevine · 2021-03-17T22:24:17Z

packages/block-library/src/file/save.js

@@ -30,6 +32,7 @@ export default function save( { attributes } ) {
 						href={ href }
 						className="wp-block-file__button"
 						download={ true }
+						aria-describedby={ fileId }


This is the main event! We're adding an aria-describedby prop to the Download button that references the file link above. That's how the Download button announces the file name from the link.

allilevine · 2021-04-15T19:10:16Z

@sarahricker @joedolson Can you review this PR from an a11y perspective? Is it still a change we want to make?

alexstine · 2021-08-26T21:18:19Z

I think this change would still be helpful but how stale is the code now? Most of this have to be reworked?

allilevine · 2021-08-27T13:07:22Z

I think this change would still be helpful but how stale is the code now? Most of this have to be reworked?

@alexstine I'm happy to refactor if you see value in making this change. It was just missing an accessibility review.

alexstine · 2021-08-27T16:43:03Z

I'm perfectly happy to do the review once it is fixed up. Sadly the Accessibility team is quite small so while I probably browsed over the related issue, I never took much of a close look. Best way to get my attention is to flag me down in Make WordPress Slack.

Thanks.

allilevine · 2021-09-02T13:10:37Z

I'm perfectly happy to do the review once it is fixed up. Sadly the Accessibility team is quite small so while I probably browsed over the related issue, I never took much of a close look. Best way to get my attention is to flag me down in Make WordPress Slack.

Thanks.

@alexstine Good to know, thanks! I've updated the PR. I did notice that the PDF preview feature that was added to the file block in the meantime uses aria-label. That would be a simpler change to make to add the fileName to the Download button, but maybe not as much of an improvement. It seems to replace the "Download" text.

alexstine · 2021-09-03T04:17:53Z

@allilevine I'll try to review this today but I may run out of time. Adding it to the Accessibility team meeting agenda in case someone else can get to it first.

alexstine

@allilevine Changes from A11Y perspective are perfect. Nice one. 👍

allilevine · 2021-09-14T21:48:39Z

@allilevine Changes from A11Y perspective are perfect. Nice one. 👍

Thank you! 🙌

allilevine · 2021-09-15T13:25:49Z

@talldan It would be great to get your review of these changes, especially to make sure they address your feedback here.

tellthemachines

This is working well! Thanks for fixing the issue. Also, great work on the PR description and explanatory comments ⭐

github-actions bot added the First-time Contributor label Jan 8, 2021

allilevine changed the title ~~File Block: Semantically link the filename link to the download button on the file block.~~ [WIP] File Block: Semantically link the filename link to the download button on the file block. Jan 8, 2021

allilevine changed the title ~~[WIP] File Block: Semantically link the filename link to the download button on the file block.~~ File Block: Semantically link the filename link to the download button on the file block. Jan 8, 2021

allilevine marked this pull request as ready for review Jan 8, 2021

allilevine requested review from ajitbohra, nerrad, ntwb and talldan as code owners Jan 8, 2021

allilevine mentioned this pull request Jan 8, 2021

File Block: Semantically link the filename link to the download button on the file block. #22914

Closed

8 tasks

allilevine force-pushed the update/file-block-describedby branch from b404cec to 7a38587 Feb 26, 2021

Base automatically changed from master to trunk Mar 1, 2021

tellthemachines added [Block] File Needs Accessibility Feedback labels Mar 3, 2021

allilevine force-pushed the update/file-block-describedby branch 2 times, most recently from beda8f2 to c2811c8 Mar 12, 2021

allilevine changed the title ~~File Block: Semantically link the filename link to the download button on the file block.~~ Improve file block download button accessibility Mar 17, 2021

allilevine reviewed Mar 17, 2021

View changes

allilevine force-pushed the update/file-block-describedby branch 3 times, most recently from d493e87 to 09d17fd Apr 15, 2021

skorasaurus added the Accessibility (a11y) label Aug 19, 2021

allilevine force-pushed the update/file-block-describedby branch from 09d17fd to eb4ffca Aug 27, 2021

allilevine added the [Status] In Progress label Aug 27, 2021

allilevine changed the title ~~Improve file block download button accessibility~~ [WIP] Improve file block download button accessibility Aug 27, 2021

allilevine force-pushed the update/file-block-describedby branch from eb4ffca to 187f89a Aug 31, 2021

allilevine added 8 commits Sep 1, 2021

Add file block deprecation entry

33d1d7f

Rebase fixtures

a41664d

Make fileId unique by using clientId instead of id

e31d8f5

Add fileId as a block attribute and update tests

3adb3b2

Update deprecation comment

3b7c3ec

Add id to link and aria-describedby to download button.

2ece54e

Clean up block and tests from rebase

8c97dd1

Remove moved tests

8b6a49e

allilevine force-pushed the update/file-block-describedby branch from 40964dd to 8b6a49e Sep 1, 2021

Update deprecation and tests

6c0439b

allilevine changed the title ~~[WIP] Improve file block download button accessibility~~ Improve file block download button accessibility Sep 2, 2021

allilevine removed [Status] In Progress First-time Contributor labels Sep 2, 2021

alexstine approved these changes Sep 14, 2021

View changes

allilevine removed the Needs Accessibility Feedback label Sep 14, 2021

tellthemachines approved these changes Sep 16, 2021

View changes

allilevine added this to the Gutenberg 11.6 milestone Sep 17, 2021

allilevine changed the title ~~Improve file block download button accessibility~~ Improve file block accessibility by adding aria-describedby to download button Sep 17, 2021

allilevine merged commit aa6217f into WordPress:trunk Sep 17, 2021
19 checks passed

WordPress / gutenberg Public

Improve file block accessibility by adding aria-describedby to download button #28062

Improve file block accessibility by adding aria-describedby to download button #28062

allilevine commented Jan 8, 2021 •

edited

eyesofjeremy commented Feb 23, 2021

allilevine commented Feb 26, 2021

allilevine commented Feb 26, 2021

eyesofjeremy commented Feb 26, 2021 •

edited

allilevine commented Mar 2, 2021

tellthemachines commented Mar 3, 2021

allilevine left a comment

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine commented Apr 15, 2021

alexstine commented Aug 26, 2021

allilevine commented Aug 27, 2021

alexstine commented Aug 27, 2021

allilevine commented Sep 2, 2021

alexstine commented Sep 3, 2021

alexstine left a comment

allilevine commented Sep 14, 2021

allilevine commented Sep 15, 2021

tellthemachines left a comment

WordPress / gutenberg Public

Improve file block accessibility by adding aria-describedby to download button #28062

Improve file block accessibility by adding aria-describedby to download button #28062

Conversation

allilevine commented Jan 8, 2021 • edited

Description

How has this been tested?

Types of changes

Checklist:

eyesofjeremy commented Feb 23, 2021

allilevine commented Feb 26, 2021

allilevine commented Feb 26, 2021

eyesofjeremy commented Feb 26, 2021 • edited

allilevine commented Mar 2, 2021

tellthemachines commented Mar 3, 2021

allilevine left a comment

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine Mar 17, 2021

allilevine commented Apr 15, 2021

alexstine commented Aug 26, 2021

allilevine commented Aug 27, 2021

alexstine commented Aug 27, 2021

allilevine commented Sep 2, 2021

alexstine commented Sep 3, 2021

alexstine left a comment

allilevine commented Sep 14, 2021

allilevine commented Sep 15, 2021

tellthemachines left a comment

allilevine commented Jan 8, 2021 •

edited

eyesofjeremy commented Feb 26, 2021 •

edited