Skip to content

Commit

Permalink
fix(ui5-menu): improve focus handling (#8348)
Browse files Browse the repository at this point in the history 
fix(ui5-menu): dispatch internal focus in event

- We no longer count on the ui5-responsive popover to retrieve
the focus over ui5-menu-item opener on ui5-menu close.

- There is a differentiation between mouse and keyboard interactions
aimed to retrieve the focus to the opener element on ui5-menu close.
  • Loading branch information
@unazko
unazko authored Feb 28, 2024
1 parent 845b6f7 commit bd33dc5
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 58 deletions.
1 change: 1 addition & 0 deletions packages/main/src/Menu.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
@mouseover={{../_itemMouseOver}}
@mouseout={{../_itemMouseOut}}
@keydown={{../_itemKeyDown}}
@_focused={{../_onfocusin}}
>
<div class="ui5-menu-item-text">
{{#if this.item.hasDummyIcon}}
Expand Down
95 changes: 47 additions & 48 deletions packages/main/src/Menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,12 @@ class Menu extends UI5Element {
@property({ type: Object, defaultValue: undefined })
_parentMenuItem?: MenuItem;

/**
* Stores parent menu item DOM representation (if there is such).
*/
@property({ type: Object, defaultValue: undefined })
_opener?: HTMLElement;

/**
* Stores menu item that have sub-menu opened.
*/
Expand Down Expand Up @@ -400,15 +406,10 @@ class Menu extends UI5Element {
}
if (!this._isSubMenu) {
this._parentMenuItem = undefined;
this._opener = undefined;
}
const popover = await this._createPopover();
popover.initialFocus = "";
for (let index = 0; index < this._currentItems.length; index++) {
if (!this._currentItems[index].item.disabled) {
popover.initialFocus = `${this._id}-menu-item-${index}`;
break;
}
}
popover.initialFocus = `${this._id}-menu-item-0`;
popover.showAt(opener);
}

Expand All @@ -422,8 +423,7 @@ class Menu extends UI5Element {
if (isPhone()) {
this._parentItemsStack = [];
}
this._popover.close();
this._popover.resetFocus();
this._popover.close(false, false, true);
}
}

Expand Down Expand Up @@ -459,13 +459,14 @@ class Menu extends UI5Element {
});
}

