Skip to content

fix(ui5-side-navigation): focus is correct after selecting overflow item #11702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 19, 2025
Merged

fix(ui5-side-navigation): focus is correct after selecting overflow item #11702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3d593fd
chore(ui5-sidenavigation): fix focus after the menu is closed
TeodorTaushanov May 19, 2025
28c6cd5
Merge remote-tracking branch 'origin/main' into sidenav_focus
TeodorTaushanov Jun 11, 2025
9076208
fix: use menu close event to focus item
TeodorTaushanov Jun 11, 2025
5678718
chore: fix lint error
TeodorTaushanov Jun 11, 2025
a74442a
chore: add tests
TeodorTaushanov Jun 11, 2025
67963bc
Merge remote-tracking branch 'origin/main' into sidenav_focus
TeodorTaushanov Jun 12, 2025
7f0d285
Merge remote-tracking branch 'origin/main' into sidenav_focus
TeodorTaushanov Jun 16, 2025
99ecbb8
Merge remote-tracking branch 'origin/main' into sidenav_focus
TeodorTaushanov Jun 18, 2025
300f853
chore: remove unused code
TeodorTaushanov Jun 18, 2025
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
98 changes: 78 additions & 20 deletions packages/fiori/cypress/specs/SideNavigation.cy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import SideNavigationItem from "../../src/SideNavigationItem.js";
import SideNavigationGroup from "../../src/SideNavigationGroup.js";
import SideNavigationSubItem from "../../src/SideNavigationSubItem.js";
import group from "@ui5/webcomponents-icons/dist/group.js";
import home from "@ui5/webcomponents-icons/dist/home.js";
import employeeApprovals from "@ui5/webcomponents-icons/dist/employee-approvals.js";
import { NAVIGATION_MENU_POPOVER_HIDDEN_TEXT } from "../../src/generated/i18n/i18n-defaults.js";
import Title from "@ui5/webcomponents/dist/Title.js";
import Label from "@ui5/webcomponents/dist/Label.js";
Expand Down Expand Up @@ -103,11 +105,11 @@ describe("Side Navigation Rendering", () => {
const TOOLTIP_TEXT = "From My Team tooltip";
cy.mount(
<SideNavigation id="sn1" accessibleName="Main" collapsed={false}>
<SideNavigationItem id="item1" text="People" icon="group" expanded={true}>
<SideNavigationSubItem id="item11" text="From My Team" icon="employee-approvals" tooltip={TOOLTIP_TEXT}></SideNavigationSubItem>
<SideNavigationSubItem id="item21" text="From My Team" icon="employee-approvals"></SideNavigationSubItem>
<SideNavigationItem id="item1" text="People" icon="{group}" expanded={true}>
<SideNavigationSubItem id="item11" text="From My Team" icon="{employeeApprovals}" tooltip={TOOLTIP_TEXT}></SideNavigationSubItem>
<SideNavigationSubItem id="item21" text="From My Team" icon="{employeeApprovals}"></SideNavigationSubItem>
</SideNavigationItem>
<SideNavigationItem id="item2" text="People" expanded icon="group" tooltip={TOOLTIP_TEXT} />
<SideNavigationItem id="item2" text="People" expanded icon="{group}" tooltip={TOOLTIP_TEXT} />
</SideNavigation>);

cy.get("#item1").should("not.have.attr", "tooltip");
Expand Down Expand Up @@ -292,7 +294,7 @@ describe("Side Navigation interaction", () => {
cy.get("#sn")
.shadow()
.find(`[ui5-side-navigation-item][text="1"]`)
.should('be.focused');
.should('be.focused');

cy.realPress("ArrowLeft");
cy.get("#unselectableItem").should('be.focused');
Expand Down Expand Up @@ -391,9 +393,9 @@ describe("Side Navigation interaction", () => {
cy.get("#sn")
.shadow()
.find(`[ui5-side-navigation-item][text="1"]`)
.should('be.focused');
.should('be.focused');

cy.realPress("ArrowRight");
cy.realPress("ArrowRight");

cy.get("#unselectableItem").should('be.focused');

Expand Down Expand Up @@ -761,22 +763,22 @@ describe("Side Navigation interaction", () => {
const handleClick = (event: Event) => {
event.preventDefault();
};

const handleSelectionChange = cy.stub().as("selectionChangeHandler");

cy.mount(
<SideNavigation id="sideNav" onClick={handleClick} onSelectionChange={handleSelectionChange}>
<SideNavigationItem id="linkItem" text="external link" unselectable={true} href="#preventDefault" />
<SideNavigationItem id="item" text="item"/>
</SideNavigation>
);

cy.url()
.should("not.include", "#preventDefault");

// Act
cy.get("#linkItem").realClick();

// Assert
cy.get("@selectionChangeHandler").should("not.have.been.called");
cy.url()
Expand All @@ -794,7 +796,7 @@ describe("Side Navigation interaction", () => {
const handleClick = (event: Event) => {
event.preventDefault();
};

const handleSelectionChange = cy.stub().as("selectionChangeHandler");

cy.mount(
Expand All @@ -819,20 +821,20 @@ describe("Side Navigation interaction", () => {

cy.url()
.should("not.include", "#test");

cy.get("#sideNav")
.shadow()
.find(".ui5-side-navigation-overflow-menu [ui5-navigation-menu-item][text='item']")
.realClick();

cy.get("@selectionChangeHandler").should("not.have.been.called");
});

it("Tests preventDefault on child items in collapsed side navigation", () => {
const handleClick = (event: Event) => {
event.preventDefault();
};

const handleSelectionChange = cy.stub().as("selectionChangeHandler");

cy.mount(
Expand Down Expand Up @@ -880,7 +882,7 @@ describe("Side Navigation interaction", () => {
{ key: "altKey", options: { altKey: true } },
{ key: "shiftKey", options: { shiftKey: true } },
];

keyModifiers.forEach(({ key, options }) => {
cy.get("#sideNav").realClick(options);
cy.get("@clickHandler").should("be.calledWithMatch", { detail: { [key]: true } });
Expand Down Expand Up @@ -978,6 +980,62 @@ describe("Side Navigation interaction", () => {

cy.get("@selectionChangeHandler").should("have.been.calledOnce");
});

it("tests selecting items in overflow menu", () => {
cy.mount(
<SideNavigation style="height: 200px" id="sideNav" collapsed={true}>
<SideNavigationItem icon={home} text="Home"></SideNavigationItem>
<SideNavigationItem icon={home} text="Home 1"></SideNavigationItem>
<SideNavigationItem icon={home} text="Home 2"></SideNavigationItem>
<SideNavigationItem icon={home} text="Home 3"></SideNavigationItem>
<SideNavigationItem icon={home} text="Home 4"></SideNavigationItem>
<SideNavigationItem icon={home} text="Home 5"></SideNavigationItem>
<SideNavigationItem id="home6" icon={home} text="Home 6"></SideNavigationItem>
<SideNavigationItem id="home7" icon={home} text="Home 7" unselectable></SideNavigationItem>
</SideNavigation>
);

cy.get("#sideNav")
.shadow()
.find(".ui5-sn-item-overflow")
.as("itemOverflow");

cy.get("@itemOverflow")
.should("be.visible");

cy.get("@itemOverflow")
.realClick()
.realClick();

cy.get("#sideNav")
.shadow()
.find(".ui5-side-navigation-overflow-menu")
.as("overflowMenu");

cy.get("@overflowMenu")
.should("be.visible");

cy.get("@overflowMenu")
.find("[ui5-navigation-menu-item][text='Home 6']")
.realClick();

cy.get("@overflowMenu")
.should("be.not.visible");

cy.get("[ui5-side-navigation-item][text='Home 6']")
.should("be.focused");

cy.get("@itemOverflow")
.realClick()
.realClick();

cy.get("@overflowMenu")
.find("[ui5-navigation-menu-item][text='Home 7']")
.realClick();

cy.get("@itemOverflow")
.should("be.focused");
});
});

describe("Side Navigation Accessibility", () => {
Expand Down Expand Up @@ -1360,7 +1418,7 @@ describe("Focusable items", () => {
);

cy.get("#item").realClick();

cy.get("#item")
.should("be.focused")
cy.get("#item")
Expand All @@ -1369,7 +1427,7 @@ describe("Focusable items", () => {
.should("have.attr", "tabindex", "0");

cy.realPress("ArrowDown");

cy.get("#parentItem")
.should("be.focused")
cy.get("#parentItem")
Expand All @@ -1378,7 +1436,7 @@ describe("Focusable items", () => {
.should("have.attr", "tabindex", "0");

cy.realPress("ArrowDown");

cy.get("#childItem")
.should("be.focused")
cy.get("#childItem")
Expand Down Expand Up @@ -1415,7 +1473,7 @@ describe("Focusable items", () => {
it("Tests external link items", () => {
cy.mount(
<SideNavigation>
<SideNavigationItem id="externalLinkItem" text="External Link Unselectable" icon="chain-link" href="https://sap.com" unselectable target="_blank" />
<SideNavigationItem id="externalLinkItem" text="External Link Unselectable" icon="{home}" href="https://sap.com" unselectable target="_blank" />
</SideNavigation>);

cy.get("#externalLinkItem")
Expand Down
17 changes: 1 addition & 16 deletions packages/fiori/src/NavigationMenuItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import navigationMenuItemCss from "./generated/themes/NavigationMenuItem.css.js"
import {
NAVIGATION_MENU_POPOVER_HIDDEN_TEXT,
} from "./generated/i18n/i18n-defaults.js";
import type SideNavigationItem from "./SideNavigationItem.js";

/**
* @class
Expand Down Expand Up @@ -159,21 +158,7 @@ class NavigationMenuItem extends MenuItem {
}

if (!this.hasSubmenu) {
sideNav?.closeMenu();
this._handleFocus(item);
}
}

_handleFocus(associatedItem: SideNavigationSelectableItemBase) {
const sideNavigation = associatedItem.sideNavigation;

if (associatedItem.nodeName.toLowerCase() === "ui5-side-navigation-sub-item") {
const parent = associatedItem.parentElement as SideNavigationItem;
sideNavigation?.focusItem(parent);
parent?.focus();
} else {
sideNavigation?.focusItem(associatedItem);
associatedItem?.focus();
sideNav?.closeMenu(shouldSelect);
}
}

Expand Down
1 change: 0 additions & 1 deletion packages/fiori/src/NavigationMenuTemplate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export default function NavigationMenuTemplate(this: NavigationMenu) {
opener={this.opener}
open={this.open}
preventInitialFocus={true}
preventFocusRestore={true}
accessibleNameRef={`${this._id}-navigationMenuPopoverText`}
onBeforeOpen={this._beforePopoverOpen}
onOpen={this._afterPopoverOpen}
Expand Down
28 changes: 24 additions & 4 deletions packages/fiori/src/SideNavigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ class SideNavigation extends UI5Element {

_onBeforeMenuOpen() {
const popover = this.getOverflowPopover();
popover._popover.preventFocusRestore = false;
(popover?.opener as HTMLElement)?.classList.add("ui5-sn-item-active");
}

Expand All @@ -289,6 +290,22 @@ class SideNavigation extends UI5Element {
(popover?.opener as HTMLElement)?.classList.remove("ui5-sn-item-active");
}

_bn?: SideNavigationSelectableItemBase;

_onMenuClose() {
const menu = this.getOverflowPopover();
if (!menu._popover.preventFocusRestore) {
return;
}

const selectedItem = this._findSelectedItem(this.items as Array<SideNavigationItem | SideNavigationGroup>);

if (selectedItem) {
this.focusItem(selectedItem);
selectedItem.focus();
}
}

get accSideNavigationPopoverHiddenText() {
return SideNavigation.i18nBundle.getText(SIDE_NAVIGATION_POPOVER_HIDDEN_TEXT);
}
Expand Down Expand Up @@ -352,9 +369,11 @@ class SideNavigation extends UI5Element {
}

this._selectItem(associatedItem);
this.closePicker();

this._popoverContents.item?.getDomRef()!.classList.add("ui5-sn-item-no-hover-effect");
setTimeout(() => {
this.closePicker();
this._popoverContents.item?.getDomRef()!.classList.add("ui5-sn-item-no-hover-effect");
});
}

getOverflowPopover() {
Expand Down Expand Up @@ -386,8 +405,9 @@ class SideNavigation extends UI5Element {
responsivePopover.open = false;
}

closeMenu() {
closeMenu(preventFocusRestore: boolean = false) {
const menu = this.getOverflowPopover();
menu._popover.preventFocusRestore = preventFocusRestore;
menu.open = false;
}

Expand Down Expand Up @@ -648,7 +668,7 @@ class SideNavigation extends UI5Element {
this._isOverflow = true;
this._menuPopoverItems = this._getOverflowItems();

this.openOverflowMenu(this._overflowItem!.getFocusDomRef() as HTMLElement);
this.openOverflowMenu(this._overflowItem!);
}

_getOverflowItems(): Array<SideNavigationItem> {
Expand Down
1 change: 1 addition & 0 deletions packages/fiori/src/SideNavigationPopoverTemplate.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default function SideNavigationTemplate(this: SideNavigation) {
id={`${this._id}-side-navigation-overflow-menu`}
onBeforeOpen={this._onBeforeMenuOpen}
onBeforeClose={this._onBeforeMenuClose}
onClose={this._onMenuClose}
class="ui5-side-navigation-popover ui5-side-navigation-overflow-menu"
>
{this._menuPopoverItems.map(item =>
Expand Down
Loading