Link | External Icon in Link communicates 'Open in new tab' to screen readers. #2909
Conversation
…icates 'Open in new tab' to screen readers.
🦋 Changeset detectedLatest commit: 1f25e85 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (6f12aca) and published all packages with changesets to npm. You can install the packages in ./dev/tools/deploy_wonder_blocks.js --tag="PR2909"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR2909 |
|
Size Change: +50 B (+0.05%) Total Size: 110 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-hqklgcncpl.chromatic.com/ Chromatic results:
|
| size="small" | ||
| style={[styles.endIcon, styles.centered]} | ||
| testId="external-icon" | ||
| aria-label="Open in new tab" |
There was a problem hiding this comment.
Do y'all agree with this wording for the aria label? I personally prefer this to "Open in a new tab" but I'm also open to other suggestions!
There was a problem hiding this comment.
This will unfortunately replace the entire accessible name for links, which isn't what we want. I think you need to combine the textContent with the "open in new tab" text. It also needs to be translatable -- maybe through a prop?
There was a problem hiding this comment.
Since the aria-label is being set on the external icon (rather than the link element), it doesn't seem to override the accessible name for the link! It seems to get included for the link name (let me know if I'm misunderstanding!):
With it being alternative text for the icon, should the label be something like (Opens in a new tab) in brackets, to help separate it from the link name? Curious what your thoughts are on this @marcysutton !
+1 to making it a prop so translated strings can be passed in. Normally the WB components takes a labels prop for strings that can be passed in, and default values are provided for english. So something like this in the props:
type Props = {
labels?: {
externalIconAriaLabel?: string;
}
}
There was a problem hiding this comment.
Oh you're right @beaesguerra! My apologies -- I missed that it was on the icon, not on the Link itself! 🤦♀️
This should be fine as it is combining the text pieces together successfully: see screenshot below!
Historically the text for these kinds of interactions is "open in a new window", rather than a Tab. But maybe that's outdated!
There was a problem hiding this comment.
Thanks folks! I'll use "(opens in a new tab)" as the default
There was a problem hiding this comment.
I've pushed up the changes and updated the demo recording in the PR description.
The changes include:
- wrapping the message in parentheses to help separate the additional info from the link text
- using "opens in a new tab" in English as a fallback
- using a Wonderblocks-style
labelsprop
Thanks for your feedback!
| size="small" | ||
| style={[styles.endIcon, styles.centered]} | ||
| testId="external-icon" | ||
| aria-label="Open in new tab" |
There was a problem hiding this comment.
This will unfortunately replace the entire accessible name for links, which isn't what we want. I think you need to combine the textContent with the "open in new tab" text. It also needs to be translatable -- maybe through a prop?
Summary:
Adds an aria-label to the externalIcon set to "Open in new tab". The aria label communicates the "new tab" information to screen reader users when they focus the link.
Issue: WB-2183
Test plan:
Screen.Recording.2025-12-16.at.5.31.41.PM.mov