Skip to content
5 changes: 5 additions & 0 deletions .changeset/cruel-eels-open.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@spectrum-web-components/menu': minor
---

**Fixed** MenuItem focus stealing from input elements on mouseover by enhanceing MenuItem's `handleMouseover` method to detect when an input element currently has focus and prevent stealing focus in those cases.
73 changes: 72 additions & 1 deletion packages/menu/src/MenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import {
CSSResultArray,
html,
INPUT_COMPONENT_PATTERN,
nothing,
PropertyValues,
TemplateResult,
Expand Down Expand Up @@ -479,13 +480,83 @@ export class MenuItem extends LikeAnchor(
this.id = `sp-menu-item-${randomID()}`;
}
}

handleMouseover(event: MouseEvent): void {
const target = event.target as HTMLElement;
if (target === this) {
this.focus();
// Get the currently focused element within the component's root context
const rootNode = this.getRootNode() as Document | ShadowRoot;
const activeElement = rootNode.activeElement as HTMLElement;

// Only focus this menu item if no input element is currently active
// This prevents interrupting user input in search boxes, text fields, etc.
if (!activeElement || !this.isInputElement(activeElement)) {
this.focus();
}
this.focused = false;
}
}

/**
* Determines if an element is an input field that should retain focus.
* Uses multiple detection strategies to identify input elements generically.
*/
private isInputElement(element: HTMLElement): boolean {
// Check for native HTML input elements
if (this.isNativeInputElement(element)) {
return true;
}

// Check for contenteditable elements (rich text editors)
if (element.contentEditable === 'true') {
return true;
}

// Check for Spectrum Web Components with input-like behavior
if (this.isSpectrumInputComponent(element)) {
return true;
}

return false;
}

/**
* Checks if an element is a native HTML input element.
*/
private isNativeInputElement(element: HTMLElement): boolean {
return (
element instanceof HTMLInputElement ||
element instanceof HTMLTextAreaElement ||
element instanceof HTMLSelectElement
);
}

