Switch to using d2l-link in object-property-item-link #5183
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| } | ||
|
|
||
| render() { | ||
| return html` | ||
| ${this._renderIcon()} | ||
| ${!this.skeleton ? html` | ||
| <a | ||
| <d2l-link | ||
| aria-label="${this.text}" |
There was a problem hiding this comment.
The aria-label shouldn't be needed? I'd expect its label to come from its internals via renderText().
There was a problem hiding this comment.
I'd mostly added it since it was a property of d2l-link, but you're right that it should just be handled within renderText. Though it does beg the question, if this component should have some property to provide more context to screen reader users like how aria-label is currently doing for d2l-link. I'm good with just removing it for now though!
There was a problem hiding this comment.
Setting aria-label in this way actually replaces the contents, so there wouldn't be more context in the same way aria-description might provide.
Having aria-label on <d2l-link> as a passthrough to its underlying <a>'s aria-label was a mistake we made initially. All the aria-* attribute are obviously native attributes and aXe has actually started complaining recently when we put them on custom elements.
What we should have done (and have correct in newer things) is to use a combination of text (or label) and text-hidden/label-hidden to flip that to the aria-label of the native element. And then add something like a description attribute to go into the aria-description.
ChrisLabattD2L
deleted the
clabatt/fix-object-property-list-link-new-tab
branch
|
🎉 This PR is included in version 3.74.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
GAUD-7228
Due to not using the
d2l-linkcomponent, whentarget="_blank", we weren't showing the "open in a new tab" functionality, so I've switched theobject-property-item-linkto use thed2l-linkcomponent instead of just the styling.