Skip to content

Commit

Permalink
feat(ui5-menu-separator): add new component (#8871)
Browse files Browse the repository at this point in the history
New component that can be slotted with menu items intended to separate the items with a line.

BREAKING CHANGE: `startsSection` property removed from MenuItems

Before:

<ui5-menu>
    <ui5-menu-item text="Item A"></ui5-menu-item>
    <ui5-menu-item text="Item B" starts-section></ui5-menu-item>
</ui5-menu>

Now:

<ui5-menu>
    <ui5-menu-item text="Item A"></ui5-menu-item>
    <ui5-menu-separator></ui5-menu-separator>
    <ui5-menu-item text="Item B"></ui5-menu-item>
</ui5-menu>

Related to: #8461
  • Loading branch information
didip1000 authored Jun 21, 2024
1 parent ca0509a commit f7fea29
Show file tree
Hide file tree
Showing 12 changed files with 163 additions and 40 deletions.
47 changes: 35 additions & 12 deletions packages/main/src/Menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ 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";
import type {
ListItemClickEventDetail,
} from "./List.js";
Expand All @@ -39,6 +40,16 @@ import menuCss from "./generated/themes/Menu.css.js";

const MENU_OPEN_DELAY = 300;

/**
* Interface for components that may be slotted inside a `ui5-menu`.
*
* **Note:** Use with `ui5-menu-item` or `ui5-menu-separator`. Implementing the interface does not guarantee that any other classes can work with the `ui5-menu`.
* @public
*/
interface IMenuItem extends UI5Element {
isSeparator: boolean;
}

type MenuItemClickEventDetail = {
item: MenuItem,
text: string,
Expand All @@ -54,9 +65,13 @@ type MenuBeforeCloseEventDetail = { escPressed: boolean };
*
* `ui5-menu` component represents a hierarchical menu structure.
*
* ### Usage
* ### Structure
*
* The `ui5-menu` can hold two types of entities:
*
* - `ui5-menu-item` components
* - `ui5-menu-separator` - used to separate menu items with a line
*
* `ui5-menu` contains `ui5-menu-item` components.
* An arbitrary hierarchy structure can be represented by recursively nesting menu items.
*
* ### Keyboard Handling
Expand Down Expand Up @@ -89,6 +104,7 @@ type MenuBeforeCloseEventDetail = { escPressed: boolean };
Button,
List,
MenuItem,
MenuSeparator,
Icon,
BusyIndicator,
],
Expand Down Expand Up @@ -223,11 +239,11 @@ class Menu extends UI5Element {
/**
* Defines the items of this component.
*
* **Note:** Use `ui5-menu-item` for the intended design.
* **Note:** Use `ui5-menu-item` and `ui5-menu-separator` for their intended design.
* @public
*/
@slot({ "default": true, type: HTMLElement, invalidateOnChildChange: true })
items!: Array<MenuItem>;
items!: Array<IMenuItem>;

static i18nBundle: I18nBundle;
_timeout?: Timeout;
Expand All @@ -252,10 +268,14 @@ class Menu extends UI5Element {
return this.shadowRoot!.querySelector<ResponsivePopover>("[ui5-responsive-popover]")!;
}

get _menuItems() {
return this.items.filter((item): item is MenuItem => !item.isSeparator);
}

onBeforeRendering() {
const siblingsWithIcon = this.items.some(item => !!item.icon);
const siblingsWithIcon = this._menuItems.some(menuItem => !!menuItem.icon);

this.items.forEach(item => {
this._menuItems.forEach(item => {
item._siblingsWithIcon = siblingsWithIcon;
});
}
Expand All @@ -281,7 +301,7 @@ class Menu extends UI5Element {

_closeItemSubMenu(item: MenuItem) {
if (item && item._popover) {
const openedSibling = item.items.find(menuItem => menuItem._popover && menuItem._popover.open);
const openedSibling = item._menuItems.find(menuItem => menuItem._popover && menuItem._popover.open);
if (openedSibling) {
this._closeItemSubMenu(openedSibling);
}
Expand All @@ -296,10 +316,12 @@ class Menu extends UI5Element {
// respect mouseover only on desktop
const item = e.target as MenuItem;

item.focus();
if (item.hasAttribute("ui5-menu-item")) {
item.focus();

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

Expand All @@ -308,7 +330,7 @@ class Menu extends UI5Element {

this._timeout = setTimeout(() => {
const opener = item.parentElement as MenuItem | Menu;
const openedSibling = opener && opener.items.find(menuItem => menuItem._popover && menuItem._popover.open);
const openedSibling = opener && opener._menuItems.find(menuItem => menuItem._popover && menuItem._popover.open);
if (openedSibling) {
this._closeItemSubMenu(openedSibling);
}
Expand Down Expand Up @@ -367,7 +389,7 @@ class Menu extends UI5Element {

_afterPopoverOpen() {
this.open = true;
this.items[0]?.focus();
this._menuItems[0]?.focus();
this.fireEvent("open", {}, false, true);
}

Expand All @@ -393,4 +415,5 @@ export type {
MenuItemClickEventDetail,
MenuBeforeCloseEventDetail,
MenuBeforeOpenEventDetail,
IMenuItem,
};
30 changes: 17 additions & 13 deletions packages/main/src/MenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
MENU_CLOSE_BUTTON_ARIA_LABEL,
} from "./generated/i18n/i18n-defaults.js";
import type { ResponsivePopoverBeforeCloseEventDetail } from "./ResponsivePopover.js";
import type { IMenuItem } from "./Menu.js";

// Styles
import menuItemCss from "./generated/themes/MenuItem.css.js";
Expand All @@ -43,6 +44,7 @@ type MenuBeforeCloseEventDetail = { escPressed: boolean };
* `import "@ui5/webcomponents/dist/MenuItem.js";`
* @constructor
* @extends ListItem
* @implements {IMenuItem}
* @since 1.3.0
* @public
*/
Expand All @@ -52,7 +54,7 @@ type MenuBeforeCloseEventDetail = { escPressed: boolean };
styles: [ListItem.styles, menuItemCss],
dependencies: [...ListItem.dependencies, ResponsivePopover, List, BusyIndicator, Icon],
})
class MenuItem extends ListItem {
class MenuItem extends ListItem implements IMenuItem {
static async onDefine() {
MenuItem.i18nBundle = await getI18nBundle("@ui5/webcomponents");
}
Expand Down Expand Up @@ -93,14 +95,6 @@ class MenuItem extends ListItem {
@property()
icon!: string;

/**
* Defines whether a visual separator should be rendered before the item.
* @default false
* @public
*/
@property({ type: Boolean })
startsSection!: boolean;

/**
* Defines whether `ui5-menu-item` is in disabled state.
*
Expand Down Expand Up @@ -158,7 +152,9 @@ class MenuItem extends ListItem {
/**
* Defines the items of this component.
*
* **Note:** If there are items added to this slot, an arrow will be displayed at the end
* **Note:** The slot can hold `ui5-menu-item` and `ui5-menu-separator` items.
*
* If there are items added to this slot, an arrow will be displayed at the end
* of the item in order to indicate that there are items added. In that case components added
* to `endContent` slot or `additionalText` content will not be displayed.
*
Expand All @@ -167,7 +163,7 @@ class MenuItem extends ListItem {
* @public
*/
@slot({ "default": true, type: HTMLElement, invalidateOnChildChange: true })
items!: Array<MenuItem>;
items!: Array<IMenuItem>;

/**
* Defines the components that should be displayed at the end of the menu item.
Expand Down Expand Up @@ -229,10 +225,14 @@ class MenuItem extends ListItem {
return MenuItem.i18nBundle.getText(MENU_CLOSE_BUTTON_ARIA_LABEL);
}

get isSeparator(): boolean {
return false;
}

onBeforeRendering() {
const siblingsWithIcon = this.items.some(item => !!item.icon);
const siblingsWithIcon = this._menuItems.some(menuItem => !!menuItem.icon);

this.items.forEach(item => {
this._menuItems.forEach(item => {
item._siblingsWithIcon = siblingsWithIcon;
});
}
Expand All @@ -254,6 +254,10 @@ class MenuItem extends ListItem {
return this.shadowRoot!.querySelector<ResponsivePopover>("[ui5-responsive-popover]")!;
}

get _menuItems() {
return this.items.filter((item): item is MenuItem => !item.isSeparator);
}

_closeAll() {
if (this._popover) {
this._popover.open = false;
Expand Down
6 changes: 6 additions & 0 deletions packages/main/src/MenuSeparator.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<ui5-li-custom
class="{{classes.main}}"
role="separator"
disabled
>
</ui5-li-custom>
66 changes: 66 additions & 0 deletions packages/main/src/MenuSeparator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import customElement from "@ui5/webcomponents-base/dist/decorators/customElement.js";
import property from "@ui5/webcomponents-base/dist/decorators/property.js";
import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js";
import type { ClassMap } from "@ui5/webcomponents-base/dist/types.js";
import menuSeparatorTemplate from "./generated/templates/MenuSeparatorTemplate.lit.js";
import menuSeparatorCss from "./generated/themes/MenuSeparator.css.js";
import ListItemBase from "./ListItemBase.js";
import ListItemCustom from "./ListItemCustom.js";
import type { IMenuItem } from "./Menu.js";
/**
* @class
* The `ui5-menu-separator` represents a horizontal line to separate menu items inside a `ui5-menu`.
* @constructor
* @extends ListItemBase
* @implements {IMenuItem}
* @public
* @since 2.0
*/
@customElement({
tag: "ui5-menu-separator",
renderer: litRender,
styles: [menuSeparatorCss],
template: menuSeparatorTemplate,
dependencies: [
ListItemCustom,
],
})

class MenuSeparator extends ListItemBase implements IMenuItem {
/**
* **Note:** This property is inherited and not supported. If set, it won't take any effect.
* @default false
* @public
*/
@property({ type: Boolean })
declare selected: boolean;

get isSeparator() {
return true;
}

get classes(): ClassMap {
return {
main: {
"ui5-menu-separator": true,
},
};
}

/**
* @override
*/
get _focusable() {
return false;
}

/**
* @override
*/
get _pressable() {
return false;
}
}
MenuSeparator.define();

export default MenuSeparator;
1 change: 1 addition & 0 deletions packages/main/src/bundle.esm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import Menu from "./Menu.js";
import NavigationMenu from "./NavigationMenu.js";
import NavigationMenuItem from "./NavigationMenuItem.js";
import MenuItem from "./MenuItem.js";
import MenuSeparator from "./MenuSeparator.js";
import Popover from "./Popover.js";
import Panel from "./Panel.js";
import RadioButton from "./RadioButton.js";
Expand Down
4 changes: 0 additions & 4 deletions packages/main/src/themes/MenuItem.css
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@
padding: var(--_ui5_menu_item_padding);
}

:host([starts-section]) {
border-top: 1px solid var(--sapGroup_ContentBorderColor);
}

:host::part(content) {
padding-inline-end: 0.25rem;
}
Expand Down
11 changes: 11 additions & 0 deletions packages/main/src/themes/MenuSeparator.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
:host {
border-top: 0.0625rem solid var(--sapGroup_ContentBorderColor);
min-height: 0.125rem;
}

.ui5-menu-separator {
border: inherit;
min-height: inherit;
background: inherit;
opacity: 1;
}
13 changes: 9 additions & 4 deletions packages/main/test/pages/Menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
<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="Open" icon="open-folder" accessible-name="Choose platform" starts-section loading-delay="100" loading>
<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 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 @@ -34,7 +36,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" loading-delay="100" loading></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 Expand Up @@ -63,11 +66,13 @@
<ui5-button id="btnOpen2">Open Menu</ui5-button> <br/><br/>
<ui5-menu id="menu2" header-text="My ui5-menu">
<ui5-menu-item id="newFolder" 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-separator></ui5-menu-separator>
<ui5-menu-item text="Open" icon="open-folder">
<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 id="preferences" text="Preferences" icon="action-settings" starts-section disabled></ui5-menu-item>
<ui5-menu-separator></ui5-menu-separator>
<ui5-menu-item id="preferences" text="Preferences" icon="action-settings" disabled></ui5-menu-item>
<ui5-menu-item text="Exit" icon="journey-arrive"></ui5-menu-item>
</ui5-menu>

Expand Down
11 changes: 6 additions & 5 deletions packages/main/test/specs/Menu.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,17 @@ 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.$$("#menu > ui5-menu-item");
const menuItems = await browser.$$("#menu > *");

await openButton.click();

assert.strictEqual(await menuItems.length, 7, "There are proper count of menu items in the top level menu");
assert.strictEqual(await menuItems.length, 9, "There are proper count of menu items in the top level menu");
assert.strictEqual(await menuItems[0].getAttribute("ui5-menu-item"), "", "The first list item is a menu item");
assert.strictEqual(await menuItems[0].getAttribute("additional-text"), "Ctrl+Alt+Shift+N", "The first list item has proper additional text set");
assert.strictEqual(await menuItems[1].getAttribute("disabled"), "true", "The second list item is disabled");
assert.strictEqual(await menuItems[2].getAttribute("starts-section"), "", "The third list item has separator addded");
assert.ok(await menuItems[3].$(".ui5-menu-item-icon-end"), "The third list item has sub-items and must have arrow right icon after the text");
assert.ok(await menuItems[4].$(".ui5-menu-item-dummy-icon"), "The fourth list item has no icon and has dummy div instead of icon");
assert.strictEqual(await menuItems[2].getAttribute("ui5-menu-separator"), "", "The third list item is a menu separator");
assert.ok(await menuItems[3].$(".ui5-menu-item-icon-end"), "The fourth list item has sub-items and must have arrow right icon after the text");
assert.ok(await menuItems[4].$(".ui5-menu-item-dummy-icon"), "The fifth list item has no icon and has dummy div instead of icon");
});

it("Sub-menu creation, opening, closing and destroying", async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
slug: ../../MenuSeparator
---

<%COMPONENT_OVERVIEW%>

<%COMPONENT_METADATA%>
Loading

0 comments on commit f7fea29

Please sign in to comment.