From 1c30e75ee7da2782d0c784097d8ae89825b0e5eb Mon Sep 17 00:00:00 2001 From: Yuriy Yevstihnyeyev Date: Wed, 15 May 2019 11:30:27 +0300 Subject: [PATCH 1/5] Propagate theme attribute to nested overlays, list-boxes and items, add tests --- src/vaadin-context-menu-overlay.html | 31 ++++++++ src/vaadin-context-menu.html | 2 + src/vaadin-contextmenu-items-mixin.html | 9 ++- test/items.html | 99 +++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/vaadin-context-menu-overlay.html b/src/vaadin-context-menu-overlay.html index 3543bad8..8ca9f406 100644 --- a/src/vaadin-context-menu-overlay.html +++ b/src/vaadin-context-menu-overlay.html @@ -69,6 +69,12 @@ parentOverlay: { type: Object, readOnly: true + }, + + theme: { + type: String, + reflectToAttribute: true, + observer: '_themeChanged' } }; } @@ -95,6 +101,31 @@ return this.content.querySelector(':not(style):not(slot)'); } + _themeChanged(theme) { + const childContextMenu = this.querySelector('vaadin-context-menu'); + const childListBox = this.querySelector('vaadin-context-menu-list-box'); + + if (childListBox) { + if (theme && theme !== '') { + childListBox.setAttribute('theme', theme); + Array.from(childListBox.querySelectorAll('vaadin-context-menu-item')) + .forEach(item => item.setAttribute('theme', theme)); + } else { + childListBox.removeAttribute('theme'); + Array.from(childListBox.querySelectorAll('vaadin-context-menu-item')) + .forEach(item => item.removeAttribute('theme')); + } + } + + if (childContextMenu) { + if (theme && theme !== '') { + childContextMenu.setAttribute('theme', theme); + } else { + childContextMenu.removeAttribute('theme'); + } + } + } + getBoundaries() { // Measure actual overlay and content sizes const overlayRect = this.getBoundingClientRect(); diff --git a/src/vaadin-context-menu.html b/src/vaadin-context-menu.html index 70c90dd9..6efeaefe 100644 --- a/src/vaadin-context-menu.html +++ b/src/vaadin-context-menu.html @@ -248,6 +248,8 @@ * * Note: the `theme` attribute value set on `` is * propagated to the internal `` component. + * In case of using nested menu items, the `theme` attribute is also propagated + * to internal `vaadin-context-menu-list-box` and `vaadin-context-menu-item`'s. * * @memberof Vaadin * @mixes Vaadin.ElementMixin diff --git a/src/vaadin-contextmenu-items-mixin.html b/src/vaadin-contextmenu-items-mixin.html index 2a7ae6f6..4b7e38b4 100644 --- a/src/vaadin-contextmenu-items-mixin.html +++ b/src/vaadin-contextmenu-items-mixin.html @@ -156,6 +156,12 @@ // Store the reference to align based on parent overlay coordinates subMenu.$.overlay._setParentOverlay(parent); + // Set theme attribute from parent element + const parentTheme = parent.getAttribute('theme'); + if (parentTheme && parentTheme !== '') { + subMenu.setAttribute('theme', parentTheme); + } + let x; content.style.minWidth = ''; if (document.documentElement.clientWidth - itemRect.right > itemRect.width) { @@ -200,6 +206,7 @@ component.setAttribute('role', 'separator'); } component.classList.add('vaadin-menu-item'); + component.setAttribute('theme', this.theme); component._item = item; @@ -236,7 +243,7 @@ __initMenu(root, menu) { if (!root.firstElementChild) { root.innerHTML = ` - + `; Polymer.flush(); diff --git a/test/items.html b/test/items.html index d06b914e..7f217351 100644 --- a/test/items.html +++ b/test/items.html @@ -20,6 +20,14 @@ + + + + From 908ba478ecd0d06df3da7334a26726c96a979a96 Mon Sep 17 00:00:00 2001 From: Yuriy Yevstihnyeyev Date: Wed, 15 May 2019 13:05:47 +0300 Subject: [PATCH 2/5] Remove theme property --- src/vaadin-context-menu-overlay.html | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/vaadin-context-menu-overlay.html b/src/vaadin-context-menu-overlay.html index 8ca9f406..bbb0e1db 100644 --- a/src/vaadin-context-menu-overlay.html +++ b/src/vaadin-context-menu-overlay.html @@ -69,16 +69,14 @@ parentOverlay: { type: Object, readOnly: true - }, - - theme: { - type: String, - reflectToAttribute: true, - observer: '_themeChanged' } }; } + static get observers() { + return ['_themeChanged(theme)']; + } + ready() { super.ready(); From e9549fb85d50882b8e03cd9b538e174dedd31f9e Mon Sep 17 00:00:00 2001 From: Yuriy Yevstihnyeyev Date: Wed, 15 May 2019 14:05:34 +0300 Subject: [PATCH 3/5] Simplify logic for theme change, update tests, check theme from parent --- src/vaadin-context-menu-overlay.html | 24 +-------- src/vaadin-contextmenu-items-mixin.html | 8 +-- test/items.html | 68 +++++-------------------- 3 files changed, 19 insertions(+), 81 deletions(-) diff --git a/src/vaadin-context-menu-overlay.html b/src/vaadin-context-menu-overlay.html index bbb0e1db..309aec83 100644 --- a/src/vaadin-context-menu-overlay.html +++ b/src/vaadin-context-menu-overlay.html @@ -100,28 +100,8 @@ } _themeChanged(theme) { - const childContextMenu = this.querySelector('vaadin-context-menu'); - const childListBox = this.querySelector('vaadin-context-menu-list-box'); - - if (childListBox) { - if (theme && theme !== '') { - childListBox.setAttribute('theme', theme); - Array.from(childListBox.querySelectorAll('vaadin-context-menu-item')) - .forEach(item => item.setAttribute('theme', theme)); - } else { - childListBox.removeAttribute('theme'); - Array.from(childListBox.querySelectorAll('vaadin-context-menu-item')) - .forEach(item => item.removeAttribute('theme')); - } - } - - if (childContextMenu) { - if (theme && theme !== '') { - childContextMenu.setAttribute('theme', theme); - } else { - childContextMenu.removeAttribute('theme'); - } - } + this.close(); + this.innerHTML = ''; } getBoundaries() { diff --git a/src/vaadin-contextmenu-items-mixin.html b/src/vaadin-contextmenu-items-mixin.html index 4b7e38b4..ebe7ef07 100644 --- a/src/vaadin-contextmenu-items-mixin.html +++ b/src/vaadin-contextmenu-items-mixin.html @@ -157,9 +157,8 @@ subMenu.$.overlay._setParentOverlay(parent); // Set theme attribute from parent element - const parentTheme = parent.getAttribute('theme'); - if (parentTheme && parentTheme !== '') { - subMenu.setAttribute('theme', parentTheme); + if (parent.theme) { + subMenu.setAttribute('theme', parent.theme); } let x; @@ -243,11 +242,12 @@ __initMenu(root, menu) { if (!root.firstElementChild) { root.innerHTML = ` - + `; Polymer.flush(); const listBox = root.querySelector('vaadin-context-menu-list-box'); + this.theme && listBox.setAttribute('theme', this.theme); listBox.classList.add('vaadin-menu-list-box'); requestAnimationFrame(() => listBox.setAttribute('role', 'menu')); const subMenu = root.querySelector('vaadin-context-menu'); diff --git a/test/items.html b/test/items.html index 7f217351..444c46d4 100644 --- a/test/items.html +++ b/test/items.html @@ -521,64 +521,22 @@ }); }); - function themeAttributeEquals(theme, el, overlay = false, listBox = false, items = false) { - const elOverlay = el.$.overlay; - const elListBox = elOverlay.querySelector('vaadin-context-menu-list-box'); - const elItems = Array.from(elListBox.querySelectorAll('vaadin-context-menu-item')); - - if (overlay) { - return elOverlay.getAttribute('theme') === theme; - } else if (listBox) { - return elListBox.getAttribute('theme') === theme; - } else if (items) { - return elItems.filter(item => item.getAttribute('theme') !== theme).length === 0; - } - - return el.getAttribute('theme') === theme; - } - - ['propagate', 'remove'].forEach(condition => { - const theme = condition === 'remove' ? null : 'foo'; + it('should propagate theme attribute to the nested elements', () => { + const overlay = subMenu.$.overlay; + const listBox = overlay.querySelector('vaadin-context-menu-list-box'); + const items = Array.from(listBox.querySelectorAll('vaadin-context-menu-item')); - it(`should ${condition} theme attribute ${condition === 'remove' ? 'from' : 'to'} the nested context-menus`, () => { - if (condition === 'remove') { - rootMenu.removeAttribute('theme'); - } - - expect(themeAttributeEquals(theme, rootMenu)).to.be.true; - expect(themeAttributeEquals(theme, subMenu)).to.be.true; - expect(themeAttributeEquals(theme, subMenu2)).to.be.true; - }); - - it(`should ${condition} theme attribute ${condition === 'remove' ? 'from' : 'to'} the nested overlays`, () => { - if (condition === 'remove') { - rootMenu.removeAttribute('theme'); - } - - expect(themeAttributeEquals(theme, rootMenu, true)).to.be.true; - expect(themeAttributeEquals(theme, subMenu, true)).to.be.true; - expect(themeAttributeEquals(theme, subMenu2, true)).to.be.true; - }); + expect(overlay.getAttribute('theme')).to.equal('foo'); + expect(listBox.getAttribute('theme')).to.equal('foo'); - it(`should ${condition} theme attribute ${condition === 'remove' ? 'from' : 'to'} the nested list-boxes`, () => { - if (condition === 'remove') { - rootMenu.removeAttribute('theme'); - } - - expect(themeAttributeEquals(theme, rootMenu, false, true)).to.be.true; - expect(themeAttributeEquals(theme, subMenu, false, true)).to.be.true; - expect(themeAttributeEquals(theme, subMenu2, false, true)).to.be.true; - }); - - it(`should ${condition} theme attribute ${condition === 'remove' ? 'from' : 'to'} the items`, () => { - if (condition === 'remove') { - rootMenu.removeAttribute('theme'); - } + const itemsThemeEqualsFoo = items.filter(item => item.getAttribute('theme') !== 'foo').length === 0; + expect(itemsThemeEqualsFoo).to.be.true; + }); - expect(themeAttributeEquals(theme, rootMenu, false, false, true)).to.be.true; - expect(themeAttributeEquals(theme, subMenu, false, false, true)).to.be.true; - expect(themeAttributeEquals(theme, subMenu2, false, false, true)).to.be.true; - }); + it('should close the submenu on theme changed', () => { + rootMenu.setAttribute('theme', 'bar'); + expect(subMenu.opened).to.be.false; + expect(subMenu2.opened).to.be.false; }); }); }); From 02f9ed9190b61d4f03a92788f3c7f074bf71477c Mon Sep 17 00:00:00 2001 From: Yuriy Yevstihnyeyev Date: Wed, 15 May 2019 14:08:29 +0300 Subject: [PATCH 4/5] Test two levels of submenus --- src/vaadin-contextmenu-items-mixin.html | 4 +--- test/items.html | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/vaadin-contextmenu-items-mixin.html b/src/vaadin-contextmenu-items-mixin.html index ebe7ef07..d0660a75 100644 --- a/src/vaadin-contextmenu-items-mixin.html +++ b/src/vaadin-contextmenu-items-mixin.html @@ -157,9 +157,7 @@ subMenu.$.overlay._setParentOverlay(parent); // Set theme attribute from parent element - if (parent.theme) { - subMenu.setAttribute('theme', parent.theme); - } + parent.theme && subMenu.setAttribute('theme', parent.theme); let x; content.style.minWidth = ''; diff --git a/test/items.html b/test/items.html index 444c46d4..a880364f 100644 --- a/test/items.html +++ b/test/items.html @@ -522,19 +522,22 @@ }); it('should propagate theme attribute to the nested elements', () => { - const overlay = subMenu.$.overlay; - const listBox = overlay.querySelector('vaadin-context-menu-list-box'); - const items = Array.from(listBox.querySelectorAll('vaadin-context-menu-item')); + [rootMenu, subMenu, subMenu2].forEach(subMenu => { + const overlay = subMenu.$.overlay; + const listBox = overlay.querySelector('vaadin-context-menu-list-box'); + const items = Array.from(listBox.querySelectorAll('vaadin-context-menu-item')); - expect(overlay.getAttribute('theme')).to.equal('foo'); - expect(listBox.getAttribute('theme')).to.equal('foo'); + expect(overlay.getAttribute('theme')).to.equal('foo'); + expect(listBox.getAttribute('theme')).to.equal('foo'); - const itemsThemeEqualsFoo = items.filter(item => item.getAttribute('theme') !== 'foo').length === 0; - expect(itemsThemeEqualsFoo).to.be.true; + const itemsThemeEqualsFoo = items.filter(item => item.getAttribute('theme') !== 'foo').length === 0; + expect(itemsThemeEqualsFoo).to.be.true; + }); }); - it('should close the submenu on theme changed', () => { + it('should close the menu and submenu on theme changed', () => { rootMenu.setAttribute('theme', 'bar'); + expect(rootMenu.opened).to.be.false; expect(subMenu.opened).to.be.false; expect(subMenu2.opened).to.be.false; }); From 1ddfcadb2f8e8a3dd57becd4fa27fd4337dcbf64 Mon Sep 17 00:00:00 2001 From: Yuriy Yevstihnyeyev Date: Thu, 16 May 2019 09:46:54 +0300 Subject: [PATCH 5/5] Not cleaning the root, but updating the theme --- src/vaadin-context-menu-overlay.html | 1 - src/vaadin-contextmenu-items-mixin.html | 15 +++++++++++-- test/items.html | 28 +++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/vaadin-context-menu-overlay.html b/src/vaadin-context-menu-overlay.html index 309aec83..79cf4461 100644 --- a/src/vaadin-context-menu-overlay.html +++ b/src/vaadin-context-menu-overlay.html @@ -101,7 +101,6 @@ _themeChanged(theme) { this.close(); - this.innerHTML = ''; } getBoundaries() { diff --git a/src/vaadin-contextmenu-items-mixin.html b/src/vaadin-contextmenu-items-mixin.html index d0660a75..ad0fb65f 100644 --- a/src/vaadin-contextmenu-items-mixin.html +++ b/src/vaadin-contextmenu-items-mixin.html @@ -157,7 +157,11 @@ subMenu.$.overlay._setParentOverlay(parent); // Set theme attribute from parent element - parent.theme && subMenu.setAttribute('theme', parent.theme); + if (parent.theme) { + subMenu.setAttribute('theme', parent.theme); + } else { + subMenu.removeAttribute('theme'); + } let x; content.style.minWidth = ''; @@ -203,7 +207,7 @@ component.setAttribute('role', 'separator'); } component.classList.add('vaadin-menu-item'); - component.setAttribute('theme', this.theme); + this.theme && component.setAttribute('theme', this.theme); component._item = item; @@ -328,6 +332,13 @@ menu.$.overlay.addEventListener(openEvent, openSubMenu); menu.$.overlay.addEventListener('keydown', e => (e.keyCode === 39 || e.keyCode === 13 || e.keyCode === 32) && openSubMenu(e)); + } else { + const listBox = root.querySelector('vaadin-context-menu-list-box'); + if (this.theme) { + listBox.setAttribute('theme', this.theme); + } else { + listBox.removeAttribute('theme'); + } } } diff --git a/test/items.html b/test/items.html index a880364f..70e5bd0f 100644 --- a/test/items.html +++ b/test/items.html @@ -541,6 +541,34 @@ expect(subMenu.opened).to.be.false; expect(subMenu2.opened).to.be.false; }); + + it('should remove theme attribute from the nested elements', async() => { + rootMenu.removeAttribute('theme'); + + // Should wait until submenus will be opened again. + await new Promise(resolve => { + open(); + flush(() => { + open(menuComponents()[0]); + flush(() => { + open(menuComponents(subMenu)[1]); + flush(resolve); + }); + }); + }); + + [rootMenu, subMenu, subMenu2].forEach(subMenu => { + const overlay = subMenu.$.overlay; + const listBox = overlay.querySelector('vaadin-context-menu-list-box'); + const items = Array.from(listBox.querySelectorAll('vaadin-context-menu-item')); + + expect(overlay.hasAttribute('theme')).to.be.false; + expect(listBox.hasAttribute('theme')).to.be.false; + + const itemsDoNotHaveTheme = items.filter(item => item.hasAttribute('theme')).length === 0; + expect(itemsDoNotHaveTheme).to.be.true; + }); + }); }); });