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

LinkControl: Improve the link preview and Copy link aria labels #60901

Open
afercia opened this issue Apr 19, 2024 · 3 comments · May be fixed by #60908
Open

LinkControl: Improve the link preview and Copy link aria labels #60901

afercia opened this issue Apr 19, 2024 · 3 comments · May be fixed by #60908
Assignees
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Apr 19, 2024

Description

The LinkControl has a couple ARIA labels that can be improved.

1
The link preview can show a 'rich data' preview of the link destination page or the link text and url. That's important information that needs to be understandable for all users. Visually, it could be improved with some visible title to explain what it is. Assuming a design change is not desired (although I'd recommend it) the semantics needs to be improved.

In fact, the whole 'preview' and the buttons are wrapped in a div element with no ARIA role and an aria label that says 'Currently selected'.
I'm not sure what the reasoning about using such a label was, but it's less than ideal. What 'Currently selected' means in this context?

For screen reader users, this section of information needs to be explained in a meaningful way. 'Currently selected' doesn' tell users anything useful.

Screenshot of Safari and VoiceOver announcing this section of content as 'Currently selected'. Note that the screen reader also announces 'group' because it attempts to assign the most generic ARIA role to a labelled div element with no explicit role set.

Screenshot 2024-04-17 at 12 39 43

2
The copy button label (and tooltip) should not contain the link URL.

  • Usign such a label makes this button hardily usable for speech recognition software users.
  • It's inconsistend with other Copy buttons.
  • The aria-label should never contain values or states. Instead, it is meant to provide an accessible name for the control, to explain what that control does.

Screenshot:

Screenshot 2024-04-17 at 12 19 28 2

Step-by-step reproduction instructions

  • Use a screen reader or explore the link preview by using your browser dev tools inspcctor.
  • Observe the link preview is wrapped in a div with an aria-label Currently selected and it is announced as such.
  • Hover or focus the Copy button.
  • Observe the tooltip (which is based on the aria-label) contains the URL value.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor labels Apr 19, 2024
@afercia afercia self-assigned this Apr 19, 2024
@afercia
Copy link
Contributor Author

afercia commented Apr 19, 2024

For a new, more meaningful, aria-label for the link preview it's worth reminding the link preview can show a few things that are conceptually different.

The fetched information of an external site displayed in a 'rich data' preview, including the site icon:

fetched

Same for an internal link.

Or, if the link destination page can't be fetched, it will display a generic icon, the link text, and the link url with the protocol part stripped out:

not fetched

Or, when the link text is the same as the link url, it may show only the link:

image

Given this UI can show different things, sometimes it may actually not be a 'preview'. A new, more meaningul, labeling should likely avoid the term 'preview' and use a more generic terminology to commuincate users this is generic information about the destination page.

@afercia
Copy link
Contributor Author

afercia commented Apr 19, 2024

Note: translatable should avoid concatenation. In the case illustrated below, the colon : is not translatable.
The WordPress l10n guidelines recomment to always use fully translatable strings, including punctuation.

label={ sprintf(
// Translators: %s is a placeholder for the link URL and an optional colon, (if a Link URL is present).
__( 'Copy link%s' ), // Ends up looking like "Copy link: https://example.com".
isEmptyURL || showIconLabels ? '' : ': ' + value.url
) }

@afercia
Copy link
Contributor Author

afercia commented Apr 19, 2024

Additionally: when the preference 'Show button text labels' is enabled, all three buttons still show the tooltips, they just repeat the visible text. That's redundant and should be avoided as it's done for other buttons in the UI.

Screenshot:

Screenshot 2024-04-19 at 15 22 29

@afercia afercia added the [Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) label Apr 19, 2024
@afercia afercia linked a pull request Apr 19, 2024 that will close this issue
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Link Editing Link components (LinkControl, URLInput) and integrations (RichText link formatting) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Block editor /packages/block-editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant