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

feat: ability to add titles for the links #549

Merged
merged 2 commits into from
Sep 26, 2023
Merged

Conversation

Kyzyl-ool
Copy link
Contributor

@Kyzyl-ool Kyzyl-ool commented Sep 8, 2023

  • Added urlTitle everywhere where links (<a> tags) were used.

This feature can be used to provide both visual and accessible warning that the link will open in a new tab

Closes #484 and #485

@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@Kyzyl-ool Kyzyl-ool force-pushed the feat/a11y-anchor-titles branch from 89b065c to b9cc8fe Compare September 18, 2023 07:03
@Kyzyl-ool Kyzyl-ool force-pushed the feat/a11y-anchor-titles branch from b9cc8fe to e9056c2 Compare September 21, 2023 16:09
<a href={url} title={text} className={classes} {...rest} {...linkExtraProps}>
<a
href={url}
title={urlTitle || text}
Copy link
Contributor

Choose a reason for hiding this comment

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

why is there a fallback to the text in this place but not in all places?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For backward compatibility I left fallback here. But I didn't do one on other places to avoid unexpected tooltips. Moreover, I expect that the link text and link title will not be the same

const iconData = getMediaImage(icon);
const {alt} = iconData;

return (
<a
href={url}
aria-label={alt}
title={alt}
title={urlTitle || alt}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can alt of icon be the title of a link?

Copy link
Contributor Author

@Kyzyl-ool Kyzyl-ool Sep 25, 2023

Choose a reason for hiding this comment

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

Yes, but the accessible name will be read by screenreader in this order:

  1. link text
  2. image alt
  3. link title

Is icon's alt is the same as the link title, the screenreader will read two sentences twice, it is not really good. So I think they should differ

@Kyzyl-ool Kyzyl-ool merged commit 514c988 into main Sep 26, 2023
3 checks passed
@Kyzyl-ool Kyzyl-ool deleted the feat/a11y-anchor-titles branch September 26, 2023 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aria-describedby for links
3 participants