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

feat: theme property for individual menu items #2401

Merged
merged 8 commits into from
Sep 2, 2021
Merged
1 change: 1 addition & 0 deletions packages/vaadin-context-menu/src/interfaces.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export interface ContextMenuItem {
component?: string | HTMLElement;
disabled?: boolean;
checked?: boolean;
theme?: string | string[];
children?: ContextMenuItem[];
}

Expand Down
2 changes: 1 addition & 1 deletion packages/vaadin-context-menu/src/vaadin-context-menu.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { ContextMenuEventMap, ContextMenuRenderer } from './interfaces';
*
* ```javascript
* contextMenu.items = [
* {text: 'Menu Item 1', children:
* {text: 'Menu Item 1', theme: 'primary', children:
* [
* {text: 'Menu Item 1-1', checked: true},
* {text: 'Menu Item 1-2'}
Expand Down
2 changes: 1 addition & 1 deletion packages/vaadin-context-menu/src/vaadin-context-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import './vaadin-context-menu-overlay.js';
*
* ```javascript
* contextMenu.items = [
* {text: 'Menu Item 1', children:
* {text: 'Menu Item 1', theme: 'primary', children:
* [
* {text: 'Menu Item 1-1', checked: true},
* {text: 'Menu Item 1-2'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ interface ItemsMixin {
*
* ```javascript
* contextMenu.items = [
* {text: 'Menu Item 1', children:
* {text: 'Menu Item 1', theme: 'primary', children:
* [
* {text: 'Menu Item 1-1', checked: true},
* {text: 'Menu Item 1-2'}
Expand Down
18 changes: 16 additions & 2 deletions packages/vaadin-context-menu/src/vaadin-contextmenu-items-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const ItemsMixin = (superClass) =>
* Either a tagName or an element instance. Defaults to "vaadin-context-menu-item".
* @property {boolean} disabled - If true, the item is disabled and cannot be selected
* @property {boolean} checked - If true, the item shows a checkmark next to it
* @property {union: string | string[]} theme - If set, sets the given theme(s) as an attribute to the menu item component, overriding any theme set on the context menu.
* @property {MenuItem[]} children - Array of child menu items
*/

Expand All @@ -64,7 +65,7 @@ export const ItemsMixin = (superClass) =>
*
* ```javascript
* contextMenu.items = [
* {text: 'Menu Item 1', children:
* {text: 'Menu Item 1', theme: 'primary', children:
* [
* {text: 'Menu Item 1-1', checked: true},
* {text: 'Menu Item 1-2'}
Expand Down Expand Up @@ -230,7 +231,15 @@ export const ItemsMixin = (superClass) =>
} else if (component.localName === 'hr') {
component.setAttribute('role', 'separator');
}
this.theme && component.setAttribute('theme', this.theme);

const itemTheme = this.__stringOrArrayToString(item.theme);
web-padawan marked this conversation as resolved.
Show resolved Hide resolved
if (itemTheme) {
component.setAttribute('theme', itemTheme);
} else if (this.theme) {
component.setAttribute('theme', this.theme);
} else {
component.removeAttribute('theme');
}

component._item = item;

Expand Down Expand Up @@ -376,4 +385,9 @@ export const ItemsMixin = (superClass) =>
}
}
}

/** @private */
__stringOrArrayToString(stringOrArray) {
return Array.isArray(stringOrArray) ? stringOrArray.join(' ') : stringOrArray;
}
};
66 changes: 65 additions & 1 deletion packages/vaadin-context-menu/test/items.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ describe('items', () => {
return menu.$.overlay.content.querySelector('vaadin-context-menu');
};

const updateItemsAndReopen = async () => {
rootMenu.items = [...rootMenu.items];
rootMenu.close();
open();
await nextFrame();
};

const getMenuItems = (menu = rootMenu) => {
const overlay = menu.$.overlay;
const listBox = overlay.querySelector('vaadin-context-menu-list-box');
return Array.from(listBox.querySelectorAll('vaadin-context-menu-item'));
};

afterEach(() => {
rootMenu.close();
});
Expand Down Expand Up @@ -492,7 +505,10 @@ describe('items', () => {
rootMenu.openOn = 'mouseover';
target = rootMenu.firstElementChild;
rootMenu.items = [
{ text: 'foo-0', children: [{ text: 'foo-0-0' }, { text: 'foo-0-1', children: [{ text: 'foo-0-1-0' }] }] },
{
text: 'foo-0',
children: [{ text: 'foo-0-0' }, { text: 'foo-0-1', children: [{ text: 'foo-0-1-0' }] }]
},
{ text: 'foo-1' }
];
open();
Expand Down Expand Up @@ -548,5 +564,53 @@ describe('items', () => {
expect(itemsDoNotHaveTheme).to.be.true;
});
});

it('should override the component theme with the item theme', async () => {
rootMenu.items[1].theme = 'bar-1';
rootMenu.items[0].children[0].theme = 'bar-0-0';
await updateItemsAndReopen();

open(menuComponents()[0]);
subMenu = getSubMenu();
await nextFrame();

const rootItems = getMenuItems();
const subItems = getMenuItems(subMenu);

expect(rootItems[0].getAttribute('theme')).to.equal('foo');
expect(rootItems[1].getAttribute('theme')).to.equal('bar-1');

expect(subItems[0].getAttribute('theme')).to.equal('bar-0-0');
expect(subItems[1].getAttribute('theme')).to.equal('foo');
});

it('should use the component theme if the item theme is removed', async () => {
rootMenu.items[1].theme = 'bar-1';
await updateItemsAndReopen();

let rootItems = getMenuItems();
expect(rootItems[1].getAttribute('theme')).to.equal('bar-1');

delete rootMenu.items[1].theme;
await updateItemsAndReopen();

rootItems = getMenuItems();
expect(rootItems[1].getAttribute('theme')).to.equal('foo');

// An empty array should also count as no value
rootMenu.items[1].theme = [];
await updateItemsAndReopen();

rootItems = getMenuItems();
expect(rootItems[1].getAttribute('theme')).to.equal('foo');
});

it('should support multiple item themes in an array', async () => {
rootMenu.items[1].theme = ['bar-1', 'bar-2', 'bar-3'];
await updateItemsAndReopen();

let rootItems = getMenuItems();
expect(rootItems[1].getAttribute('theme')).to.equal('bar-1 bar-2 bar-3');
});
});
});
1 change: 1 addition & 0 deletions packages/vaadin-menu-bar/src/interfaces.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export interface MenuBarItem {
text?: string;
component?: string | HTMLElement;
disabled?: boolean;
theme?: string | string[];
children?: SubMenuItem[];
}

Expand Down
30 changes: 23 additions & 7 deletions packages/vaadin-menu-bar/src/vaadin-menu-bar-buttons-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,17 +249,28 @@ export const ButtonsMixin = (superClass) =>

/** @private */
__applyTheme(theme) {
this._buttons.forEach((btn) => {
if (theme) {
btn.setAttribute('theme', theme);
} else {
btn.removeAttribute('theme');
}
});
this._buttons.forEach((btn) => this._setButtonTheme(btn, theme));

this.__detectOverflow();
}

/** @protected */
_setButtonTheme(btn, hostTheme) {
let theme = hostTheme;

// item theme takes precedence over host theme
const itemTheme = btn.item && btn.item.theme;
if (itemTheme) {
theme = Array.isArray(itemTheme) ? itemTheme.join(' ') : itemTheme;
}

if (theme) {
btn.setAttribute('theme', theme);
} else {
btn.removeAttribute('theme');
}
}

/** @protected */
_appendButton(button) {
this._container.insertBefore(button, this._overflow);
Expand Down Expand Up @@ -313,4 +324,9 @@ export const ButtonsMixin = (superClass) =>
this.__detectOverflow.bind(this)
);
}

/** @private */
__stringOrArrayToString(stringOrArray) {
return Array.isArray(stringOrArray) ? stringOrArray.join(' ') : stringOrArray;
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,17 @@ export const InteractionsMixin = (superClass) =>
/** @private */
_themeChanged(theme) {
if (theme) {
this._buttons.forEach((button) => button.setAttribute('theme', theme));
this._subMenu.setAttribute('theme', theme);
} else {
this._buttons.forEach((button) => button.removeAttribute('theme'));
this._subMenu.removeAttribute('theme');
}
}

/** @private */
__stringOrArrayToString(stringOrArray) {
return Array.isArray(stringOrArray) ? stringOrArray.join(' ') : stringOrArray;
}

/** @protected */
_setExpanded(button, expanded) {
button.toggleAttribute('expanded', expanded);
Expand Down
1 change: 1 addition & 0 deletions packages/vaadin-menu-bar/src/vaadin-menu-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class MenuBarElement extends ButtonsMixin(InteractionsMixin(ElementMixin(Themabl
* @property {union: string | object} component - The component to represent the button content.
* Either a tagName or an element instance. Defaults to "vaadin-context-menu-item".
* @property {boolean} disabled - If true, the button is disabled and cannot be activated.
* @property {union: string | string[]} theme - Theme(s) to be set as the theme attribute of the button, overriding any theme set on the menu bar.
* @property {SubMenuItem[]} children - Array of submenu items.
*/

Expand Down
32 changes: 32 additions & 0 deletions packages/vaadin-menu-bar/test/menu-bar.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,38 @@ describe('root menu layout', () => {
menu.removeAttribute('theme');
buttons.forEach((btn) => expect(btn.hasAttribute('theme')).to.be.false);
});

it('should override the theme attribute of the component with the item.theme property', async () => {
menu.setAttribute('theme', 'contained');
menu.items[1].theme = 'item-theme';
menu.items = [...menu.items];
await nextRender(menu);
buttons = menu._buttons;

expect(buttons[0].getAttribute('theme')).to.equal('contained');
expect(buttons[1].getAttribute('theme')).to.equal('item-theme');

menu.removeAttribute('theme');

expect(buttons[0].hasAttribute('theme')).to.be.false;
expect(buttons[1].getAttribute('theme')).to.equal('item-theme');
});

it('should support setting multiple themes with an array', async () => {
menu.items[1].theme = ['theme-1', 'theme-2'];
menu.items = [...menu.items];
await nextRender(menu);
buttons = menu._buttons;

expect(buttons[1].getAttribute('theme')).to.equal('theme-1 theme-2');

menu.items[1].theme = [];
menu.items = [...menu.items];
await nextRender(menu);
buttons = menu._buttons;

expect(buttons[1].hasAttribute('theme')).to.be.false;
});
});
});

Expand Down
19 changes: 18 additions & 1 deletion packages/vaadin-menu-bar/test/sub-menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ describe('theme attribute', () => {
{
text: 'Menu Item 1',
children: [
{ text: 'Menu Item 1 1' },
{ text: 'Menu Item 1 1', theme: 'sub-item-theme' },
{
text: 'Menu Item 1 2'
}
Expand Down Expand Up @@ -618,6 +618,23 @@ describe('theme attribute', () => {
menu.removeAttribute('theme');
expect(subMenu.hasAttribute('theme')).to.be.false;
});

it('should override the component theme attribute with the item.theme property', async () => {
let items = subMenu.$.overlay.querySelectorAll('vaadin-context-menu-item');

expect(items[0].getAttribute('theme')).to.equal('sub-item-theme');
expect(items[1].getAttribute('theme')).to.equal('foo');

subMenu.close();
menu.removeAttribute('theme');
buttons[0].dispatchEvent(new CustomEvent('mouseover', { bubbles: true, composed: true }));
await nextRender(subMenu);

items = subMenu.$.overlay.querySelectorAll('vaadin-context-menu-item');

expect(items[0].getAttribute('theme')).to.equal('sub-item-theme');
expect(items[1].hasAttribute('theme')).to.be.false;
});
});

describe('touch', () => {
Expand Down