diff --git a/packages/menu/src/Menu.ts b/packages/menu/src/Menu.ts index 28cbfb6f743..bb6dc650f77 100644 --- a/packages/menu/src/Menu.ts +++ b/packages/menu/src/Menu.ts @@ -72,6 +72,16 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { protected rovingTabindexController?: RovingTabindexController; + /** + * Tracks the last hovered menu item for keyboard navigation + */ + private hoveredItem?: MenuItem; + + /** + * Tracks whether we have a global keyboard listener active + */ + private hasGlobalKeyListener = false; + /** * iPad scroll detection properties * @@ -103,30 +113,30 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { /** * Minimum vertical movement (in pixels) required to trigger scrolling detection. - * + * * This threshold is consistent with other components in the design system: * - Card component uses 10px for click vs. drag detection * - Menu component uses 10px for scroll vs. selection detection - * + * * The 10px threshold is carefully chosen to: * - Allow for natural finger tremor and accidental touches * - Distinguish between intentional scroll gestures and taps * - Provide consistent behavior across the platform - * + * * @see {@link packages/card/src/Card.ts} for similar threshold usage */ private scrollThreshold = 10; // pixels /** * Maximum time (in milliseconds) for a movement to be considered scrolling. - * + * * This threshold is consistent with other timing values in the design system: * - Longpress duration: 300ms (ActionButton, LongpressController) * - Scroll detection: 300ms (Menu component) - * + * * Quick movements within this timeframe are likely intentional scrolls, * while slower movements are more likely taps or selections. - * + * * @see {@link packages/action-button/src/ActionButton.ts} for longpress duration * @see {@link packages/overlay/src/LongpressController.ts} for longpress duration */ @@ -431,6 +441,26 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { new RovingTabindexController(this, { direction: 'vertical', focusInIndex: (elements: MenuItem[] | undefined) => { + // If we have a hovered item and no current focus, use it for transition + if ( + this.hoveredItem && + elements?.includes(this.hoveredItem) && + !this.matches(':focus-within') + ) { + const hoveredIndex = elements.indexOf( + this.hoveredItem + ); + if ( + hoveredIndex !== -1 && + !this.hoveredItem.disabled + ) { + // Clear hovered item after using it for focus transition + this.hoveredItem = undefined; + return hoveredIndex; + } + } + + // Fall back to original logic (selected item or first enabled) let firstEnabledIndex = -1; const firstSelectedIndex = elements?.findIndex( (el, index) => { @@ -524,6 +554,51 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { } } + /** + * Sets the hovered item for keyboard navigation without stealing focus + */ + public setHoveredItem(item: MenuItem): void { + this.hoveredItem = item; + + // Add global keyboard listener when we have a hovered item + if (item && !this.hasGlobalKeyListener) { + this.hasGlobalKeyListener = true; + document.addEventListener( + 'keydown', + this.handleGlobalKeydown, + true + ); + } + } + + /** + * Handle global keyboard events for hover-to-focus transition + */ + private handleGlobalKeydown = (event: KeyboardEvent): void => { + if ( + [ + 'ArrowUp', + 'ArrowDown', + 'ArrowLeft', + 'ArrowRight', + 'Home', + 'End', + ].includes(event.key) + ) { + if (this.hoveredItem) { + // Focus the menu to enable keyboard navigation + this.focus(); + // Clear the global listener + this.hasGlobalKeyListener = false; + document.removeEventListener( + 'keydown', + this.handleGlobalKeydown, + true + ); + } + } + }; + /** * Handles touchstart events for iPad scroll detection. * @@ -576,7 +651,7 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { * Resets the scrolling state after a short delay (100ms) to allow for * any final touch events to be processed. This delay prevents immediate * state changes that could interfere with the selection logic. - * + * * The 100ms delay is consistent with the design system's approach to * touch event handling and ensures that any final touch events or * gesture recognition can complete before the scrolling state is reset. @@ -1089,6 +1164,17 @@ export class Menu extends SizedMixin(SpectrumElement, { noDefaultSize: true }) { this.selectedItemsMap.clear(); this.childItemSet.clear(); this.descendentOverlays = new Map(); + + // Clean up global listener if active + if (this.hasGlobalKeyListener) { + document.removeEventListener( + 'keydown', + this.handleGlobalKeydown, + true + ); + this.hasGlobalKeyListener = false; + } + super.disconnectedCallback(); } diff --git a/packages/menu/src/MenuItem.ts b/packages/menu/src/MenuItem.ts index 05f899b1f1a..2da8e5f4c31 100644 --- a/packages/menu/src/MenuItem.ts +++ b/packages/menu/src/MenuItem.ts @@ -482,7 +482,8 @@ export class MenuItem extends LikeAnchor( handleMouseover(event: MouseEvent): void { const target = event.target as HTMLElement; if (target === this) { - this.focus(); + // Track hovered item for keyboard navigation without stealing focus + this.menuData.parentMenu?.setHoveredItem(this); this.focused = false; } } diff --git a/packages/menu/stories/menu.stories.ts b/packages/menu/stories/menu.stories.ts index bebe33f6148..992d1a1d1ad 100644 --- a/packages/menu/stories/menu.stories.ts +++ b/packages/menu/stories/menu.stories.ts @@ -28,6 +28,11 @@ import '@spectrum-web-components/icons-workflow/icons/sp-icon-export.js'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-folder-open.js'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-share.js'; import '@spectrum-web-components/icons-workflow/icons/sp-icon-show-menu.js'; +import '@spectrum-web-components/search/sp-search.js'; +import '@spectrum-web-components/textfield/sp-textfield.js'; +import '@spectrum-web-components/number-field/sp-number-field.js'; +import '@spectrum-web-components/combobox/sp-combobox.js'; +import '@spectrum-web-components/color-field/sp-color-field.js'; export default { component: 'sp-menu', @@ -484,3 +489,105 @@ export const dynamicRemoval = (): TemplateResult => { `; }; + +export const InputsWithMenu = (): TemplateResult => { + return html` +
+

Input Focus Demo

+

+ Try typing in any input field below, then hover over the menu + items. The input should maintain focus and not be interrupted. + This demonstrates the fix for focus stealing from all supported + input types. +

+ +
+ +
+ + +
+ + +
+ + +
+ + +
+ + +
+ + +
+ + +
+ + +
+ + +
+ + +
+ + +
+
+ + + + Search Results + Recent Searches + Saved Searches + Advanced Search + Search Settings + Clear History + + +
+ `; +}; + +InputsWithMenu.parameters = { + tags: ['!dev'], +}; + +InputsWithMenu.swc_vrt = { + skip: true, +}; + +InputsWithMenu.parameters = { + // Disables Chromatic's snapshotting on a global level + chromatic: { disableSnapshot: true }, +}; diff --git a/packages/menu/test/menu.test.ts b/packages/menu/test/menu.test.ts index 050fc61cd5a..988350c8f51 100644 --- a/packages/menu/test/menu.test.ts +++ b/packages/menu/test/menu.test.ts @@ -82,7 +82,6 @@ describe('Menu', () => { Disabled item 2 `); - const firstItem = el.querySelector('sp-menu-item') as MenuItem; await elementUpdated(el); expect(document.activeElement).to.not.equal(el); @@ -91,7 +90,6 @@ describe('Menu', () => { await elementUpdated(el); expect(document.activeElement).to.not.equal(el); expect(focusinSpy.callCount).to.equal(0); - firstItem.focus(); await elementUpdated(el); expect(document.activeElement).to.not.equal(el); expect(focusinSpy.callCount).to.equal(0); @@ -338,24 +336,37 @@ describe('Menu', () => { expect(document.activeElement).to.equal(firstItem); expect(firstItem.focused, 'first item focused').to.be.true; + // Hover over second item - should not steal focus await sendMouseTo(secondItem); + await elementUpdated(el); + // Active element should remain on first item since focus is not stolen on hover expect(document.activeElement, 'active element after hover').to.equal( - secondItem + firstItem ); - expect(document.activeElement).to.equal(secondItem); + expect(firstItem.focused, 'first item should still be focused').to.be + .true; expect( secondItem.focused, 'second item should not have focus styling on hover' ).to.be.false; - await sendKeys({ press: 'ArrowUp' }); + // Test keyboard navigation still works + await sendKeys({ press: 'ArrowDown' }); + await elementUpdated(el); - expect(document.activeElement).to.equal(firstItem); expect( - firstItem.focused, - 'first item should have focus styling after keyboard' + document.activeElement, + 'active element after arrow down' + ).to.equal(secondItem); + expect( + secondItem.focused, + 'second item should be focused after keyboard' ).to.be.true; + expect( + firstItem.focused, + 'first item should not be focused after keyboard' + ).to.be.false; }); it('handle focus and late descendant additions', async () => {