Skip to content

Commit

Permalink
fix(ui5-menu): restore focus to the opener (#9041)
Browse files Browse the repository at this point in the history
- The focus is restored over the opener element after root menu close.
- Redundant icon dependency is removed from the menu.
- Escape keyboard key press closes all dialogs on mobile device.
- Sub-menu could not be opened from a disabled ui5-menu-item.
- The leftover start-section property usage is replaced with the ui5-menu-separator component usage.

Fixes: #9317
  • Loading branch information
unazko authored Jul 3, 2024
1 parent 1fb5822 commit 0ada944
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 8 deletions.
1 change: 0 additions & 1 deletion packages/main/src/Menu.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
.opener={{opener}}
?open={{open}}
prevent-initial-focus
prevent-focus-restore
hide-arrow
allow-target-overlap
@ui5-before-open={{_beforePopoverOpen}}
Expand Down
3 changes: 0 additions & 3 deletions packages/main/src/Menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import ResponsivePopover from "./ResponsivePopover.js";
import type { ResponsivePopoverBeforeCloseEventDetail } from "./ResponsivePopover.js";
import Button from "./Button.js";
import List from "./List.js";
import Icon from "./Icon.js";
import BusyIndicator from "./BusyIndicator.js";
import MenuItem from "./MenuItem.js";
import MenuSeparator from "./MenuSeparator.js";
Expand Down Expand Up @@ -104,7 +103,6 @@ type MenuBeforeCloseEventDetail = { escPressed: boolean };
List,
MenuItem,
MenuSeparator,
Icon,
BusyIndicator,
],
})
Expand Down Expand Up @@ -387,7 +385,6 @@ class Menu extends UI5Element {
}

_afterPopoverOpen() {
this.open = true;
this._menuItems[0]?.focus();
this.fireEvent("open", {}, false, true);
}
Expand Down
5 changes: 4 additions & 1 deletion packages/main/src/MenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class MenuItem extends ListItem implements IMenuItem {
}

get hasSubmenu() {
return !!(this.items.length || this.loading);
return !!(this.items.length || this.loading) && !this.disabled;
}

get hasEndContent() {
Expand Down Expand Up @@ -296,6 +296,9 @@ class MenuItem extends ListItem implements IMenuItem {
this.selected = false;
if (e.detail.escPressed) {
this.focus();
if (isPhone()) {
this.fireEvent("close-menu", {});
}
}
}

Expand Down
11 changes: 8 additions & 3 deletions packages/main/test/pages/Menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
<ui5-button id="btnOpen">Open Menu</ui5-button> <br/>
<ui5-menu id="menu" header-text="My ui5-menu">
<ui5-menu-item text="New File(selection prevented)" accessible-name="Opens a file explorer" additional-text="Ctrl+Alt+Shift+N" tooltip="Select a file - prevent default" 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="New Folder with very long title for a menu item" additional-text="Ctrl+F" icon="add-folder" disabled>
<ui5-menu-item text="test.txt"></ui5-menu-item>
</ui5-menu-item>
<ui5-menu-separator></ui5-menu-separator>
<ui5-menu-item text="Open" icon="open-folder" accessible-name="Choose platform" loading-delay="100" loading>
<ui5-menu-item text="Open Locally" icon="open-folder" additional-text="Ctrl+K">
Expand Down Expand Up @@ -85,10 +87,12 @@
<ui5-button id="newFavorite" slot="endContent" icon="favorite" design="Transparent"></ui5-button>
</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" accessible-name="Choose platform" starts-section>
<ui5-menu-separator></ui5-menu-separator>
<ui5-menu-item text="Open" icon="open-folder" accessible-name="Choose platform">
<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-separator></ui5-menu-separator>
<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>
Expand All @@ -102,7 +106,8 @@
<ui5-menu-item text="Save on Cloud" icon="upload-to-cloud"></ui5-menu-item>
</ui5-menu-item>
<ui5-menu-item text="Close" additional-text="Ctrl+W"></ui5-menu-item>
<ui5-menu-item text="Preferences" icon="action-settings" starts-section disabled></ui5-menu-item>
<ui5-menu-separator></ui5-menu-separator>
<ui5-menu-item text="Preferences" icon="action-settings" disabled></ui5-menu-item>
<ui5-menu-item text="Exit" icon="journey-arrive"></ui5-menu-item>
</ui5-menu>

Expand Down

0 comments on commit 0ada944

Please sign in to comment.