Skip to content

Commit e2d98ef

Browse files
fix(ui5-toolbar): harden keyboard delegation for nested controls
Improve WCAG toolbar keyboard behavior while preventing regressions for nested and overflowed controls. - add key-level ownership hook shouldHandleOwnKeyboardNavigation(event) - use key-aware delegation in toolbar and overflow key handlers - preserve native input/textarea/contenteditable navigation (Arrow/Home/End) - narrow overflow preventDefault to toolbar-consumed navigation only - refactor navigation item collection for standard and overflow paths - update keyboard handling documentation in Toolbar - add cypress regressions for input Home/End in toolbar and overflow - fix toolbar cypress type check for itemsToOverflow membership assertion
1 parent 0e45f1e commit e2d98ef

File tree

4 files changed

+171
-21
lines changed

4 files changed

+171
-21
lines changed

packages/main/cypress/specs/Toolbar.cy.tsx

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,89 @@ describe("Toolbar general interaction", () => {
460460
});
461461
});
462462

463+
it("Should preserve Home/End behavior for text inputs inside toolbar-item", () => {
464+
cy.mount(
465+
<Toolbar>
466+
<ToolbarButton text="First"></ToolbarButton>
467+
<ToolbarItem>
468+
<input data-testid="editor" defaultValue="abcdef" />
469+
</ToolbarItem>
470+
<ToolbarButton text="Last"></ToolbarButton>
471+
</Toolbar>
472+
);
473+
474+
cy.get("[data-testid='editor']")
475+
.realClick()
476+
.then($input => {
477+
const input = $input[0] as HTMLInputElement;
478+
input.setSelectionRange(3, 3);
479+
});
480+
481+
cy.realPress("Home");
482+
cy.get("[data-testid='editor']")
483+
.should("be.focused")
484+
.then($input => {
485+
expect(($input[0] as HTMLInputElement).selectionStart).to.equal(0);
486+
});
487+
488+
cy.get("[data-testid='editor']")
489+
.then($input => {
490+
const input = $input[0] as HTMLInputElement;
491+
input.setSelectionRange(2, 2);
492+
});
493+
494+
cy.realPress("End");
495+
cy.get("[data-testid='editor']")
496+
.should("be.focused")
497+
.then($input => {
498+
expect(($input[0] as HTMLInputElement).selectionStart).to.equal(6);
499+
});
500+
});
501+
502+
it("Should not suppress input arrow/home/end behavior inside overflow popover", () => {
503+
cy.viewport(220, 600);
504+
505+
cy.mount(
506+
<Toolbar>
507+
<ToolbarButton text="First"></ToolbarButton>
508+
<ToolbarItem overflow-priority="AlwaysOverflow">
509+
<input data-testid="overflow-editor" defaultValue="abcdef" />
510+
</ToolbarItem>
511+
<ToolbarButton text="Last"></ToolbarButton>
512+
</Toolbar>
513+
);
514+
515+
cy.get("[ui5-toolbar]")
516+
.shadow()
517+
.find(".ui5-tb-overflow-btn")
518+
.realClick();
519+
520+
cy.get("[ui5-toolbar]")
521+
.shadow()
522+
.find(".ui5-overflow-popover")
523+
.should("have.prop", "open", true);
524+
525+
cy.get("[data-testid='overflow-editor']")
526+
.realClick()
527+
.then($input => {
528+
const input = $input[0] as HTMLInputElement;
529+
input.setSelectionRange(2, 2);
530+
});
531+
532+
cy.realPress("ArrowLeft");
533+
cy.get("[data-testid='overflow-editor']")
534+
.should("be.focused")
535+
.then($input => {
536+
expect(($input[0] as HTMLInputElement).selectionStart).to.equal(1);
537+
});
538+
539+
cy.realPress("Home");
540+
cy.get("[data-testid='overflow-editor']")
541+
.then($input => {
542+
expect(($input[0] as HTMLInputElement).selectionStart).to.equal(0);
543+
});
544+
});
545+
463546
it("Should move button with alwaysOverflow priority to overflow popover", () => {
464547

465548
cy.mount(
@@ -987,7 +1070,7 @@ describe("ToolbarButton", () => {
9871070
const toolbar = $toolbar[0] as Toolbar;
9881071
const addButton = document.getElementById("add-btn") as ToolbarButton;
9891072

990-
expect(toolbar.itemsToOverflow.includes(addButton)).to.be.true;
1073+
expect(toolbar.itemsToOverflow.some(item => item._id === addButton._id)).to.be.true;
9911074

9921075
const initialOverflowCount = toolbar.itemsToOverflow.length;
9931076
const initialItemsWidth = toolbar.itemsWidth;

packages/main/src/Toolbar.ts

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ function parsePxValue(styleSet: CSSStyleDeclaration, propertyName: string): numb
7272
* The `ui5-toolbar` provides advanced keyboard handling.
7373
*
7474
* - The control is not interactive, but can contain of interactive elements
75-
* - [Tab] - iterates through elements
75+
* - [Left]/[Right]/[Up]/[Down] - navigate among toolbar items
76+
* - [Home]/[End] - move to first/last toolbar item
77+
* - [Tab] / [Shift] + [Tab] - move through toolbar items and exit at edges
7678
*
7779
* ### ES6 Module Import
7880
* `import "@ui5/webcomponents/dist/Toolbar.js";`
@@ -594,9 +596,24 @@ class Toolbar extends UI5Element {
594596
}
595597

596598
_getNavigationItems(): Array<ToolbarNavigationItem> {
599+
const itemFocusRefs = this._getNavigationItemsFromItems(this.standardItems);
600+
601+
if (!this.hideOverflowButton && this.overflowButtonDOM) {
602+
const overflowButtonFocusRef = this.overflowButtonDOM.getFocusDomRef() as HTMLElement;
603+
if (overflowButtonFocusRef && this._isVisible(overflowButtonFocusRef)) {
604+
itemFocusRefs.push({
605+
focusRef: overflowButtonFocusRef,
606+
});
607+
}
608+
}
609+
610+
return itemFocusRefs;
611+
}
612+
613+
_getNavigationItemsFromItems(items: Array<ToolbarItemBase>): Array<ToolbarNavigationItem> {
597614
const itemFocusRefs: Array<ToolbarNavigationItem> = [];
598615

599-
this.standardItems
616+
items
600617
.filter(item => {
601618
if (!item.isInteractive || item.hidden || item.isOverflowed) {
602619
return false;
@@ -615,15 +632,6 @@ class Toolbar extends UI5Element {
615632
});
616633
});
617634

618-
if (!this.hideOverflowButton && this.overflowButtonDOM) {
619-
const overflowButtonFocusRef = this.overflowButtonDOM.getFocusDomRef() as HTMLElement;
620-
if (overflowButtonFocusRef && this._isVisible(overflowButtonFocusRef)) {
621-
itemFocusRefs.push({
622-
focusRef: overflowButtonFocusRef,
623-
});
624-
}
625-
}
626-
627635
return itemFocusRefs;
628636
}
629637

@@ -644,12 +652,16 @@ class Toolbar extends UI5Element {
644652
&& style.visibility !== "hidden";
645653
}
646654

647-
_shouldItemHandleOwnNavigation(item?: ToolbarItemBase | null): boolean {
648-
return !!item?.handlesOwnKeyboardNavigation;
655+
_shouldItemHandleOwnNavigation(item: ToolbarItemBase | undefined, e: KeyboardEvent): boolean {
656+
return !!item?.shouldHandleOwnKeyboardNavigation(e);
649657
}
650658

651659
_shouldChildHandleNavigation(element: HTMLElement, e: KeyboardEvent): boolean {
652-
// If element is an input or textarea, check cursor position
660+
if (element.isContentEditable) {
661+
return isLeft(e) || isRight(e) || isHome(e) || isEnd(e) || isUp(e) || isDown(e);
662+
}
663+
664+
// If element is an input or textarea, preserve native text/caret behavior.
653665
if (element.tagName === "INPUT" || element.tagName === "TEXTAREA") {
654666
const input = element as HTMLInputElement | HTMLTextAreaElement;
655667
const cursorPos = input.selectionStart || 0;
@@ -664,6 +676,18 @@ class Toolbar extends UI5Element {
664676
if (isRight(e) && cursorPos < textLength) {
665677
return true;
666678
}
679+
680+
if (isHome(e) && cursorPos > 0) {
681+
return true;
682+
}
683+
684+
if (isEnd(e) && cursorPos < textLength) {
685+
return true;
686+
}
687+
688+
if (element.tagName === "TEXTAREA" && (isUp(e) || isDown(e))) {
689+
return true;
690+
}
667691
}
668692

669693
return false;
@@ -765,10 +789,33 @@ class Toolbar extends UI5Element {
765789
}
766790

767791
_onoverflowkeydown(e: KeyboardEvent) {
768-
// Prevent arrow keys from scrolling the page while focus is inside the overflow popover
769-
if (isLeft(e) || isRight(e) || isUp(e) || isDown(e) || isHome(e) || isEnd(e)) {
770-
e.preventDefault();
792+
const isNavigationKey = isLeft(e) || isRight(e) || isUp(e) || isDown(e) || isHome(e) || isEnd(e);
793+
if (!isNavigationKey) {
794+
return;
795+
}
796+
797+
const activeElement = getActiveElement();
798+
if (!activeElement) {
799+
return;
771800
}
801+
802+
const navigationItems = this._getNavigationItemsFromItems(this.overflowItems);
803+
const currentIndex = this._getCurrentItemIndex(navigationItems, activeElement as HTMLElement);
804+
if (currentIndex === -1) {
805+
return;
806+
}
807+
808+
const currentNavigationItem = navigationItems[currentIndex];
809+
if (this._shouldItemHandleOwnNavigation(currentNavigationItem?.item, e)) {
810+
return;
811+
}
812+
813+
if (this._shouldChildHandleNavigation(activeElement as HTMLElement, e)) {
814+
return;
815+
}
816+
817+
// Prevent page scrolling only when toolbar should consume the key.
818+
e.preventDefault();
772819
}
773820

774821
_onkeydown(e: KeyboardEvent) {
@@ -795,9 +842,7 @@ class Toolbar extends UI5Element {
795842
}
796843

797844
const currentNavigationItem = navigationItems[currentIndex];
798-
if (isNavigationKey
799-
&& this._shouldItemHandleOwnNavigation(currentNavigationItem?.item)
800-
&& (isUp(e) || isDown(e) || isHome(e) || isEnd(e))) {
845+
if (isNavigationKey && this._shouldItemHandleOwnNavigation(currentNavigationItem?.item, e)) {
801846
return;
802847
}
803848

packages/main/src/ToolbarItemBase.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,21 @@ class ToolbarItemBase extends UI5Element {
6060
@property({ type: Boolean })
6161
handlesOwnKeyboardNavigation = false;
6262

63+
/**
64+
* Defines whether the toolbar item handles keyboard navigation for a given key.
65+
*
66+
* Override this method in complex toolbar items that need to preserve
67+
* specific key handling.
68+
* @public
69+
* @since 2.22.0
70+
*/
71+
shouldHandleOwnKeyboardNavigation(e: KeyboardEvent): boolean {
72+
if (e.defaultPrevented) {
73+
return this.handlesOwnKeyboardNavigation;
74+
}
75+
return this.handlesOwnKeyboardNavigation;
76+
}
77+
6378
_isOverflowed: boolean = false;
6479

6580
get isOverflowed(): boolean {

packages/main/src/ToolbarSelect.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,13 @@ class ToolbarSelect extends ToolbarItemBase {
7474

7575
handlesOwnKeyboardNavigation = true;
7676

77+
shouldHandleOwnKeyboardNavigation(e: KeyboardEvent): boolean {
78+
return e.key === "ArrowUp"
79+
|| e.key === "ArrowDown"
80+
|| e.key === "Home"
81+
|| e.key === "End";
82+
}
83+
7784
/**
7885
* Defines the width of the select.
7986
*

0 commit comments

Comments
 (0)