Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
91 changes: 91 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,89 @@ 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>
<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>
`;
};
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'
);
});
});
29 changes: 29 additions & 0 deletions tools/base/src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Copyright 2025 Adobe. All rights reserved.
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. You may obtain a copy
* of the License at http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/

/**
* Regular expression pattern to match Spectrum Web Components input elements.
* Used to identify components that should maintain focus during menu interactions.
*/
export const INPUT_COMPONENT_PATTERN =
Copy link
Contributor

Choose a reason for hiding this comment

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

@rubencarvalho Let me know what you think about this! I dig it since we do reuse this logic and it feels like a more global concept to have updated in one place.

/^(SP-SEARCH|SP-TEXTFIELD|SP-NUMBER-FIELD|SP-COMBOBOX|SP-COLOR-FIELD)$/;

/**
* Array of input component tag names for easier iteration and maintenance.
*/
export const INPUT_COMPONENT_TAGS = [
'SP-SEARCH',
'SP-TEXTFIELD',
'SP-NUMBER-FIELD',
'SP-COMBOBOX',
'SP-COLOR-FIELD',
] as const;
1 change: 1 addition & 0 deletions tools/base/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@

export * from './Base.js';
export * from './sizedMixin.js';
export * from './constants.js';
export * from 'lit';
Loading