From 1386314ce03fe883840bb95c79d11ceca32d1e84 Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Mon, 17 Oct 2022 17:00:28 +0300 Subject: [PATCH] refactor!: remove template support from overlay (#4753) --- .../field-highlighter/src/vaadin-user-tags.js | 2 +- packages/overlay/package.json | 1 + packages/overlay/src/vaadin-overlay.d.ts | 47 +--- packages/overlay/src/vaadin-overlay.js | 207 +--------------- packages/overlay/test/animations.test.js | 3 +- packages/overlay/test/focus-trap.test.js | 55 ++--- packages/overlay/test/legacy.test.js | 2 +- packages/overlay/test/multiple.test.js | 9 +- packages/overlay/test/overlay.test.js | 35 +-- .../test/position-mixin-listeners.test.js | 1 + packages/overlay/test/position-mixin.test.js | 1 + packages/overlay/test/renderer.test.js | 66 ------ packages/overlay/test/restore-focus.test.js | 2 +- packages/overlay/test/styling.test.js | 162 ------------- packages/overlay/test/template.test.js | 221 +++--------------- .../overlay/test/visual/lumo/overlay.test.js | 14 +- .../test/visual/material/overlay.test.js | 14 +- 17 files changed, 107 insertions(+), 735 deletions(-) delete mode 100644 packages/overlay/test/styling.test.js diff --git a/packages/field-highlighter/src/vaadin-user-tags.js b/packages/field-highlighter/src/vaadin-user-tags.js index 2e0a9725074..c4db3b01324 100644 --- a/packages/field-highlighter/src/vaadin-user-tags.js +++ b/packages/field-highlighter/src/vaadin-user-tags.js @@ -209,7 +209,7 @@ export class UserTags extends PolymerElement { } get wrapper() { - return this.$.overlay.content.querySelector('[part="tags"]'); + return this.$.overlay.querySelector('[part="tags"]'); } createUserTag(user) { diff --git a/packages/overlay/package.json b/packages/overlay/package.json index 8e8165e91bc..d7407835e49 100644 --- a/packages/overlay/package.json +++ b/packages/overlay/package.json @@ -44,6 +44,7 @@ "@esm-bundle/chai": "^4.3.4", "@polymer/iron-overlay-behavior": "^3.0.0", "@vaadin/button": "24.0.0-alpha0", + "@vaadin/polymer-legacy-adapter": "24.0.0-alpha0", "@vaadin/radio-group": "24.0.0-alpha0", "@vaadin/testing-helpers": "^0.3.2", "@vaadin/text-field": "24.0.0-alpha0", diff --git a/packages/overlay/src/vaadin-overlay.d.ts b/packages/overlay/src/vaadin-overlay.d.ts index 8fb8ce1cb76..77b21d033c7 100644 --- a/packages/overlay/src/vaadin-overlay.d.ts +++ b/packages/overlay/src/vaadin-overlay.d.ts @@ -55,13 +55,10 @@ export type OverlayEventMap = HTMLElementEventMap & OverlayCustomEventMap; /** * `` is a Web Component for creating overlays. The content of the overlay - * can be populated in two ways: imperatively by using renderer callback function and - * declaratively by using Polymer's Templates. + * can be populated imperatively by using `renderer` callback function. * * ### Rendering * - * By default, the overlay uses the content provided by using the renderer callback function. - * * The renderer function provides `root`, `owner`, `model` arguments when applicable. * Generate DOM content by using `model` object properties if needed, append it to the `root` * element and control the state of the host element by accessing `owner`. Before generating new @@ -82,39 +79,9 @@ export type OverlayEventMap = HTMLElementEventMap & OverlayCustomEventMap; * in the next renderer call and will be provided with the `root` argument. * On first call it will be empty. * - * **NOTE:** when the renderer property is defined, the ` `); - overlay._observer.flush(); overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); }); @@ -109,7 +109,6 @@ describe('vaadin-overlay', () => { `); - overlay._observer.flush(); backdrop = overlay.$.backdrop; overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); @@ -151,7 +150,6 @@ describe('vaadin-overlay', () => { `); - overlay._observer.flush(); overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); }); @@ -201,7 +199,7 @@ describe('vaadin-overlay', () => {
@@ -209,7 +207,6 @@ describe('vaadin-overlay', () => { overlay = parent.children[0]; overlayPart = overlay.$.overlay; backdrop = overlay.$.backdrop; - overlay._observer.flush(); overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); }); @@ -408,12 +405,11 @@ describe('vaadin-overlay', () => { overlay = fixtureSync(` `); overlayPart = overlay.$.overlay; - overlay._observer.flush(); overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); }); @@ -431,7 +427,7 @@ describe('vaadin-overlay', () => { }); it('should fit in viewport when huge content is used', () => { - const lastChild = overlay.content.lastElementChild; + const lastChild = overlay.lastElementChild; lastChild.setAttribute('style', 'display: block; width: 2000px; height: 2000px;'); const rect = overlay.getBoundingClientRect(); @@ -453,7 +449,7 @@ describe('vaadin-overlay', () => { it('should center overlayPart in overlay with flex by default', () => { // The “default” fixture content is too large to test this - overlay.content.textContent = 'foo'; + overlay.textContent = 'foo'; const overlayRect = overlay.getBoundingClientRect(); const overlayPartRect = overlayPart.getBoundingClientRect(); @@ -477,20 +473,14 @@ describe('vaadin-overlay', () => { }); describe('wrapped overlay', () => { - let overlay, parent; + let overlay; beforeEach(async () => { - parent = fixtureSync(` -
- - - -
- `); - overlay = parent.children[0]; - overlay._observer.flush(); + overlay = fixtureSync(''); + overlay.renderer = (root) => { + const el = document.createElement('overlay-wrapper'); + root.appendChild(el); + }; overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); }); @@ -500,7 +490,7 @@ describe('vaadin-overlay', () => { }); it('should not exit modal state when opened changes from undefined to false', () => { - const wrapper = overlay.content.querySelector('overlay-wrapper'); + const wrapper = overlay.querySelector('overlay-wrapper'); const inner = wrapper.shadowRoot.querySelector('vaadin-overlay'); const spy = sinon.spy(inner, '_exitModalState'); wrapper.opened = false; @@ -520,7 +510,6 @@ describe('vaadin-overlay', () => { `); - overlay._observer.flush(); overlay.opened = true; await oneEvent(overlay, 'vaadin-overlay-open'); }); diff --git a/packages/overlay/test/position-mixin-listeners.test.js b/packages/overlay/test/position-mixin-listeners.test.js index 02f40fd5f78..4204ca937db 100644 --- a/packages/overlay/test/position-mixin-listeners.test.js +++ b/packages/overlay/test/position-mixin-listeners.test.js @@ -1,6 +1,7 @@ import { expect } from '@esm-bundle/chai'; import { aTimeout, fire, fixtureSync, nextFrame } from '@vaadin/testing-helpers'; import sinon from 'sinon'; +import '@vaadin/polymer-legacy-adapter/template-renderer.js'; import { Overlay } from '../src/vaadin-overlay.js'; import { PositionMixin } from '../src/vaadin-overlay-position-mixin.js'; diff --git a/packages/overlay/test/position-mixin.test.js b/packages/overlay/test/position-mixin.test.js index 6c22e109fc9..fb1a846b088 100644 --- a/packages/overlay/test/position-mixin.test.js +++ b/packages/overlay/test/position-mixin.test.js @@ -1,6 +1,7 @@ import { expect } from '@esm-bundle/chai'; import { fixtureSync, oneEvent } from '@vaadin/testing-helpers'; import { setViewport } from '@web/test-runner-commands'; +import '@vaadin/polymer-legacy-adapter/template-renderer.js'; import { css } from 'lit'; import { registerStyles } from '@vaadin/vaadin-themable-mixin/register-styles'; import { Overlay } from '../src/vaadin-overlay.js'; diff --git a/packages/overlay/test/renderer.test.js b/packages/overlay/test/renderer.test.js index c439c08aa5e..84b6dca10db 100644 --- a/packages/overlay/test/renderer.test.js +++ b/packages/overlay/test/renderer.test.js @@ -67,15 +67,6 @@ describe('renderer', () => { }; }); - it('should not call renderer on template change', () => { - const spy = sinon.spy(); - overlay.opened = true; - overlay.renderer = () => spy(); - spy.resetHistory(); - overlay.template = null; - expect(spy.called).to.be.false; - }); - it('should call renderer on model change', () => { const spy = sinon.spy(); overlay.opened = true; @@ -94,15 +85,6 @@ describe('renderer', () => { expect(spy.calledOnce).to.be.true; }); - it('should remove template when added after renderer', () => { - overlay.renderer = () => {}; - const template = document.createElement('template'); - expect(() => { - overlay.template = template; - }).to.throw(Error); - expect(overlay.template).to.be.not.ok; - }); - it('should run renderers when requesting content update', () => { overlay.renderer = sinon.spy(); overlay.requestContentUpdate(); @@ -129,52 +111,4 @@ describe('renderer', () => { expect(overlay.textContent.trim()).to.equal(''); }); }); - - describe('with template', () => { - let overlay; - - beforeEach(() => { - overlay = fixtureSync(` - - - - `); - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should fallback to render content with Templatizer when renderer is not defined', () => { - expect(overlay.textContent.trim()).to.equal('overlay-content'); - }); - - it('should throw an error when setting a renderer if there is already a template', () => { - expect(() => { - overlay.renderer = () => {}; - }).to.throw(Error); - }); - - it('should not restamp the template on model change', () => { - const lastInstance = overlay._instance; - expect(lastInstance).to.be.ok; - overlay.model = {}; - expect(overlay._instance).to.equal(lastInstance); - }); - - it('should not restamp the template on owner change', () => { - const lastInstance = overlay._instance; - overlay.owner = {}; - expect(overlay._instance).to.equal(lastInstance); - }); - - it('should remove renderer when added after template', () => { - expect(() => { - overlay.renderer = () => {}; - }).to.throw(Error); - expect(overlay.renderer).to.be.not.ok; - }); - }); }); diff --git a/packages/overlay/test/restore-focus.test.js b/packages/overlay/test/restore-focus.test.js index 3be7218efaf..c1a7fa16506 100644 --- a/packages/overlay/test/restore-focus.test.js +++ b/packages/overlay/test/restore-focus.test.js @@ -1,5 +1,6 @@ import { expect } from '@esm-bundle/chai'; import { fixtureSync } from '@vaadin/testing-helpers'; +import '@vaadin/polymer-legacy-adapter/template-renderer.js'; import '@vaadin/text-field/vaadin-text-field.js'; import '../src/vaadin-overlay.js'; import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; @@ -37,7 +38,6 @@ describe('restore focus', () => { wrapper = fixtureSync(''); overlay = wrapper.$.overlay; focusable = wrapper.$.focusable; - overlay._observer.flush(); window.focus(); }); diff --git a/packages/overlay/test/styling.test.js b/packages/overlay/test/styling.test.js deleted file mode 100644 index 01f55649481..00000000000 --- a/packages/overlay/test/styling.test.js +++ /dev/null @@ -1,162 +0,0 @@ -import { expect } from '@esm-bundle/chai'; -import { fixtureSync, oneEvent } from '@vaadin/testing-helpers'; -import '../vaadin-overlay.js'; -import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; -import { ThemableMixin } from '@vaadin/vaadin-themable-mixin'; -import { css, registerStyles } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; - -registerStyles( - 'overlay-local-styles', - css` - .themed-styles-content { - font-family: '.themed-styles-content'; - } - `, -); - -customElements.define( - 'overlay-local-styles', - class extends ThemableMixin(PolymerElement) { - static get is() { - return 'overlay-local-styles'; - } - - static get template() { - return html` - - - - - - `; - } - - static get properties() { - return { - opened: Boolean, - }; - } - }, -); - -function getComputedStyleSource(element) { - return getComputedStyle(element) - .getPropertyValue('font-family') - .trim() - .replace(/^([''"])(.*)\1/, '$2'); -} - -describe('overlay content styling', () => { - let overlay, globalStyle; - - before(() => { - globalStyle = document.createElement('style'); - globalStyle.innerHTML = ` - .global-styles-content { - font-family: ".global-styles-content"; - } - `; - document.head.appendChild(globalStyle); - }); - - describe('local styles', () => { - beforeEach(async () => { - const localStyles = fixtureSync(''); - overlay = localStyles.$.overlay; - overlay.opened = true; - await oneEvent(overlay, 'vaadin-overlay-open'); - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should stamp content into a shadowRoot', () => { - expect(overlay.content).to.be.instanceof(ShadowRoot); - }); - - it('should not slot the content', () => { - expect(overlay.content.children[0].assignedSlot).to.be.null; - expect(overlay.content.children[1].assignedSlot).to.be.null; - }); - - it('should apply local styles to overlay content', () => { - const localNode = overlay.content.querySelector('.local-styles-content'); - expect(getComputedStyleSource(localNode)).to.equal('.local-styles-content'); - }); - - it('should apply styles from themable mixin to overlay content', () => { - const themedNode = overlay.content.querySelector('.themed-styles-content'); - expect(getComputedStyleSource(themedNode)).to.equal('.themed-styles-content'); - }); - - it('should not apply global styles to overlay content', () => { - const globalNode = overlay.content.querySelector('.global-styles-content'); - expect(getComputedStyleSource(globalNode)).to.not.equal('.global-styles-content'); - }); - - it('should not apply host styles to overlay content', () => { - expect(getComputedStyleSource(overlay.content.host)).to.not.equal(':host'); - }); - - it('should not apply local part styles to overlay', () => { - const overlayPart = overlay.shadowRoot.querySelector('[part="overlay"]'); - expect(getComputedStyleSource(overlayPart)).to.not.equal('[part="overlay"]'); - }); - }); - - describe('global styles', () => { - beforeEach(async () => { - overlay = fixtureSync(` - - - - `); - overlay.opened = true; - await oneEvent(overlay, 'vaadin-overlay-open'); - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should stamp content into overlay host', () => { - expect(overlay.content).to.equal(overlay); - }); - - it('should slot content into content part', () => { - const slot = overlay.$.content.firstElementChild; - expect(overlay.content.children[0].assignedSlot).to.equal(slot); - expect(overlay.content.children[1].assignedSlot).to.equal(slot); - }); - - it('should apply global styles to overlay content', () => { - const globalNode = overlay.content.querySelector('.global-styles-content'); - expect(getComputedStyleSource(globalNode)).to.equal('.global-styles-content'); - }); - - it('should not apply local styles to overlay content', () => { - const localNode = overlay.content.querySelector('.local-styles-content'); - expect(getComputedStyleSource(localNode)).to.not.equal('.local-styles-content'); - }); - }); -}); diff --git a/packages/overlay/test/template.test.js b/packages/overlay/test/template.test.js index a9dac48b635..47d851633d6 100644 --- a/packages/overlay/test/template.test.js +++ b/packages/overlay/test/template.test.js @@ -1,21 +1,9 @@ import { expect } from '@esm-bundle/chai'; import { fixtureSync } from '@vaadin/testing-helpers'; +import '@vaadin/polymer-legacy-adapter/template-renderer.js'; import '../vaadin-overlay.js'; import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; -customElements.define( - 'overlay-opened-wrapper', - class extends PolymerElement { - static get template() { - return html` - - - - `; - } - }, -); - customElements.define( 'overlay-props-wrapper', class extends PolymerElement { @@ -45,28 +33,8 @@ customElements.define( }, ); -function createTemplate(html) { - const template = document.createElement('template'); - template.innerHTML = html; - return template; -} - describe('overlay template', () => { let overlay; - let externalTemplate; - - describe('template with initially opened overlay', () => { - let wrapper, overlay; - - beforeEach(() => { - wrapper = fixtureSync(''); - overlay = wrapper.$.overlay; - }); - - it('should use content part shadowRoot for contents when initially opened in component scope', () => { - expect(overlay.content).to.be.instanceOf(ShadowRoot); - }); - }); describe('in component scope', () => { let wrapper; @@ -74,97 +42,33 @@ describe('overlay template', () => { beforeEach(() => { wrapper = fixtureSync(''); overlay = wrapper.$.overlay; - externalTemplate = createTemplate('external template content'); + overlay.opened = true; }); - describe('child template', () => { - beforeEach(() => { - overlay.opened = true; - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should use child template by default', () => { - const childTemplate = overlay.querySelector('template'); - expect(childTemplate).to.be.ok; - expect(overlay.template).to.equal(childTemplate); - }); - - it('should use content part shadowRoot for contents', () => { - expect(overlay.content).to.equal(overlay.$.content.shadowRoot); - }); - - it('should stamp contents', () => { - expect(overlay.content).to.be.ok; - expect(overlay.content.textContent).to.contain('my-overlay-view content'); - }); - - it('should forwardHostProp for props', () => { - expect(overlay.content.textContent).to.contain('hostProp: foo'); - wrapper.hostProp = 'bar'; - expect(overlay.content.textContent).to.contain('hostProp: bar'); - }); - - it('should forwardHostProp for paths', () => { - expect(overlay.content.textContent).to.contain('hostPath.subPath: foo'); - wrapper.set('hostPath.subPath', 'bar'); - expect(overlay.content.textContent).to.contain('hostPath.subPath: bar'); - }); + afterEach(() => { + overlay.opened = false; }); - describe('external template disconnected', () => { - beforeEach(() => { - overlay.template = externalTemplate; - overlay.opened = true; - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should use content part shadowRoot for contents', () => { - expect(overlay.content).to.equal(overlay.$.content.shadowRoot); - }); - - it('should stamp contents', () => { - expect(overlay.content).to.be.ok; - expect(overlay.content.textContent).to.contain('external template content'); - }); - - it('should not use default child template', () => { - expect(overlay.content.textContent).to.not.contain('my-overlay-view content'); - }); + it('should use overlay host for contents, slot in content part', () => { + const contentSlot = overlay.$.content.querySelector('slot'); + expect(contentSlot).to.be.ok; + expect(overlay.lastChild.assignedSlot).to.equal(contentSlot); }); - describe('external template connected in document scope', () => { - beforeEach(() => { - document.body.appendChild(externalTemplate); - overlay.template = externalTemplate; - overlay.opened = true; - }); - - afterEach(() => { - overlay.opened = false; - document.body.removeChild(externalTemplate); - }); - - it('should use overlay host for contents, slot in content part', () => { - expect(overlay.content).to.equal(overlay); - const contentSlot = overlay.$.content.querySelector('slot'); - expect(contentSlot).to.be.ok; - expect(overlay.lastChild.assignedSlot).to.equal(contentSlot); - }); + it('should stamp contents', () => { + expect(overlay.textContent).to.contain('my-overlay-view content'); + }); - it('should stamp contents', () => { - expect(overlay.content).to.be.ok; - expect(overlay.content.textContent).to.contain('external template content'); - }); + it('should forwardHostProp for props', () => { + expect(overlay.textContent).to.contain('hostProp: foo'); + wrapper.hostProp = 'bar'; + expect(overlay.textContent).to.contain('hostProp: bar'); + }); - it('should not use default child template', () => { - expect(overlay.content.textContent).to.not.contain('my-overlay-view content'); - }); + it('should forwardHostProp for paths', () => { + expect(overlay.textContent).to.contain('hostPath.subPath: foo'); + wrapper.set('hostPath.subPath', 'bar'); + expect(overlay.textContent).to.contain('hostPath.subPath: bar'); }); }); @@ -175,87 +79,24 @@ describe('overlay template', () => { `); - externalTemplate = createTemplate('external template content'); }); - describe('child template', () => { - beforeEach(() => { - overlay.opened = true; - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should use child template by default', () => { - const childTemplate = overlay.querySelector('template'); - expect(childTemplate).to.be.ok; - expect(overlay.template).to.equal(childTemplate); - }); - - it('should use overlay host for contents, slot in content part', () => { - expect(overlay.content).to.equal(overlay); - const contentSlot = overlay.$.content.querySelector('slot'); - expect(contentSlot).to.be.ok; - expect(overlay.lastChild.assignedSlot).to.equal(contentSlot); - }); - - it('should stamp contents', () => { - expect(overlay.content).to.be.ok; - expect(overlay.content.textContent).to.contain('plain overlay content'); - }); + beforeEach(() => { + overlay.opened = true; }); - describe('external template disconnected', () => { - beforeEach(() => { - overlay.template = externalTemplate; - overlay.opened = true; - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should use content part shadowRoot for contents', () => { - expect(overlay.content).to.equal(overlay.$.content.shadowRoot); - }); - - it('should stamp contents', () => { - expect(overlay.content).to.be.ok; - expect(overlay.content.textContent).to.contain('external template content'); - }); - - it('should not use default child template', () => { - expect(overlay.content.textContent).to.not.contain('plain overlay content'); - }); + afterEach(() => { + overlay.opened = false; }); - describe('external template connected in component scope', () => { - let wrapper; - - beforeEach(() => { - wrapper = fixtureSync(''); - wrapper.shadowRoot.appendChild(externalTemplate); - overlay.template = externalTemplate; - overlay.opened = true; - }); - - afterEach(() => { - overlay.opened = false; - }); - - it('should use content part shadowRoot for contents', () => { - expect(overlay.content).to.equal(overlay.$.content.shadowRoot); - }); - - it('should stamp contents', () => { - expect(overlay.content).to.be.ok; - expect(overlay.content.textContent).to.contain('external template content'); - }); + it('should use overlay host for contents, slot in content part', () => { + const contentSlot = overlay.$.content.querySelector('slot'); + expect(contentSlot).to.be.ok; + expect(overlay.lastChild.assignedSlot).to.equal(contentSlot); + }); - it('should not use default child template', () => { - expect(overlay.content.textContent).to.not.contain('plain overlay content'); - }); + it('should stamp contents', () => { + expect(overlay.textContent).to.contain('plain overlay content'); }); }); }); diff --git a/packages/overlay/test/visual/lumo/overlay.test.js b/packages/overlay/test/visual/lumo/overlay.test.js index 2f0bd4f0317..a9862a14de7 100644 --- a/packages/overlay/test/visual/lumo/overlay.test.js +++ b/packages/overlay/test/visual/lumo/overlay.test.js @@ -9,16 +9,10 @@ describe('overlay', () => { div = document.createElement('div'); div.style.height = '100%'; - element = fixtureSync( - ` - - - - `, - div, - ); + element = fixtureSync('', div); + element.renderer = (root) => { + root.textContent = 'Simple overlay with text'; + }; }); it('basic', async () => { diff --git a/packages/overlay/test/visual/material/overlay.test.js b/packages/overlay/test/visual/material/overlay.test.js index e1565ed31cf..f303b74b4a2 100644 --- a/packages/overlay/test/visual/material/overlay.test.js +++ b/packages/overlay/test/visual/material/overlay.test.js @@ -9,16 +9,10 @@ describe('overlay', () => { div = document.createElement('div'); div.style.height = '100%'; - element = fixtureSync( - ` - - - - `, - div, - ); + element = fixtureSync('', div); + element.renderer = (root) => { + root.textContent = 'Simple overlay with text'; + }; }); it('basic', async () => {