/**
* Checks if an element is a Spectrum Web Component with input behavior.
* Uses ARIA roles and component patterns for generic detection.
*/
private isSpectrumInputComponent(element: HTMLElement): boolean {
// Check if it's a Spectrum Web Component
if (!element.tagName.startsWith('SP-')) {
return false;
}

// Check ARIA role for input-like behavior
const role = element.getAttribute('role');
const inputRoles = ['textbox', 'searchbox', 'combobox', 'slider'];
if (role && inputRoles.includes(role)) {
return true;
}

// Check for components that typically contain input elements
// This covers components like sp-search, sp-textfield, sp-number-field, etc.
const inputComponentPattern = INPUT_COMPONENT_PATTERN;
if (inputComponentPattern.test(element.tagName)) {
return true;
}

return false;
}
/**
* forward key info from keydown event to parent menu
*/
Expand Down
107 changes: 107 additions & 0 deletions packages/menu/stories/menu.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -484,3 +489,105 @@ export const dynamicRemoval = (): TemplateResult => {
</sp-menu>
`;
};

export const InputsWithMenu = (): TemplateResult => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great test case but I don't think we need a new story for it. If you wanted to replicate the behavior in a story, we could probably use the play functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a comprehensive test coverage for the focus behavior in packages/menu/test/menu.test.ts (lines 848-984) with the test 'does not steal focus from input elements on mouseover' that validates all the same input types and scenarios. Let me know if I am missing something here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main thought with not adding this as a story is that it's a lot of manual instructions for the viewer of the page to complete to get the experience you're demonstrating. Maybe we defer adding any new stories until we have a chance to talk about Storybook docs more as a team?

return html`
<div style="padding: 20px; max-width: 600px;">
<h3>Input Focus Demo</h3>
<p>
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.
</p>

<div
style="display: grid; gap: 16px; grid-template-columns: 1fr 1fr; margin-bottom: 20px;"
>
<!-- Search Input -->
<div>
<label for="demo-search">Search:</label>
<sp-search
id="demo-search"
placeholder="Search input..."
style="width: 100%; margin-top: 8px;"
></sp-search>
</div>

<!-- Textfield Input -->
<div>
<label for="demo-textfield">Textfield:</label>
<sp-textfield
id="demo-textfield"
placeholder="Textfield input..."
style="width: 100%; margin-top: 8px;"
></sp-textfield>
</div>

<!-- Number Field Input -->
<div>
<label for="demo-number">Number Field:</label>
<sp-number-field
id="demo-number"
placeholder="Number input..."
style="width: 100%; margin-top: 8px;"
></sp-number-field>
</div>

<!-- Combobox Input -->
<div>
<label for="demo-combobox">Combobox:</label>
<sp-combobox
id="demo-combobox"
placeholder="Combobox input..."
style="width: 100%; margin-top: 8px;"
></sp-combobox>
</div>

<!-- Color Field Input -->
<div>
<label for="demo-color">Color Field:</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<label for="demo-color">Color Field:</label>
<label for="demo-color">Color field:</label>

Minor nit: try to use sentence case headings in our demos to reinforce the design system content standards where possible!

<sp-color-field
id="demo-color"
placeholder="Color input..."
style="width: 100%; margin-top: 8px;"
></sp-color-field>
</div>

<!-- Native Input -->
<div>
<label for="demo-native">Native Input:</label>
<input
id="demo-native"
placeholder="Native input..."
style="width: 100%; margin-top: 8px; padding: 8px; border: 1px solid #ccc; border-radius: 4px;"
/>
</div>
</div>

<sp-popover open>
<sp-menu>
<sp-menu-item>Search Results</sp-menu-item>
<sp-menu-item>Recent Searches</sp-menu-item>
<sp-menu-item>Saved Searches</sp-menu-item>
<sp-menu-item>Advanced Search</sp-menu-item>
<sp-menu-item>Search Settings</sp-menu-item>
<sp-menu-item>Clear History</sp-menu-item>
</sp-menu>
</sp-popover>
</div>
`;
};

InputsWithMenu.parameters = {
tags: ['!dev'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

};

InputsWithMenu.swc_vrt = {
skip: true,
};

InputsWithMenu.parameters = {
// Disables Chromatic's snapshotting on a global level
chromatic: { disableSnapshot: true },
};
144 changes: 144 additions & 0 deletions packages/menu/test/menu.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ import '@spectrum-web-components/menu/sp-menu-divider.js';
import '@spectrum-web-components/menu/sp-menu-group.js';
import '@spectrum-web-components/menu/sp-menu-item.js';
import '@spectrum-web-components/menu/sp-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';
import '@spectrum-web-components/popover/sp-popover.js';
import { isFirefox, isWebKit } from '@spectrum-web-components/shared';
import { sendKeys } from '@web/test-runner-commands';
import { spy } from 'sinon';
Expand Down Expand Up @@ -838,4 +844,142 @@ describe('Menu', () => {
// Test that the component can be disconnected without errors
el.remove();
});

it('does not steal focus from input elements on mouseover', async () => {
const el = await fixture<Menu>(html`
<div>
<sp-search
id="test-search"
placeholder="Search input..."
></sp-search>
<sp-textfield
id="test-textfield"
placeholder="Textfield input..."
></sp-textfield>
<sp-number-field
id="test-number"
placeholder="Number input..."
></sp-number-field>
<sp-combobox
id="test-combobox"
placeholder="Combobox input..."
></sp-combobox>
<sp-color-field
id="test-color"
placeholder="Color input..."
></sp-color-field>
<input id="test-native" placeholder="Native input..." />

<sp-popover open>
<sp-menu>
<sp-menu-item id="menu-item-1">
Menu Item 1
</sp-menu-item>
<sp-menu-item id="menu-item-2">
Menu Item 2
</sp-menu-item>
<sp-menu-item id="menu-item-3">
Menu Item 3
</sp-menu-item>
</sp-menu>
</sp-popover>
</div>
`);

await elementUpdated(el);

const searchInput = el.querySelector('#test-search') as HTMLElement;
const textfieldInput = el.querySelector(
'#test-textfield'
) as HTMLElement;
const numberInput = el.querySelector('#test-number') as HTMLElement;
const comboboxInput = el.querySelector('#test-combobox') as HTMLElement;
const colorInput = el.querySelector('#test-color') as HTMLElement;
const nativeInput = el.querySelector(
'#test-native'
) as HTMLInputElement;

const menuItem1 = el.querySelector('#menu-item-1') as MenuItem;
const menuItem2 = el.querySelector('#menu-item-2') as MenuItem;
const menuItem3 = el.querySelector('#menu-item-3') as MenuItem;

// Test with sp-search
searchInput.focus();
await elementUpdated(el);
expect(document.activeElement).to.equal(searchInput);

await sendMouseTo(menuItem1);
await elementUpdated(el);
expect(document.activeElement).to.equal(
searchInput,
'sp-search should retain focus'
);

await sendMouseTo(menuItem2);
await elementUpdated(el);
expect(document.activeElement).to.equal(
searchInput,
'sp-search should retain focus'
);

// Test with sp-textfield
textfieldInput.focus();
await elementUpdated(el);
expect(document.activeElement).to.equal(textfieldInput);

await sendMouseTo(menuItem1);
await elementUpdated(el);
expect(document.activeElement).to.equal(
textfieldInput,
'sp-textfield should retain focus'
);

// Test with sp-number-field
numberInput.focus();
await elementUpdated(el);
expect(document.activeElement).to.equal(numberInput);

await sendMouseTo(menuItem2);
await elementUpdated(el);
expect(document.activeElement).to.equal(
numberInput,
'sp-number-field should retain focus'
);

// Test with sp-combobox
comboboxInput.focus();
await elementUpdated(el);
expect(document.activeElement).to.equal(comboboxInput);

await sendMouseTo(menuItem3);
await elementUpdated(el);
expect(document.activeElement).to.equal(
comboboxInput,
'sp-combobox should retain focus'
);

// Test with sp-color-field
colorInput.focus();
await elementUpdated(el);
expect(document.activeElement).to.equal(colorInput);

await sendMouseTo(menuItem1);
await elementUpdated(el);
expect(document.activeElement).to.equal(
colorInput,
'sp-color-field should retain focus'
);

// Test with native input
nativeInput.focus();
await elementUpdated(el);
expect(document.activeElement).to.equal(nativeInput);

await sendMouseTo(menuItem2);
await elementUpdated(el);
expect(document.activeElement).to.equal(
nativeInput,
'native input should retain focus'
);
});
});
Loading
Loading