accesibility issue #2704
accesibility issue #2704
Conversation
There was a problem hiding this comment.
@CrisGuzmanS : Thanks for the contribution. I think there's an easier way to get the aria-expanded attribute to work though. You shouldn't need to change its value in a more "manual" fashion in the code. See comments inline.
| (mouseleave)="deactivateSection($event)"> | ||
| (mouseleave)="deactivateSection($event)" | ||
| aria-haspopup="menu" | ||
| aria-expanded="false"> |
There was a problem hiding this comment.
Rather than hardcoding this to false and changing the value in the component.ts code, I think you can just use isActive here (similar to the other attributes on this same <div>. Something like this should work:
[attr.aria-expanded]="isActive"
I haven't tested it, but you shouldn't need to hardcode this.
| @@ -39,11 +39,17 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp | |||
| * @param {Event} event The user event that triggered this function | |||
| */ | |||
| activateSection(event): void { | |||
|
|
|||
| console.log(event.target); | |||
There was a problem hiding this comment.
This looks like it was for debugging & should be removed.
| super.activateSection(event); | ||
| targetElement.setAttribute('aria-expanded', 'true'); |
There was a problem hiding this comment.
There are a bunch of failures here from yarn lint. It looks like you have trailing spaces on several lines in this file. Those should be removed until you get yarn lint to pass locally.
tdonohue
added
bug
accessibility
1 APPROVAL
|
Hi @CrisGuzmanS @tdonohue |
|
@davide-negretti : Thanks for pointing out the conflicts. Since this PR isn't complete & yours is, I'm going to close this PR in favor of #2676 . Apologies @CrisGuzmanS , as I didn't notice the conflict earlier myself when I gave this a quick review. |
References
Add references/links to any related issues or PRs. These may include:
pr-number(if a REST API PR is required to test this)Description
Short summary of changes (1-2 sentences).
Instructions for Reviewers
i added attributes in the html component, and the aria-expanded equals to true.
List of changes in this PR:
Include guidance for how to test or review your PR.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
yarn lintyarn check-circ-deps)