_createSubMenu(item: MenuItem, openerId: string) {
_createSubMenu(item: MenuItem, opener: HTMLElement) {
const ctor = this.constructor as typeof Menu;
const subMenu = document.createElement(ctor.getMetadata().getTag()) as Menu;

subMenu._isSubMenu = true;
subMenu.setAttribute("id", `submenu-${openerId}`);
subMenu.setAttribute("id", `submenu-${opener.id}`);
subMenu._parentMenuItem = item;
subMenu._opener = opener;
subMenu.busy = item.busy;
subMenu.busyDelay = item.busyDelay;
const fragment = this._clonedItemsFragment(item);
Expand All @@ -485,51 +486,54 @@ class Menu extends UI5Element {
return fragment;
}

_openItemSubMenu(item: MenuItem, opener: HTMLElement, actionId: string) {
_openItemSubMenu(item: MenuItem, opener: HTMLElement) {
const mainMenu = this._findMainMenu(item);
mainMenu.fireEvent<MenuBeforeOpenEventDetail>("before-open", {
item,
}, false, false);
item._subMenu!.showAt(opener);
item._preventSubMenuClose = true;
this._openedSubMenuItem = item;
this._subMenuOpenerId = actionId;
this._subMenuOpenerId = opener.id;
}

_closeItemSubMenu(item: MenuItem, forceClose = false) {
_closeItemSubMenu(item: MenuItem, forceClose = false, keyboard = false) {
if (item) {
if (forceClose) {
item._preventSubMenuClose = false;
this._closeSubMenuPopover(item._subMenu!, true);
this._closeSubMenuPopover(item._subMenu!, forceClose, keyboard);
} else {
setTimeout(() => this._closeSubMenuPopover(item._subMenu!), 0);
}
}
}

_closeSubMenuPopover(subMenu: Menu, forceClose = false) {
_closeSubMenuPopover(subMenu: Menu, forceClose = false, keyboard = false) {
if (subMenu) {
const parentItem = subMenu._parentMenuItem!;

if (forceClose || !parentItem._preventSubMenuClose) {
subMenu.close();
subMenu.remove();
parentItem._subMenu = undefined;
if (keyboard) {
subMenu._opener?.focus();
}
this._openedSubMenuItem = undefined;
this._subMenuOpenerId = "";
}
}
}

_prepareSubMenuDesktopTablet(item: MenuItem, opener: HTMLElement, actionId: string) {
if (actionId !== this._subMenuOpenerId || (item && item.hasSubmenu)) {
_prepareSubMenuDesktopTablet(item: MenuItem, opener: HTMLElement) {
if (opener.id !== this._subMenuOpenerId || (item && item.hasSubmenu)) {
// close opened sub-menu if there is any opened
this._closeItemSubMenu(this._openedSubMenuItem!, true);
}
if (item && item.hasSubmenu) {
// create new sub-menu
this._createSubMenu(item, actionId);
this._openItemSubMenu(item, opener, actionId);
this._createSubMenu(item, opener);
this._openItemSubMenu(item, opener);
}
if (this._parentMenuItem) {
this._parentMenuItem._preventSubMenuClose = true;
Expand All @@ -542,46 +546,44 @@ class Menu extends UI5Element {
this._parentItemsStack.push(item);
}

_startOpenTimeout(item: MenuItem, opener: OpenerStandardListItem, hoverId: string) {
// If theres already a timeout, clears it
this._clearTimeout();
_onfocusin(e: FocusEvent): void {
const target = e.target as HTMLElement;
const menuListItem = target.hasAttribute("ui5-menu-li")
? target as MenuListItem
: (target.getRootNode() as ShadowRoot).host as MenuListItem;
const item = menuListItem.associatedItem as MenuItem;
const mainMenu = this._findMainMenu(item);
mainMenu?.fireEvent("item-focus", { ref: menuListItem, item });
}

_startOpenTimeout(item: MenuItem, opener: OpenerStandardListItem) {
clearTimeout(this._timeout);

// Sets the new timeout
this._timeout = setTimeout(() => {
this._prepareSubMenuDesktopTablet(item, opener, hoverId);
this._prepareSubMenuDesktopTablet(item, opener);
}, MENU_OPEN_DELAY);
}

_startCloseTimeout(item: MenuItem) {
// If theres already a timeout, clears it
this._clearTimeout();
clearTimeout(this._timeout);

// Sets the new timeout
this._timeout = setTimeout(() => {
this._closeItemSubMenu(item);
}, MENU_CLOSE_DELAY);
}

_clearTimeout() {
if (this._timeout) {
clearTimeout(this._timeout);
}
}

_itemMouseOver(e: MouseEvent) {
if (isDesktop()) {
// respect mouseover only on desktop
const opener = e.target as OpenerStandardListItem;
const item = opener.associatedItem;
const hoverId = opener.getAttribute("id")!;

opener.focus();

// If there is a pending close operation, cancel it
this._clearTimeout();

// Opens submenu with 300ms delay
this._startOpenTimeout(item, opener, hoverId);
this._startOpenTimeout(item, opener);
}
}

Expand All @@ -596,8 +598,7 @@ class Menu extends UI5Element {
const opener = e.target as OpenerStandardListItem;
const item = opener.associatedItem;

// If there is a pending open operation, cancel it
this._clearTimeout();
clearTimeout(this._timeout);

// Close submenu with 400ms delay
if (item && item.hasSubmenu && item._subMenu) {
Expand All @@ -609,28 +610,26 @@ class Menu extends UI5Element {
}

_itemKeyDown(e: KeyboardEvent) {
const isMenuClose = this.isRtl ? isRight(e) : isLeft(e);
const isMenuOpen = this.isRtl ? isLeft(e) : isRight(e);
const shouldCloseMenu = this.isRtl ? isRight(e) : isLeft(e);
const shouldOpenMenu = this.isRtl ? isLeft(e) : isRight(e);

if (isEnter(e)) {
e.preventDefault();
}
if (isMenuOpen) {
if (shouldOpenMenu) {
const opener = e.target as OpenerStandardListItem;
const item = opener.associatedItem;
const hoverId = opener.getAttribute("id")!;

item.hasSubmenu && this._prepareSubMenuDesktopTablet(item, opener, hoverId);
} else if (isMenuClose && this._isSubMenu && this._parentMenuItem) {
item.hasSubmenu && this._prepareSubMenuDesktopTablet(item, opener);
} else if (shouldCloseMenu && this._isSubMenu && this._parentMenuItem) {
const parentMenuItemParent = this._parentMenuItem.parentElement as Menu;
parentMenuItemParent._closeItemSubMenu(this._parentMenuItem, true);
parentMenuItemParent._closeItemSubMenu(this._parentMenuItem, true, true);
}
}

_itemClick(e: CustomEvent<ListItemClickEventDetail>) {
const opener = e.detail.item as OpenerStandardListItem;
const item = opener.associatedItem;
const actionId = opener.getAttribute("id")!;

if (!item.hasSubmenu) {
// click on an item that doesn't have sub-items fires an "item-click" event
Expand Down Expand Up @@ -672,7 +671,7 @@ class Menu extends UI5Element {
this._prepareSubMenuPhone(item);
} else if (isTablet()) {
// prepares and opens sub-menu on tablet
this._prepareSubMenuDesktopTablet(item, opener, actionId);
this._prepareSubMenuDesktopTablet(item, opener);
}
}

Expand Down
9 changes: 2 additions & 7 deletions packages/main/src/NavigationMenu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ class NavigationMenu extends Menu {
// respect mouseover only on desktop
const opener = e.target as OpenerStandardListItem;
let item = opener.associatedItem;
const hoverId = opener.getAttribute("id")!;

if (!item) {
// for nested <a>
Expand All @@ -82,11 +81,8 @@ class NavigationMenu extends Menu {
}
}

// If there is a pending close operation, cancel it
this._clearTimeout();

// Opens submenu with 300ms delay
this._startOpenTimeout(item, opener, hoverId);
this._startOpenTimeout(item, opener);
}
}

Expand All @@ -109,7 +105,6 @@ class NavigationMenu extends Menu {
_itemClick(e: CustomEvent<ListItemClickEventDetail>) {
const opener = e.detail.item as OpenerStandardListItem;
const item = opener.associatedItem;
const actionId = opener.getAttribute("id")!;
const mainMenu = this._findMainMenu(item);
const prevented = !mainMenu.fireEvent<MenuItemClickEventDetail>("item-click", {
"item": item,
Expand All @@ -134,7 +129,7 @@ class NavigationMenu extends Menu {
this._prepareSubMenuPhone(item);
} else if (isTablet()) {
// prepares and opens sub-menu on tablet
this._prepareSubMenuDesktopTablet(item, opener, actionId);
this._prepareSubMenuDesktopTablet(item, opener);
}
}
get accSideNavigationPopoverHiddenText() {
Expand Down
65 changes: 63 additions & 2 deletions packages/main/test/pages/Menu.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@
<body class="bg">
<ui5-button id="btnOpen">Open Menu</ui5-button> <br/>
<ui5-menu id="menu" header-text="My ui5-menu">

<ui5-menu-item text="New File" accessible-name="Opens a file explorer" additional-text="Ctrl+Alt+Shift+N" tooltip="Select a file" icon="add-document"></ui5-menu-item>
<ui5-menu-item text="New Folder with very long title for a menu item" additional-text="Ctrl+F" icon="add-folder" disabled></ui5-menu-item>
<ui5-menu-item text="Open" icon="open-folder" starts-section busy-delay="100" busy>
<ui5-menu-item text="Open Locally" icon="open-folder" additional-text="Ctrl+K">
<ui5-menu-item text="Open from C"></ui5-menu-item>
<ui5-menu-item text="Open from D"></ui5-menu-item>
<ui5-menu-item text="Open from E"></ui5-menu-item>
<ui5-menu-item text="Open from E" disabled></ui5-menu-item>
</ui5-menu-item>
<ui5-menu-item text="Open from SAP Cloud" additional-text="Ctrl+L"></ui5-menu-item>
</ui5-menu-item>
Expand Down Expand Up @@ -60,6 +59,24 @@
<ui5-title level="H5" class="header-title">Event logger</ui5-title>
<ui5-input id="eventLogger" style="width: 100%;"></ui5-input>

<ui5-button id="btnOpen2">Open Menu</ui5-button> <br/>
<ui5-menu id="menu2" header-text="My ui5-menu">
<ui5-menu-item text="New Folder" additional-text="Ctrl+F" icon="add-folder" disabled></ui5-menu-item>
<ui5-menu-item text="Open" icon="open-folder" starts-section>
<ui5-menu-item text="Open Locally" icon="open-folder" additional-text="Ctrl+K"></ui5-menu-item>
<ui5-menu-item text="Open from SAP Cloud" additional-text="Ctrl+L" disabled></ui5-menu-item>
</ui5-menu-item>
<ui5-menu-item text="Preferences" icon="action-settings" starts-section disabled></ui5-menu-item>
<ui5-menu-item text="Exit" icon="journey-arrive"></ui5-menu-item>
</ui5-menu>

<ui5-popover id="detailsPopover">
<div>
<ui5-label for="detailsLink">For more information:</ui5-label>
</div>
<ui5-link id="detailsLink" href="https://en.wikipedia.org/wiki/Google_logo" target="_blank"></ui5-link>
</ui5-popover>

<script>
btnOpen.accessibilityAttributes = {
hasPopup: "Menu",
Expand Down Expand Up @@ -114,6 +131,7 @@
});

menu.addEventListener("ui5-after-open", function() {
shouldOpenDetailsPopover = true;
var opener = menu.opener.id ? menu.opener.id : menu.opener;
openerInput.value = opener;
eventLogger.value += "* [after-open] ";
Expand All @@ -125,6 +143,7 @@
});

menu.addEventListener("ui5-after-close", function() {
shouldOpenDetailsPopover = false;
openerInput.value = "";
btnToggleOpen.pressed = false;
eventLogger.value += "* [after-close]";
Expand All @@ -135,6 +154,48 @@
window["sap-ui-webcomponents-bundle"].applyDirection();
});

btnOpen2.accessibilityAttributes = {
hasPopup: "Menu",
};

btnOpen2.addEventListener("click", function() {
menu2.showAt(btnOpen2);
});

let shouldOpenDetailsPopover = false;

detailsPopover.addEventListener("ui5-before-close", function() {
shouldOpenDetailsPopover = false;
});

detailsPopover.addEventListener("ui5-after-close", function() {
shouldOpenDetailsPopover = true;
});

menu2.addEventListener("ui5-after-open", function() {
shouldOpenDetailsPopover = true;
});

menu2.addEventListener("ui5-after-close", function() {
shouldOpenDetailsPopover = false;
});

menu2.addEventListener("ui5-item-focus", function(event) {
const item = event.detail.item;
const opener = event.detail.ref;

if (detailsPopover.isOpen()) {
detailsPopover.close(false, false, true);
}

if (item.disabled && shouldOpenDetailsPopover) {
const link = detailsPopover.querySelector("#detailsLink");
link.textContent = item.text;
detailsPopover.showAt(opener, true)
link.focus();
}
});

</script>
</body>
</html>
2 changes: 1 addition & 1 deletion packages/main/test/specs/Menu.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe("Menu interaction", () => {
it("Top level menu items appearance", async () => {
await browser.url(`test/pages/Menu.html`);
const openButton = await browser.$("#btnOpen");
const menuItems = await browser.$$("ui5-menu>ui5-menu-item");
const menuItems = await browser.$$("ui5-menu[id='menu']>ui5-menu-item");

openButton.click();

Expand Down

0 comments on commit bd33dc5

Please sign in to comment.