From a2dc5222352f8422f68c17576a17bc7e159b8ec3 Mon Sep 17 00:00:00 2001 From: Karim Daher Date: Thu, 21 May 2026 01:23:34 +0300 Subject: [PATCH] fix(aria/menu): prevent stale active state on menubar items The menubar's `activeItem` signal is set to the first item on init so the roving tabindex has an anchor, but `data-active` was bound directly to this anchor. That caused two visual bugs: - The first menubar item rendered as active before any interaction. - After hovering over an item and moving the pointer outside the menubar, the last hovered item stayed visually active because no mouseout handler reset the anchor. Separate the tabstop concern from the visual highlight by tracking `isHovered` on `MenuBarPattern` and clearing it on mouseout, and by gating `MenuItemPattern.active` for menubar parents on whether the menubar is focused, hovered, or the item is expanded. The roving tabindex behaviour is unaffected. --- src/aria/menu/menu-bar.ts | 1 + src/aria/menu/menu.spec.ts | 99 +++++++++++++++++++++++++++++++++++ src/aria/private/menu/menu.ts | 24 ++++++++- 3 files changed, 123 insertions(+), 1 deletion(-) diff --git a/src/aria/menu/menu-bar.ts b/src/aria/menu/menu-bar.ts index 8c92f1d25996..30e8f15ab6b6 100644 --- a/src/aria/menu/menu-bar.ts +++ b/src/aria/menu/menu-bar.ts @@ -62,6 +62,7 @@ import {MENU_COMPONENT} from './menu-tokens'; '[attr.tabindex]': '_pattern.tabIndex()', '(keydown)': '_pattern.onKeydown($event)', '(mouseover)': '_pattern.onMouseOver($event)', + '(mouseout)': '_pattern.onMouseOut($event)', '(click)': '_pattern.onClick($event)', '(focusin)': '_pattern.onFocusIn()', '(focusout)': '_pattern.onFocusOut($event)', diff --git a/src/aria/menu/menu.spec.ts b/src/aria/menu/menu.spec.ts index 1306cfacd8bc..8525b0cb2a15 100644 --- a/src/aria/menu/menu.spec.ts +++ b/src/aria/menu/menu.spec.ts @@ -759,6 +759,23 @@ describe('Menu Trigger Pattern', () => { focusout(getMenu()!, document.body); expect(isExpanded()).toBe(false); }); + + it('should close programmatically without refocusing the trigger', () => { + const trigger = getTrigger(); + const triggerDirective = fixture.debugElement + .query(By.directive(MenuTrigger)) + .injector.get(MenuTrigger); + + click(trigger); + expect(isExpanded()).toBe(true); + expect(document.activeElement).toBe(getItem('Apple')); + + triggerDirective.close(); + fixture.detectChanges(); + + expect(isExpanded()).toBe(false); + expect(document.activeElement).not.toBe(trigger); + }); }); describe('Selection', () => { @@ -1221,6 +1238,88 @@ describe('Menu Bar Pattern', () => { expect(document.activeElement).toBe(undo); }); }); + + describe('Active state', () => { + const mouseout = (element: Element, relatedTarget?: EventTarget) => { + element.dispatchEvent(new MouseEvent('mouseout', {bubbles: true, relatedTarget} as any)); + fixture.detectChanges(); + }; + + function isActive(text: string): boolean { + return getMenuBarItem(text)?.getAttribute('data-active') === 'true'; + } + + it('should not mark any menubar item as active on initial render', () => { + TestBed.configureTestingModule({providers: [provideFakeDirectionality('ltr')]}); + fixture = TestBed.createComponent(MenuBarExample); + fixture.detectChanges(); + + expect(isActive('File')).toBe(false); + expect(isActive('Edit')).toBe(false); + expect(isActive('View')).toBe(false); + }); + + it('should mark a menubar item as active when hovered', () => { + TestBed.configureTestingModule({providers: [provideFakeDirectionality('ltr')]}); + fixture = TestBed.createComponent(MenuBarExample); + fixture.detectChanges(); + + mouseover(getMenuBarItem('Edit')!); + + expect(isActive('Edit')).toBe(true); + expect(isActive('File')).toBe(false); + }); + + it('should clear the active state when the pointer leaves the menubar', () => { + TestBed.configureTestingModule({providers: [provideFakeDirectionality('ltr')]}); + fixture = TestBed.createComponent(MenuBarExample); + fixture.detectChanges(); + + const edit = getMenuBarItem('Edit')!; + mouseover(edit); + expect(isActive('Edit')).toBe(true); + + mouseout(edit, document.body); + + expect(isActive('Edit')).toBe(false); + expect(isActive('File')).toBe(false); + expect(isActive('View')).toBe(false); + }); + + it('should keep the active state while the pointer moves between menubar items', () => { + TestBed.configureTestingModule({providers: [provideFakeDirectionality('ltr')]}); + fixture = TestBed.createComponent(MenuBarExample); + fixture.detectChanges(); + + const file = getMenuBarItem('File')!; + const edit = getMenuBarItem('Edit')!; + + mouseover(file); + expect(isActive('File')).toBe(true); + + mouseout(file, edit); + mouseover(edit); + + expect(isActive('Edit')).toBe(true); + }); + + it('should mark the focused menubar item as active', () => { + setupMenu(); + fixture.detectChanges(); + + expect(isActive('File')).toBe(true); + }); + + it('should clear the active state when focus leaves the menubar', () => { + setupMenu(); + fixture.detectChanges(); + expect(isActive('File')).toBe(true); + + focusout(getMenuBarItem('File')!, document.body); + + expect(isActive('File')).toBe(false); + }); + }); }); @Component({ diff --git a/src/aria/private/menu/menu.ts b/src/aria/private/menu/menu.ts index 70657833c7ec..39ec6d40d15c 100644 --- a/src/aria/private/menu/menu.ts +++ b/src/aria/private/menu/menu.ts @@ -503,6 +503,9 @@ export class MenuBarPattern { /** Whether the menubar or any of its children are currently focused. */ readonly isFocused = signal(false); + /** Whether the pointer is currently over a menubar item. */ + readonly isHovered = signal(false); + /** Whether the menubar has been interacted with. */ readonly hasBeenInteracted = signal(false); @@ -567,10 +570,20 @@ export class MenuBarPattern { const item = this.inputs.items().find(i => i.element()?.contains(event.target as Node)); if (item) { + this.isHovered.set(true); this.goto(item, {focusElement: this.isFocused()}); } } + /** Handles mouseout events for the menu bar. */ + onMouseOut(event: MouseEvent) { + const relatedTarget = event.relatedTarget as Node | null; + + if (!this.inputs.element()?.contains(relatedTarget)) { + this.isHovered.set(false); + } + } + /** Handles focusin events for the menu bar. */ onFocusIn() { this.isFocused.set(true); @@ -773,7 +786,16 @@ export class MenuItemPattern implements ListItem { readonly element: SignalLike; /** Whether the menu item is active. */ - readonly active = computed(() => this.inputs.parent()?.inputs.activeItem() === this); + readonly active = computed(() => { + const parent = this.inputs.parent(); + if (parent?.inputs.activeItem() !== this) { + return false; + } + if (parent instanceof MenuBarPattern) { + return parent.isFocused() || parent.isHovered() || this._expanded(); + } + return true; + }); /** Whether the menu item has received interaction. */ readonly hasBeenInteracted = signal(false);