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

bug(cdk/menu): Trigger does not update ARIA values correctly in OnPush components #27725

Closed
1 task
sbarfurth opened this issue Aug 29, 2023 · 1 comment · Fixed by #27726
Closed
1 task
Labels
Accessibility This issue is related to accessibility (a11y) area: cdk/menu P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent

Comments

@sbarfurth
Copy link
Contributor

Is this a regression?

  • Yes, this behavior used to work in the previous version

The previous version in which this bug was not present was

No response

Description

I recently noticed that the menu trigger (CdkMenuTrigger) misbehaves when inside OnPush components because the OnPush change detection strategy will cascade into child directives implicitly. Specifically, the aria-expanded property will not correctly follow the open sub-menu. After some investigation this seems to simply be caused by the CdkMenuTrigger not using detectChanges() or markForCheck() from the ChangeDetectorRef after opening and closing the menu overlay. The attachment state of the menu overlay is used to populate aria-expanded directly.

This can be worked around by having the OnPush parent listen to (opened) and (closed) events emitted by the trigger, but that is not very scalable since each menu item would then require two additional handlers inside every OnPush component.

I will be sending a pull request that will make the CdkMenuTrigger call relevant methods from the ChangeDetectorRef at appropriate times to support being placed in OnPush components.

Reproduction

StackBlitz link: https://stackblitz.com/edit/components-issue-uc7xxl

Note: In the StackBlitz the style of menu items is changed based on the aria-expanded property of the element. If the element has aria-expanded set to true the item will appear with a black background and white text. This is supposed to make it easy to see how aria-expanded updates.

Steps to reproduce:

  1. Create a an inline CdkMenu or a CdkMenuBar in a component using ChangeDetectionStrategy.OnPush
  2. Add sub-menus to the items of the inline menu
  3. Click any of the items to open its sub-menu
  4. Hover to any other item with a sub-menu

Expected Behavior

When hovering over another item with a sub-menu, the aria-expanded property of the newly hovered item should be set to true and that of the previous item should be set to false. The aria-expanded property should be true for the item that has an open sub-menu at all times. When clicking outside the menu all sub-menus should close and aria-expanded should be false for all items in the menu.

Actual Behavior

The aria-expanded property continues to be true on the initially clicked element regardless of the item hovered and where the open sub-menu is. It also continues to be true for an item even when all sub-menus are closed.

Environment

  • Angular: 16.2.1
  • CDK/Material: 16.2.1
  • Browser(s): Version 116.0.5845.110 (Official Build) (arm64)
  • Operating System (e.g. Windows, macOS, Ubuntu): macOS
@sbarfurth sbarfurth added the needs triage This issue needs to be triaged by the team label Aug 29, 2023
@sbarfurth sbarfurth changed the title bug(CdkMenu): Trigger does not update ARIA values correctly in OnPush components bug(cdk/menu): Trigger does not update ARIA values correctly in OnPush components Aug 29, 2023
sbarfurth added a commit to sbarfurth/components that referenced this issue Aug 29, 2023
…gger

Fixes an issue where menu triggers placed inside components using OnPush change detection
would not have their `aria-expanded` property updated correctly when switching between
sibling triggers.

Fixes angular#27725
@wagnermaciel wagnermaciel added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent Accessibility This issue is related to accessibility (a11y) area: cdk/menu and removed needs triage This issue needs to be triaged by the team labels Aug 31, 2023
crisbeto pushed a commit that referenced this issue Sep 4, 2023
…gger (#27726)

Fixes an issue where menu triggers placed inside components using OnPush change detection
would not have their `aria-expanded` property updated correctly when switching between
sibling triggers.

Fixes #27725

(cherry picked from commit 8eb494e)
crisbeto pushed a commit that referenced this issue Sep 4, 2023
…gger (#27726)

Fixes an issue where menu triggers placed inside components using OnPush change detection
would not have their `aria-expanded` property updated correctly when switching between
sibling triggers.

Fixes #27725
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) area: cdk/menu P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants