Skip to content

Commit 4880da4

Browse files
RajdeepcRajdeep ChandraRajdeep ChandraRajdeep Chandra
authored
fix(menu): MenuItem focus stealing from input elements on mouseover (#5732)
* fix(menu): added check to find focused element within root context * fix(menu): added story * fix(menu): added test * chore(menu): added changeset * fix(menu): added global const for component input pattern * fix(menu): remove delimiter from the regexp constructor * chore: skipped prod and vrt tests on the new story * chore: fix tests helpers * fix: check for cross root boundary * fix: code comments --------- Co-authored-by: Rajdeep Chandra <rajdeepchandra@Rajdeeps-MacBook-Pro-6.local> Co-authored-by: Rajdeep Chandra <rajdeepchandra@rajdeeps-mbp-7.macromedia.com> Co-authored-by: Rajdeep Chandra <rajdeepchandra@Rajdeeps-MacBook-Pro-7.local>
1 parent 7d23140 commit 4880da4

File tree

6 files changed

+377
-1
lines changed

6 files changed

+377
-1
lines changed

.changeset/cruel-eels-open.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@spectrum-web-components/menu': minor
3+
---
4+
5+
**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.

packages/menu/src/MenuItem.ts

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import {
1414
CSSResultArray,
1515
html,
16+
INPUT_COMPONENT_PATTERN,
1617
nothing,
1718
PropertyValues,
1819
TemplateResult,
@@ -479,13 +480,101 @@ export class MenuItem extends LikeAnchor(
479480
this.id = `sp-menu-item-${randomID()}`;
480481
}
481482
}
483+
484+
private getActiveElementSafely(): HTMLElement | null {
485+
let root = this.getRootNode() as Document | ShadowRoot;
486+
let activeElement = root.activeElement as HTMLElement;
487+
488+
// If no active element in current context and we're in shadow DOM,
489+
// traverse up to find the document-level active element
490+
if (!activeElement && root !== document) {
491+
while (root && root !== document && 'host' in root) {
492+
root = (root as ShadowRoot).host.getRootNode() as
493+
| Document
494+
| ShadowRoot;
495+
activeElement = root.activeElement as HTMLElement;
496+
if (activeElement) break;
497+
}
498+
}
499+
500+
return activeElement;
501+
}
502+
482503
handleMouseover(event: MouseEvent): void {
483504
const target = event.target as HTMLElement;
484505
if (target === this) {
485-
this.focus();
506+
// Check for active input elements across shadow boundaries
507+
const activeElement = this.getActiveElementSafely();
508+
509+
// Only focus this menu item if no input element is currently active
510+
// This prevents interrupting user input in search boxes, text fields, etc.
511+
if (!activeElement || !this.isInputElement(activeElement)) {
512+
this.focus();
513+
}
486514
this.focused = false;
487515
}
488516
}
517+
518+
/**
519+
* Determines if an element is an input field that should retain focus.
520+
* Uses multiple detection strategies to identify input elements generically.
521+
*/
522+
private isInputElement(element: HTMLElement): boolean {
523+
// Check for native HTML input elements
524+
if (this.isNativeInputElement(element)) {
525+
return true;
526+
}
527+
528+
// Check for contenteditable elements (rich text editors)
529+
if (element.contentEditable === 'true') {
530+
return true;
531+
}
532+
533+
// Check for Spectrum Web Components with input-like behavior
534+
if (this.isSpectrumInputComponent(element)) {
535+
return true;
536+
}
537+
538+
return false;
539+
}
540+
541+
/**
542+
* Checks if an element is a native HTML input element.
543+
*/
544+
private isNativeInputElement(element: HTMLElement): boolean {
545+
return (
546+
element instanceof HTMLInputElement ||
547+
element instanceof HTMLTextAreaElement ||
548+
element instanceof HTMLSelectElement
549+
);
550+
}
551+
552+
/**
553+
* Checks if an element is a Spectrum Web Component with input behavior.
554+
* Uses ARIA roles and component patterns for generic detection.
555+
*/
556+
private isSpectrumInputComponent(element: HTMLElement): boolean {
557+
// Check if it's a Spectrum Web Component
558+
if (!element.tagName.startsWith('SP-')) {
559+
return false;
560+
}
561+
562+
// Check ARIA role for input-like behavior
563+
const role = element.getAttribute('role');
564+
const inputRoles = ['textbox', 'searchbox', 'combobox', 'slider'];
565+
if (role && inputRoles.includes(role)) {
566+
return true;
567+
}
568+
569+
// Check for components that typically contain input elements
570+
// This covers components like sp-search, sp-textfield, sp-number-field, etc.
571+
const inputComponentPattern = INPUT_COMPONENT_PATTERN;
572+
if (inputComponentPattern.test(element.tagName)) {
573+
return true;
574+
}
575+
576+
return false;
577+
}
489578
/**
490579
* forward key info from keydown event to parent menu
491580
*/

packages/menu/stories/menu.stories.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ import '@spectrum-web-components/icons-workflow/icons/sp-icon-export.js';
2828
import '@spectrum-web-components/icons-workflow/icons/sp-icon-folder-open.js';
2929
import '@spectrum-web-components/icons-workflow/icons/sp-icon-share.js';
3030
import '@spectrum-web-components/icons-workflow/icons/sp-icon-show-menu.js';
31+
import '@spectrum-web-components/search/sp-search.js';
32+
import '@spectrum-web-components/textfield/sp-textfield.js';
33+
import '@spectrum-web-components/number-field/sp-number-field.js';
34+
import '@spectrum-web-components/combobox/sp-combobox.js';
35+
import '@spectrum-web-components/color-field/sp-color-field.js';
3136

3237
export default {
3338
component: 'sp-menu',
@@ -484,3 +489,105 @@ export const dynamicRemoval = (): TemplateResult => {
484489
</sp-menu>
485490
`;
486491
};
492+
493+
export const InputsWithMenu = (): TemplateResult => {
494+
return html`
495+
<div style="padding: 20px; max-width: 600px;">
496+
<h3>Input Focus Demo</h3>
497+
<p>
498+
Try typing in any input field below, then hover over the menu
499+
items. The input should maintain focus and not be interrupted.
500+
This demonstrates the fix for focus stealing from all supported
501+
input types.
502+
</p>
503+
504+
<div
505+
style="display: grid; gap: 16px; grid-template-columns: 1fr 1fr; margin-bottom: 20px;"
506+
>
507+
<!-- Search Input -->
508+
<div>
509+
<label for="demo-search">Search:</label>
510+
<sp-search
511+
id="demo-search"
512+
placeholder="Search input..."
513+
style="width: 100%; margin-top: 8px;"
514+
></sp-search>
515+
</div>
516+
517+
<!-- Textfield Input -->
518+
<div>
519+
<label for="demo-textfield">Textfield:</label>
520+
<sp-textfield
521+
id="demo-textfield"
522+
placeholder="Textfield input..."
523+
style="width: 100%; margin-top: 8px;"
524+
></sp-textfield>
525+
</div>
526+
527+
<!-- Number Field Input -->
528+
<div>
529+
<label for="demo-number">Number Field:</label>
530+
<sp-number-field
531+
id="demo-number"
532+
placeholder="Number input..."
533+
style="width: 100%; margin-top: 8px;"
534+
></sp-number-field>
535+
</div>
536+
537+
<!-- Combobox Input -->
538+
<div>
539+
<label for="demo-combobox">Combobox:</label>
540+
<sp-combobox
541+
id="demo-combobox"
542+
placeholder="Combobox input..."
543+
style="width: 100%; margin-top: 8px;"
544+
></sp-combobox>
545+
</div>
546+
547+
<!-- Color Field Input -->
548+
<div>
549+
<label for="demo-color">Color Field:</label>
550+
<sp-color-field
551+
id="demo-color"
552+
placeholder="Color input..."
553+
style="width: 100%; margin-top: 8px;"
554+
></sp-color-field>
555+
</div>
556+
557+
<!-- Native Input -->
558+
<div>
559+
<label for="demo-native">Native Input:</label>
560+
<input
561+
id="demo-native"
562+
placeholder="Native input..."
563+
style="width: 100%; margin-top: 8px; padding: 8px; border: 1px solid #ccc; border-radius: 4px;"
564+
/>
565+
</div>
566+
</div>
567+
568+
<sp-popover open>
569+
<sp-menu>
570+
<sp-menu-item>Search Results</sp-menu-item>
571+
<sp-menu-item>Recent Searches</sp-menu-item>
572+
<sp-menu-item>Saved Searches</sp-menu-item>
573+
<sp-menu-item>Advanced Search</sp-menu-item>
574+
<sp-menu-item>Search Settings</sp-menu-item>
575+
<sp-menu-item>Clear History</sp-menu-item>
576+
</sp-menu>
577+
</sp-popover>
578+
</div>
579+
`;
580+
};
581+
582+
InputsWithMenu.parameters = {
583+
tags: ['!dev'],
584+
};
585+
586+
InputsWithMenu.swc_vrt = {
587+
skip: true,
588+
};
589+
590+
InputsWithMenu.parameters = {
591+
// Disables Chromatic's snapshotting on a global level
592+
chromatic: { disableSnapshot: true },
593+
};

packages/menu/test/menu.test.ts

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ import '@spectrum-web-components/menu/sp-menu-divider.js';
2323
import '@spectrum-web-components/menu/sp-menu-group.js';
2424
import '@spectrum-web-components/menu/sp-menu-item.js';
2525
import '@spectrum-web-components/menu/sp-menu.js';
26+
import '@spectrum-web-components/search/sp-search.js';
27+
import '@spectrum-web-components/textfield/sp-textfield.js';
28+
import '@spectrum-web-components/number-field/sp-number-field.js';
29+
import '@spectrum-web-components/combobox/sp-combobox.js';
30+
import '@spectrum-web-components/color-field/sp-color-field.js';
31+
import '@spectrum-web-components/popover/sp-popover.js';
2632
import { isFirefox, isWebKit } from '@spectrum-web-components/shared';
2733
import { sendKeys } from '@web/test-runner-commands';
2834
import { spy } from 'sinon';
@@ -834,4 +840,142 @@ describe('Menu', () => {
834840
// Test that the component can be disconnected without errors
835841
el.remove();
836842
});
843+
844+
it('does not steal focus from input elements on mouseover', async () => {
845+
const el = await fixture<Menu>(html`
846+
<div>
847+
<sp-search
848+
id="test-search"
849+
placeholder="Search input..."
850+
></sp-search>
851+
<sp-textfield
852+
id="test-textfield"
853+
placeholder="Textfield input..."
854+
></sp-textfield>
855+
<sp-number-field
856+
id="test-number"
857+
placeholder="Number input..."
858+
></sp-number-field>
859+
<sp-combobox
860+
id="test-combobox"
861+
placeholder="Combobox input..."
862+
></sp-combobox>
863+
<sp-color-field
864+
id="test-color"
865+
placeholder="Color input..."
866+
></sp-color-field>
867+
<input id="test-native" placeholder="Native input..." />
868+
869+
<sp-popover open>
870+
<sp-menu>
871+
<sp-menu-item id="menu-item-1">
872+
Menu Item 1
873+
</sp-menu-item>
874+
<sp-menu-item id="menu-item-2">
875+
Menu Item 2
876+
</sp-menu-item>
877+
<sp-menu-item id="menu-item-3">
878+
Menu Item 3
879+
</sp-menu-item>
880+
</sp-menu>
881+
</sp-popover>
882+
</div>
883+
`);
884+
885+
await elementUpdated(el);
886+
887+
const searchInput = el.querySelector('#test-search') as HTMLElement;
888+
const textfieldInput = el.querySelector(
889+
'#test-textfield'
890+
) as HTMLElement;
891+
const numberInput = el.querySelector('#test-number') as HTMLElement;
892+
const comboboxInput = el.querySelector('#test-combobox') as HTMLElement;
893+
const colorInput = el.querySelector('#test-color') as HTMLElement;
894+
const nativeInput = el.querySelector(
895+
'#test-native'
896+
) as HTMLInputElement;
897+
898+
const menuItem1 = el.querySelector('#menu-item-1') as MenuItem;
899+
const menuItem2 = el.querySelector('#menu-item-2') as MenuItem;
900+
const menuItem3 = el.querySelector('#menu-item-3') as MenuItem;
901+
902+
// Test with sp-search
903+
searchInput.focus();
904+
await elementUpdated(el);
905+
expect(document.activeElement).to.equal(searchInput);
906+
907+
await mouseMoveOver(menuItem1);
908+
await elementUpdated(el);
909+
expect(document.activeElement).to.equal(
910+
searchInput,
911+
'sp-search should retain focus on mouseover'
912+
);
913+
914+
await mouseMoveOver(menuItem2);
915+
await elementUpdated(el);
916+
expect(document.activeElement).to.equal(
917+
searchInput,
918+
'sp-search should retain focus on mouseover'
919+
);
920+
921+
// Test with sp-textfield
922+
textfieldInput.focus();
923+
await elementUpdated(el);
924+
expect(document.activeElement).to.equal(textfieldInput);
925+
926+
await mouseMoveOver(menuItem1);
927+
await elementUpdated(el);
928+
expect(document.activeElement).to.equal(
929+
textfieldInput,
930+
'sp-textfield should retain focus on mouseover'
931+
);
932+
933+
// Test with sp-number-field
934+
numberInput.focus();
935+
await elementUpdated(el);
936+
expect(document.activeElement).to.equal(numberInput);
937+
938+
await mouseMoveOver(menuItem2);
939+
await elementUpdated(el);
940+
expect(document.activeElement).to.equal(
941+
numberInput,
942+
'sp-number-field should retain focus on mouseover'
943+
);
944+
945+
// Test with sp-combobox
946+
comboboxInput.focus();
947+
await elementUpdated(el);
948+
expect(document.activeElement).to.equal(comboboxInput);
949+
950+
await mouseMoveOver(menuItem3);
951+
await elementUpdated(el);
952+
expect(document.activeElement).to.equal(
953+
comboboxInput,
954+
'sp-combobox should retain focus on mouseover'
955+
);
956+
957+
// Test with sp-color-field
958+
colorInput.focus();
959+
await elementUpdated(el);
960+
expect(document.activeElement).to.equal(colorInput);
961+
962+
await mouseMoveOver(menuItem1);
963+
await elementUpdated(el);
964+
expect(document.activeElement).to.equal(
965+
colorInput,
966+
'sp-color-field should retain focus on mouseover'
967+
);
968+
969+
// Test with native input
970+
nativeInput.focus();
971+
await elementUpdated(el);
972+
expect(document.activeElement).to.equal(nativeInput);
973+
974+
await mouseMoveOver(menuItem2);
975+
await elementUpdated(el);
976+
expect(document.activeElement).to.equal(
977+
nativeInput,
978+
'native input should retain focus on mouseover'
979+
);
980+
});
837981
});

0 commit comments

Comments
 (0)