Navbar: Add aria-label attribute
#2379
Conversation
✅ Deploy Preview for moduswebcomponents ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
coliff
changed the title
aria-label attribute
| @@ -364,7 +367,7 @@ export class ModusNavbar { | |||
|
|
|||
| return ( | |||
| <Host id={this.componentId}> | |||
| <nav class={`${direction} ${shadow} ${variant}`}> | |||
| <nav class={`${direction} ${shadow} ${variant}`} aria-label={this.ariaLabel}> | |||
There was a problem hiding this comment.
If no aria-label is set then it renders as <nav class aria-label> - however it shouldn't show aria-label at all if not specified. Can you fix that please? By default it shouldn't have an aria-label at all.
There was a problem hiding this comment.
I was not able to reproduce this locally. When running locally, I got desired behavior. I added some unit tests for these user journeys.
There was a problem hiding this comment.
Adding aria-label to the <modus-navbar> is problematic, I think it should be on the <nav> only.
See Accessibility issues at: https://deploy-preview-2379--moduswebcomponents.netlify.app/?path=/story/components-navbar--default
|
I changed the prop name to |
|
It's still rendering empty
|
|
Hi @austinoneil - the outstanding issue on this has been resolved now with your merged PR (empty aria-label fix) so once the conflicts are resolved here we can merge too. thanks :-) |
|
doesn't seem to work: |
|
@coliff fixed. |
Description
Add
aria-labelattribute to navbar.References #2212
Type of change
How Has This Been Tested?
e2e tests, inspect element on storybook
Checklist