Skip to content
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.

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

fix(ui5-menu): focus first element after adding new items #10005

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
27 changes: 26 additions & 1 deletion packages/main/src/Menu.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
import UI5Element, { type ChangeInfo } 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.js";
Expand Down Expand Up @@ -234,6 +234,12 @@ class Menu extends UI5Element {
@property({ converter: DOMReferenceConverter })
opener?: HTMLElement | string;

/**
* Indicates if the menu item has a submenu.
*/
@property({ type: Boolean, noAttribute: true})
_newItemsAdded?: boolean;

/**
* Defines the items of this component.
*
Expand Down Expand Up @@ -277,6 +283,25 @@ class Menu extends UI5Element {
this._menuItems.forEach(item => {
item._siblingsWithIcon = siblingsWithIcon;
});

if (this._newItemsAdded) {
if (this._menuItems.length > 0) {
const hasSelectedOrFocusedItem = this._menuItems.some(item => item.selected);

unazko marked this conversation as resolved.
Show resolved Hide resolved
if (!hasSelectedOrFocusedItem) {
this._menuItems[0].focus();
}
if (this._menuItems[0].focused) {
this._newItemsAdded = false;
}
}
}
}

onInvalidation(changeInfo: ChangeInfo) {
if (changeInfo.reason === "children" && changeInfo.type === "slot") {
this._newItemsAdded = true;
}
}

_close() {
Expand Down
27 changes: 27 additions & 0 deletions packages/main/src/MenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type { IMenuItem } from "./Menu.js";

// Styles
import menuItemCss from "./generated/themes/MenuItem.css.js";
import type { ChangeInfo } from "@ui5/webcomponents-base/dist/UI5Element.js";

type MenuBeforeOpenEventDetail = { item?: MenuItem };
type MenuBeforeCloseEventDetail = { escPressed: boolean };
Expand Down Expand Up @@ -166,6 +167,12 @@ class MenuItem extends ListItem implements IMenuItem {
@property({ type: Boolean, noAttribute: true })
_siblingsWithIcon = false;

/**
* Indicates if the menu item has a submenu.
*/
@property({ type: Boolean, noAttribute: true})
_newItemsAdded?: boolean;

/**
* Defines the items of this component.
*
Expand Down Expand Up @@ -259,8 +266,28 @@ class MenuItem extends ListItem implements IMenuItem {
this._menuItems.forEach(item => {
item._siblingsWithIcon = siblingsWithIcon;
});

if (this._newItemsAdded) {
if (this._menuItems.length > 0) {
const hasSelectedOrFocusedItem = this._menuItems.some(item => item.selected);

if (!hasSelectedOrFocusedItem) {
this._menuItems[0].focus();
}
if (this._menuItems[0].focused) {
this._newItemsAdded = false;
}
}
}
}

onInvalidation(changeInfo: ChangeInfo) {
if (changeInfo.reason === "children" && changeInfo.type === "slot") {
this._newItemsAdded = true;
}
}


get _focusable() {
return true;
}
Expand Down
28 changes: 26 additions & 2 deletions packages/main/test/pages/Menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
</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">
<!-- <ui5-menu-item text="Open Locally" icon="open-folder" additional-text="Ctrl+K">
<ui5-menu-item id="menuItemFromC" text="Open from C"></ui5-menu-item>
<ui5-menu-item id="menuItemFromD" 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>
<ui5-menu-item text="Open from SAP Cloud" additional-text="Ctrl+L"></ui5-menu-item> -->
</ui5-menu-item>
<ui5-menu-item text="Save with very long title for a menu item text inside" icon="save">
<ui5-menu-item text="Save Locally" icon="save">
Expand All @@ -43,6 +43,9 @@
<ui5-menu-item id="menuItemExit" text="Exit" icon="journey-arrive"></ui5-menu-item>
</ui5-menu>

<ui5-menu id="delaymenu" loading-delay="100" loading>
</ui5-menu>

<ui5-title level="H5" class="header-title">Clicked menu item text</ui5-title>
<ui5-input id="selectionInput"></ui5-input>

Expand Down Expand Up @@ -119,6 +122,9 @@
<ui5-link id="detailsLink" href="https://en.wikipedia.org/wiki/Google_logo" target="_blank"></ui5-link>
</ui5-popover>


<ui5-button id="btnAddOpenerDelay">Delayed</ui5-button>

<script>
btnOpen.accessibilityAttributes = {
hasPopup: "menu",
Expand Down Expand Up @@ -146,6 +152,24 @@
menu.open = btnToggleOpen.pressed;
});

btnAddOpenerDelay.addEventListener("click", function() {
delaymenu.opener = "btnAddOpenerDelay";
delaymenu.open = !delaymenu.open;
});

delaymenu.addEventListener("ui5-before-open", function(event) {
setTimeout(function() {
delaymenu.removeAttribute("loading");
delaymenu.removeAttribute("loading-delay");
let oneNode = document.createElement("ui5-menu-item");
oneNode.setAttribute("text", "Open from Amazon Cloud");
let twoNode = document.createElement("ui5-menu-item");
twoNode.setAttribute("text", "Open from Google Cloud");
delaymenu.append(oneNode, twoNode);
fetched = true;
}, 1000);
});

let fetched = false;

menu.addEventListener("ui5-before-open", function(event) {
Expand Down
Loading