-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve Link Control accessibility #54063
Conversation
Size Change: +445 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
@getdave Where can I find the link control that should be tested in this PR? Is it just the Link button in the block toolbar? |
@alexstine Apologies my testing instructions didn't publish yesterday as I was writing them whilst on a train 😅 The control is the standard interface for creating links in the block editor. I have updated the testing instructions section in the PR description. Also worth testing alongside #54085 |
@getdave I am testing this in Firefox and this control seems largely inaccessible.
I am unable to Tab/Shift+Tab throughout the popover so unsure where I would find a preview mode or any other options. Thanks. |
Noting to myself that we need to land #52620 to improve a11y of the initial link creation step. |
@alexstine I'm sorry this didn't work well for you. Honestly I think the control needs improvement which is party why I wanted to get your opinion. I see the problem though. The Link creation phase (which is where you got to) is flawed because there is no submit button. We are restoring that in #52620 but it's currently blocked by some test failures which I'm working on. Also allow me to outline the two "modes" as this is confusing terminology:
As the flow clearly doesn't work as things stand, I won't ask for any more testing here until #52620 is merged. I'll also ask @jeryj to give this a run through so I have objective input from someone other than myself. Once we have the basic flow ironed out I'd be very grateful if you would consider re-testing. This control is used a lot throughout the interface and so I'm extremely keen to improve it's accessibility. Thank you. |
Been playing around with this for a bit. The control is trying to be a popover and a dialog at once, while also not being dismissible. I get why, but it's making it hard to come up with interactions since there's no standard to follow. The link preview part is especially difficult, as I understand why it's visually helpful to get a quick confirmation that you've added the link you think you added. That same quick confirmation isn't communicated to a screen reader yet. I think some semantic grouping/naming of the popover would be helpful for its various states, like "Add link/Edit link/Preview link". I don't rely on a screen reader, so it's possible I'm making incorrect assumptions about that. Link CreationThis flow feels pretty good to me.
Link EditingMostly OK. Would be improved by not switching to Preview once you move though all the tab stops on the edit field.
Link Preview
Also, found a bug while testing:
Summary/Specific RecommendationsAs always, my caveat of, "I'm trying my best here but I'm not an a11y expert." Here's a focused summary:
Thanks for working on this, @getdave! |
One more note and it has been discussed in previous issues but never fixed. When editing a link, you must use the shortcut. From what I can tell, there is only an unlink button in the formatting toolbar and there should also likely be an edit option there for best discovery. It makes little sense that by highlighting text, I would then tab to find a random edit button for my created link when tabbing under any other condition would land me in the sidebar. Other than that comment, agree completely with @jeryj assessment. |
I was wondering about that too. I tried looking in the toolbar and expecting that there'd be a way to reach the edit link form from the toolbar. |
86a82b3
to
350d96e
Compare
We have a problem here with handling the focus trap. The I'm not sure we can simply force constrained tabbing on the Either way to have fine grained control over the focus trap we're going to need to amend the I wonder whether the real solution to all of this is to simply do-away with the preview mode entirely as suggested in #50892. That would allow us to avoid this mode switching entirely and greatly simplify the control itself. What do we think? If we agree then adding comments to #50892 would be a good first step. |
Thanks for looking into this @jeryj and for a great summary. I think one of the things we should look at next is #50892 as the preview doesn't seem to offer much value relative to the confusion it causes UX-wise. Combining everything into a single edit mode could be a good move (cc @richtabor who originally proposed this).
✅ I've updated the existing visually hidden label and description attached to the control via
I don't think we do switch to the preview popover. From what I can see the focus trap on the edit form is not in place and thus the focus moves out of the control entirely. This is a really complex problem because of the two modes. I suggested a potential solution here.
I wasn't able to replicate this at first then I realised the flow is:
This behaviour also supports the view that preview is superfluous as we'd need to have the exact same behaviour for sighted users. |
@alexstine @jeryj Here is a PR to remove the preview step as previously suggested by @richtabor (and others). That PR is not addressing the accessibility Issues I'm trying to fix in this PR, but perhaps once that one is merged we'll be in a better place to iterate here. |
I think all of these changes are now in trunk so we can close this. |
What?
An iteration to improve the accessibility of the Link Control component.
No visuals should change as a result of the PR.
Complimented by #54085
Requires #52620
Why?
Some issues were identified in #51060. This starts to look to address these a long with other issues.
It's currently very difficult to write e2e tests using Playwright's accessible selectors and so this further supports my theory that the a11y of this control is not what it could / should be.
How?
Please also see
Questions
section below.I tested the following with Voiceover on MacOS:
speak()
) when user switches mode from "edit mode" to "preview mode".aria-labelledby
andaria-describedby
attributes on component to improve labelling and (hopefully) comprehension of the control.role=dialog
on all instances that are within aPopover
to indicate that they are overlaid on the content.<dl>
markup when in "preview" mode so that all fields have associated definitions.Questions
<form>
tag when in edit mode. This was intentionally removed because there are instances where the component is used when it is already within a wrapping<form>
element and this was causing problems. However, should we perhaps have an option to utilise a<form>
tag in "edit" mode? Is this useful for users of assistive tech? If so why and what advantages does it convey?role=dialog
when the control is used within aPopover
do we need to have a focus trap? Officially Dialog's do not have to trap focus but given that the UI can enter an "edit" mode where there is a form it seems like you shouldn't be able to tab out of the UI whilst in the middle of editing. If so we cannot use the existing<Modal>
from@wordpress/components
but I should be able to modify our usage ofPopover
.aria-labelledby
andaria-describedby
attributes be on the Popover or the LinkControl component markup?Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast