Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
143 changes: 143 additions & 0 deletions packages/main/cypress/specs/Bar.cy.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Bar from "../../src/Bar.js";
import Button from "../../src/Button.js";
import Input from "../../src/Input.js";

describe("Bar Accessibility", () => {
it("Should use accessibleName property as aria-label", () => {
Expand Down Expand Up @@ -64,4 +65,146 @@ describe("Bar Accessibility", () => {
.find(".ui5-bar-root")
.should("have.attr", "aria-label", "External Navigation Label");
});
});

describe("Bar Keyboard Navigation", () => {
it("ArrowRight moves focus forward through all three slots", () => {
cy.mount(
<Bar>
<Button id="btn-start" slot="startContent">Start</Button>
<Button id="btn-mid">Middle</Button>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#btn-start").realClick().should("be.focused");
cy.realPress("ArrowRight");
cy.get("#btn-mid").should("be.focused");
cy.realPress("ArrowRight");
cy.get("#btn-end").should("be.focused");
});

it("ArrowLeft moves focus backward", () => {
cy.mount(
<Bar>
<Button id="btn-start" slot="startContent">Start</Button>
<Button id="btn-mid">Middle</Button>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#btn-start").realClick().should("be.focused");
cy.realPress("ArrowRight");
cy.get("#btn-mid").should("be.focused");
cy.realPress("ArrowRight");
cy.get("#btn-end").should("be.focused");
cy.realPress("ArrowLeft");
cy.get("#btn-mid").should("be.focused");
cy.realPress("ArrowLeft");
cy.get("#btn-start").should("be.focused");
});

it("ArrowRight at last item does not move focus", () => {
cy.mount(
<Bar>
<Button id="btn-start" slot="startContent">Start</Button>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#btn-start").realClick().should("be.focused");
cy.realPress("ArrowRight");
cy.get("#btn-end").should("be.focused");
cy.realPress("ArrowRight");
cy.get("#btn-end").should("be.focused");
});

it("ArrowLeft at first item does not move focus", () => {
cy.mount(
<Bar>
<Button id="btn-start" slot="startContent">Start</Button>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#btn-start").realClick().should("be.focused");
cy.realPress("ArrowLeft");
cy.get("#btn-start").should("be.focused");
});

it("End key jumps to last focusable item", () => {
cy.mount(
<Bar>
<Button id="btn-start" slot="startContent">Start</Button>
<Button id="btn-mid">Middle</Button>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#btn-start").realClick().should("be.focused");
cy.realPress("End");
cy.get("#btn-end").should("be.focused");
});

it("Home key jumps to first focusable item", () => {
cy.mount(
<Bar>
<Button id="btn-start" slot="startContent">Start</Button>
<Button id="btn-mid">Middle</Button>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#btn-start").realClick().should("be.focused");
cy.realPress("End");
cy.get("#btn-end").should("be.focused");
cy.realPress("Home");
cy.get("#btn-start").should("be.focused");
});

it("ArrowRight inside input with mid-text caret does not move focus", () => {
cy.mount(
<Bar>
<Input id="input-start" slot="startContent" value="hello"></Input>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#input-start").realClick();
// place caret at position 2 (middle of "hello")
cy.get("#input-start").shadow().find("input").then($input => {
$input[0].setSelectionRange(2, 2);
});
cy.realPress("ArrowRight");
cy.get("#btn-end").should("not.be.focused");
});

it("ArrowRight at end of input text moves focus to next item", () => {
cy.mount(
<Bar>
<Input id="input-start" slot="startContent" value="hello"></Input>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#input-start").realClick();
cy.get("#input-start").shadow().find("input").then($input => {
$input[0].setSelectionRange(5, 5); // end of "hello"
});
cy.realPress("ArrowRight");
cy.get("#btn-end").should("be.focused");
});

it("Navigation is disabled when accessibleRole is None", () => {
cy.mount(
<Bar accessibleRole="None">
<Button id="btn-start" slot="startContent">Start</Button>
<Button id="btn-end" slot="endContent">End</Button>
</Bar>
);

cy.get("#btn-start").realClick().should("be.focused");
cy.realPress("ArrowRight");
cy.get("#btn-end").should("not.be.focused");
});
});
165 changes: 162 additions & 3 deletions packages/main/src/Bar.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
import UI5Element, { instanceOfUI5Element } from "@ui5/webcomponents-base/dist/UI5Element.js";
import type { DefaultSlot, Slot } from "@ui5/webcomponents-base/dist/UI5Element.js";
import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
import property from "@ui5/webcomponents-base/dist/decorators/property.js";
import slot from "@ui5/webcomponents-base/dist/decorators/slot-strict.js";
import jsxRenderer from "@ui5/webcomponents-base/dist/renderer/JsxRenderer.js";
import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
import { getEffectiveAriaLabelText } from "@ui5/webcomponents-base/dist/util/AccessibilityTextsHelper.js";
import isElementHidden from "@ui5/webcomponents-base/dist/util/isElementHidden.js";
import getActiveElement from "@ui5/webcomponents-base/dist/util/getActiveElement.js";
import {
isLeft,
isRight,
isHome,
isEnd,
} from "@ui5/webcomponents-base/dist/Keys.js";
import type BarDesign from "./types/BarDesign.js";
import type BarAccessibleRole from "./types/BarAccessibleRole.js";

Expand Down Expand Up @@ -36,6 +44,13 @@ import type { AriaRole } from "@ui5/webcomponents-base/dist/types.js";
*
* ### Keyboard Handling
*
* The `ui5-bar` provides advanced keyboard handling among interactive components inside it, no matter in which slot they are placed.
*
* #### Regular Navigation
* - [Left] / [Right] - navigate backward/forward among interactive components
* - [Home] / [End] - move to first/last interactive components
* - [Tab] / [Shift]+[Tab] - navigate forward/backward among interactive components
*
* #### Fast Navigation
* This component provides a build in fast navigation group which can be used via [F6] / [Shift] + [F6] / [Ctrl] + [Alt/Option] / [Down] or [Ctrl] + [Alt/Option] + [Up].
* In order to use this functionality, you need to import the following module:
Expand Down Expand Up @@ -76,9 +91,9 @@ class Bar extends UI5Element {
*
* - By default, accessibleRole is set to "Toolbar", which renders the ARIA role "toolbar".
*
* - Use the default accessibleRole value "Toolbar" only when the component contains two or more active, interactive elements (such as buttons, links, or input fields) within the bar.
* - Use the default accessibleRole value "Toolbar" only when the component contains three or more active, interactive elements (such as buttons, links, or input fields) within the bar.
*
* - If there is only one or no active element, set accessibleRole to "None" to avoid rendering the ARIA role "toolbar", as that role implies a grouping of multiple interactive controls.
* - If there is only one, two or no active element, set accessibleRole to "None" to avoid rendering the ARIA role "toolbar", as that role implies a grouping of multiple interactive controls.
*
* @public
* @default "Toolbar"
Expand Down Expand Up @@ -128,6 +143,7 @@ class Bar extends UI5Element {
endContent!: Slot<HTMLElement>;

_handleResizeBound: () => void;
_onKeyDownBound: (e: KeyboardEvent) => void;

get accInfo() {
return {
Expand All @@ -148,6 +164,7 @@ class Bar extends UI5Element {
super();

this._handleResizeBound = this.handleResize.bind(this);
this._onKeyDownBound = this._onKeyDown.bind(this);
}

handleResize() {
Expand All @@ -166,6 +183,8 @@ class Bar extends UI5Element {
this.getDomRef()!.querySelectorAll(".ui5-bar-content-container").forEach(child => {
ResizeHandler.register(child as HTMLElement, this._handleResizeBound);
}, this);

this.addEventListener("keydown", this._onKeyDownBound, true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This listener steals keys if we have slotted self-navigating child elems. For example slotted slider, segmented button, or breadcrumb loses their own Left/Right/Home/End at the bar boundary.
I think in the Toolbar they use this util, to go around it - getArrowNavState(), please double-check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, good point. It steals kbd navigation and wouldn't work if there is such component there. The problem is that getArrowNavState should be implemented in all components that have its own arrow handling.

}

onExitDOM() {
Expand All @@ -174,11 +193,151 @@ class Bar extends UI5Element {
this.getDomRef()!.querySelectorAll(".ui5-bar-content-container").forEach(child => {
ResizeHandler.deregister(child as HTMLElement, this._handleResizeBound);
}, this);

this.removeEventListener("keydown", this._onKeyDownBound, true);
}

get effectiveRole() {
return this.accessibleRole.toLowerCase() === "toolbar" ? "toolbar" as AriaRole : undefined;
}

_collectFocusableElements(): Array<HTMLElement> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use getTabbableElements here ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope, because getTabbableElements return ALL tabbable elements (for example each segmented button item, and using it to focus an element later would break default behaviour of the components that use item navigation or similar mechanism.

const slotSelectors = [
"slot[name=\"startContent\"]",
"slot:not([name])",
"slot[name=\"endContent\"]",
];
const result: Array<HTMLElement> = [];

slotSelectors.forEach(sel => {
const slotEl = this.shadowRoot!.querySelector<HTMLSlotElement>(sel);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

something minor, but let's use more descriptive names here:

sel → slottedElement / slotElement
el → elementInBar or something that makes sense

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok

if (!slotEl) {
return;
}
(slotEl.assignedElements({ flatten: true }) as HTMLElement[]).forEach(el => {
result.push(...this._getFocusableFromElement(el));
});
});
return result;
}

_getFocusableFromElement(el: HTMLElement): Array<HTMLElement> {
if (isElementHidden(el)) {
return [];
}

if (instanceOfUI5Element(el)) {
const focusRef = el.getFocusDomRef();
if (focusRef && focusRef.tabIndex >= 0 && !isElementHidden(focusRef) && !(focusRef as HTMLInputElement).disabled) {
return [focusRef];
}
return [];
}

if (el.tabIndex >= 0 && !(el as HTMLInputElement).disabled) {
return [el];
}

// Non-focusable container: recurse into children
const nested: Array<HTMLElement> = [];
Array.from(el.children).forEach(child => {
nested.push(...this._getFocusableFromElement(child as HTMLElement));
});
return nested;
}

_hasCaretNavigation(el: EventTarget | null): el is HTMLInputElement | HTMLTextAreaElement {
if (!(el instanceof HTMLElement)) {
return false;
}
const tag = el.tagName.toLowerCase();
if (tag === "textarea") {
return true;
}
if (tag !== "input") {
return false;
}
const type = (el as HTMLInputElement).type.toLowerCase();
return ["text", "search", "url", "tel", "password", ""].includes(type);
}

_onKeyDown(e: KeyboardEvent) {
if (this.effectiveRole !== "toolbar") {
return;
}

const isForward = this.effectiveDir === "rtl" ? isLeft(e) : isRight(e);
const isBackward = this.effectiveDir === "rtl" ? isRight(e) : isLeft(e);
const isHomeKey = isHome(e);
const isEndKey = isEnd(e);

if (!isForward && !isBackward && !isHomeKey && !isEndKey) {
return;
}

const items = this._collectFocusableElements();
if (items.length === 0) {
return;
}

const active = getActiveElement() as HTMLElement | null;
if (!active) {
return;
}

const currentIndex = items.findIndex(item => this._isNodeInsideElement(active, item));
if (currentIndex === -1) {
return;
}

if (this._hasCaretNavigation(active)) {
const input = active as HTMLInputElement;
if (isHomeKey || isEndKey) {
return;
}
if (isForward && input.selectionStart !== input.value.length) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For inputs where selectionStart returns null, this check will result in !== 0 being true, and the navigation will be trapped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

return;
}
if (isBackward && input.selectionStart !== 0) {
return;
}
}

let nextIndex: number;
if (isHomeKey) {
nextIndex = 0;
} else if (isEndKey) {
nextIndex = items.length - 1;
} else if (isForward) {
nextIndex = Math.min(currentIndex + 1, items.length - 1);
} else {
nextIndex = Math.max(currentIndex - 1, 0);
}

if (nextIndex === currentIndex) {
return;
}

items[nextIndex].focus();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we move focus, but the tabindex is not chaning, is this expected ?

Image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it supposed to be changed? In my opinion - NO!

e.preventDefault();
e.stopPropagation();
}

_isNodeInsideElement(node: Node, element: HTMLElement): boolean {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is absolutely identical with the same method in the Toolbar.ts, we could extract it in a until, e.g. in the base/packages

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, we can think on it.

let current: Node | null = node;
while (current) {
if (current === element) {
return true;
}
const root = current.getRootNode?.();
if (root instanceof ShadowRoot) {
current = root.host;
} else {
current = current.parentNode;
}
}
return false;
}
}

Bar.define();
Expand Down
14 changes: 14 additions & 0 deletions packages/main/test/pages/Bar.html
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@
<ui5-button design="Negative" slot="endContent">Decline</ui5-button>
<ui5-button design="Transparent" slot="endContent">Cancel</ui5-button>
</ui5-bar>
<br>
<ui5-title level="3">Mixed content (texts, buttons, links, inputs)</ui5-title>
<ui5-bar design="Header">
<ui5-button icon="nav-back" slot="startContent">Back</ui5-button>
<ui5-link slot="startContent" href="#">Home</ui5-link>

<ui5-label>Search:</ui5-label>
<ui5-input placeholder="Type here..."></ui5-input>
<ui5-button icon="search">Search</ui5-button>

<ui5-link slot="endContent" href="#">Help</ui5-link>
<ui5-button icon="action-settings" design="Transparent" slot="endContent">Settings</ui5-button>
<ui5-button design="Positive" slot="endContent">Save</ui5-button>
</ui5-bar>
</section>

<section>
Expand Down
Loading