Skip to content

feat(ui5-bar): add arrow left/arrow right/home/end navigation - #13977

Open
NHristov-sap wants to merge 2 commits into
mainfrom
BL_bar_arrows
Open

feat(ui5-bar): add arrow left/arrow right/home/end navigation#13977
NHristov-sap wants to merge 2 commits into
mainfrom
BL_bar_arrows

Conversation

@NHristov-sap

Copy link
Copy Markdown
Contributor

The WAI-ARIA toolbar pattern requires that interactive controls inside a toolbar are navigable with arrow keys. Without this, keyboard users have no standard way to move between actions in the bar other than Tab, which is cumbersome when many controls are present.

This PR introduces arrow key navigation to ui5-bar. When the bar has role="toolbar" (the default), pressing ArrowRight / ArrowLeft moves focus between all focusable elements across the bar's three slots (startContent, middle/default, endContent) in DOM order. Home and End jump directly to the first or last focusable item. Text inputs inside the bar are handled carefully — arrow keys only leave the input when the cursor is already at its edge, so normal text editing still works as expected. ui5-bar components with accessibleRole different than Toolbar are unaffected.

@NHristov-sap
NHristov-sap deployed to netlify-preview August 24, 2026 14:07 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown

👋 Heads-up: dev close is in effect

Thanks for the contribution! This repository is currently in dev close ahead of release 2.26 (scheduled 2026-08-26, UTC). See the release schedule for the full timeline.

This PR appears to introduce public-API changes (detected by diffing the Custom Elements Manifest against the latest published version on npm):

@ui5/webcomponents

  • 🔄 changed attribute accessible-role (type)

Could you please hold off on merging into main until the release ships? Public-API changes are best landed in the next dev cycle so they don't slip into the release at the last minute. Once the release is out, this PR is good to go.

If this change must ship in the current release, please request a review from one or two members of @UI5/ui5-team-webc so the team can sign off explicitly.

💬 False positive? If you believe this PR doesn't actually change the public API (e.g. only internal refactoring, or an entry the detector mis-attributed), please reply on this thread — your feedback helps us improve the detection during this trial run.

Posted automatically by the Dev Close Notice workflow.

@sap-ui5-webcomponents-release

Copy link
Copy Markdown

@NHristov-sap
NHristov-sap requested a review from hinzzx August 25, 2026 07:41
@NHristov-sap
NHristov-sap deployed to netlify-preview August 26, 2026 07:55 — with GitHub Actions Active
Comment thread packages/main/src/Bar.ts
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.

Comment thread packages/main/src/Bar.ts
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

Comment thread packages/main/src/Bar.ts
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

Comment thread packages/main/src/Bar.ts
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.

Comment thread packages/main/src/Bar.ts
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!

Comment thread packages/main/src/Bar.ts
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants