From 01a97dfe3ca8ef5c61decb1d01042631a64903d9 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Thu, 3 Oct 2024 10:42:31 +0300 Subject: [PATCH 1/2] fix: delay parsing htmlValue when RTE is attached but not rendered (#7890) * fix: delay parsing htmlValue when RTE is attached but not rendered * refactor: simplify test setup * refactor: move attach tests to own file * refactor: remove condition not needed --- .../src/vaadin-rich-text-editor-mixin.js | 32 ++++++- .../rich-text-editor/test/attach-lit.test.js | 3 + .../test/attach-polymer.test.js | 3 + .../rich-text-editor/test/attach.common.js | 90 +++++++++++++++++++ .../rich-text-editor/test/basic.common.js | 66 -------------- 5 files changed, 124 insertions(+), 70 deletions(-) create mode 100644 packages/rich-text-editor/test/attach-lit.test.js create mode 100644 packages/rich-text-editor/test/attach-polymer.test.js create mode 100644 packages/rich-text-editor/test/attach.common.js diff --git a/packages/rich-text-editor/src/vaadin-rich-text-editor-mixin.js b/packages/rich-text-editor/src/vaadin-rich-text-editor-mixin.js index 532a976b28..76d9da1496 100644 --- a/packages/rich-text-editor/src/vaadin-rich-text-editor-mixin.js +++ b/packages/rich-text-editor/src/vaadin-rich-text-editor-mixin.js @@ -703,10 +703,26 @@ export const RichTextEditorMixin = (superClass) => */ dangerouslySetHtmlValue(htmlValue) { if (!this._editor) { - // The editor isn't ready yet, store the value for later - this.__pendingHtmlValue = htmlValue; - // Clear a possible value to prevent it from clearing the pending htmlValue once the editor property is set - this.value = ''; + this.__savePendingHtmlValue(htmlValue); + + return; + } + + // In Firefox, the styles are not properly computed when the element is placed + // in a Lit component, as the element is first attached to the DOM and then + // the shadowRoot is initialized. This causes the `hmlValue` to not be correctly + // parsed into the delta format used by Quill. To work around this, we check + // if the display property is set and if not, we wait for the element to intersect + // with the viewport before trying to set the value again. + if (!getComputedStyle(this).display) { + this.__savePendingHtmlValue(htmlValue); + const observer = new IntersectionObserver(() => { + if (getComputedStyle(this).display) { + this.__flushPendingHtmlValue(); + observer.disconnect(); + } + }); + observer.observe(this); return; } @@ -733,6 +749,14 @@ export const RichTextEditorMixin = (superClass) => this._editor.setContents(deltaFromHtml, SOURCE.API); } + /** @private */ + __savePendingHtmlValue(htmlValue) { + // The editor isn't ready yet, store the value for later + this.__pendingHtmlValue = htmlValue; + // Clear a possible value to prevent it from clearing the pending htmlValue once the editor property is set + this.value = ''; + } + /** @private */ __flushPendingHtmlValue() { if (this.__pendingHtmlValue) { diff --git a/packages/rich-text-editor/test/attach-lit.test.js b/packages/rich-text-editor/test/attach-lit.test.js new file mode 100644 index 0000000000..f1265383d6 --- /dev/null +++ b/packages/rich-text-editor/test/attach-lit.test.js @@ -0,0 +1,3 @@ +import '../theme/lumo/vaadin-rich-text-editor-styles.js'; +import '../src/vaadin-lit-rich-text-editor.js'; +import './attach.common.js'; diff --git a/packages/rich-text-editor/test/attach-polymer.test.js b/packages/rich-text-editor/test/attach-polymer.test.js new file mode 100644 index 0000000000..1cd4e8e513 --- /dev/null +++ b/packages/rich-text-editor/test/attach-polymer.test.js @@ -0,0 +1,3 @@ +import '../theme/lumo/vaadin-rich-text-editor-styles.js'; +import '../src/vaadin-rich-text-editor.js'; +import './attach.common.js'; diff --git a/packages/rich-text-editor/test/attach.common.js b/packages/rich-text-editor/test/attach.common.js new file mode 100644 index 0000000000..b39f210d82 --- /dev/null +++ b/packages/rich-text-editor/test/attach.common.js @@ -0,0 +1,90 @@ +import { expect } from '@vaadin/chai-plugins'; +import { fixtureSync, nextRender, nextUpdate } from '@vaadin/testing-helpers'; +import sinon from 'sinon'; + +describe('attach/detach', () => { + let rte, editor; + + const flushValueDebouncer = () => rte.__debounceSetValue && rte.__debounceSetValue.flush(); + + async function attach(shadow = false) { + const parent = fixtureSync('
'); + if (shadow) { + parent.attachShadow({ mode: 'open' }); + } + parent.appendChild(rte); + await nextRender(); + flushValueDebouncer(); + } + + beforeEach(async () => { + rte = fixtureSync(''); + await nextRender(); + flushValueDebouncer(); + editor = rte._editor; + }); + + describe('detach and re-attach', () => { + it('should disconnect the emitter when detached', () => { + const spy = sinon.spy(editor.emitter, 'disconnect'); + + rte.parentNode.removeChild(rte); + + expect(spy).to.be.calledOnce; + }); + + it('should re-connect the emitter when detached and re-attached', async () => { + const parent = rte.parentNode; + parent.removeChild(rte); + + const spy = sinon.spy(editor.emitter, 'connect'); + + parent.appendChild(rte); + await nextUpdate(rte); + + expect(spy).to.be.calledOnce; + }); + + it('should parse htmlValue correctly when element is attached but not rendered', async () => { + await attach(true); + rte.dangerouslySetHtmlValue('

Foo

'); + rte.parentNode.shadowRoot.innerHTML = ''; + await nextRender(); + flushValueDebouncer(); + expect(rte.htmlValue).to.equal('

Foo

'); + }); + }); + + describe('unattached rich text editor', () => { + beforeEach(() => { + rte = document.createElement('vaadin-rich-text-editor'); + }); + + it('should not throw when setting html value', () => { + expect(() => rte.dangerouslySetHtmlValue('

Foo

')).to.not.throw(Error); + }); + + it('should have the html value once attached', async () => { + rte.dangerouslySetHtmlValue('

Foo

'); + await attach(); + + expect(rte.htmlValue).to.equal('

Foo

'); + }); + + it('should override the htmlValue', async () => { + rte.dangerouslySetHtmlValue('

Foo

'); + rte.value = JSON.stringify([{ insert: 'Vaadin' }]); + await attach(); + + expect(rte.htmlValue).to.equal('

Vaadin

'); + }); + + it('should override the value', async () => { + rte.value = JSON.stringify([{ insert: 'Vaadin' }]); + rte.dangerouslySetHtmlValue('

Foo

'); + await attach(); + + expect(rte.htmlValue).to.equal('

Foo

'); + }); + }); +}); diff --git a/packages/rich-text-editor/test/basic.common.js b/packages/rich-text-editor/test/basic.common.js index 361d5de886..3a36f4752c 100644 --- a/packages/rich-text-editor/test/basic.common.js +++ b/packages/rich-text-editor/test/basic.common.js @@ -1046,70 +1046,4 @@ describe('rich text editor', () => { ); }); }); - - describe('detach and re-attach', () => { - it('should disconnect the emitter when detached', () => { - const spy = sinon.spy(editor.emitter, 'disconnect'); - - rte.parentNode.removeChild(rte); - - expect(spy).to.be.calledOnce; - }); - - it('should re-connect the emitter when detached and re-attached', async () => { - const parent = rte.parentNode; - parent.removeChild(rte); - - const spy = sinon.spy(editor.emitter, 'connect'); - - parent.appendChild(rte); - await nextUpdate(rte); - - expect(spy).to.be.calledOnce; - }); - }); -}); - -describe('unattached rich text editor', () => { - let rte; - - beforeEach(() => { - rte = document.createElement('vaadin-rich-text-editor'); - }); - - const flushValueDebouncer = () => rte.__debounceSetValue && rte.__debounceSetValue.flush(); - - async function attach() { - const parent = fixtureSync('
'); - parent.appendChild(rte); - await nextRender(); - flushValueDebouncer(); - } - - it('should not throw when setting html value', () => { - expect(() => rte.dangerouslySetHtmlValue('

Foo

')).to.not.throw(Error); - }); - - it('should have the html value once attached', async () => { - rte.dangerouslySetHtmlValue('

Foo

'); - await attach(); - - expect(rte.htmlValue).to.equal('

Foo

'); - }); - - it('should override the htmlValue', async () => { - rte.dangerouslySetHtmlValue('

Foo

'); - rte.value = JSON.stringify([{ insert: 'Vaadin' }]); - await attach(); - - expect(rte.htmlValue).to.equal('

Vaadin

'); - }); - - it('should override the value', async () => { - rte.value = JSON.stringify([{ insert: 'Vaadin' }]); - rte.dangerouslySetHtmlValue('

Foo

'); - await attach(); - - expect(rte.htmlValue).to.equal('

Foo

'); - }); }); From 9b0f014550a7f0b34e333138f9a2cee5a6eca547 Mon Sep 17 00:00:00 2001 From: Diego Cardoso Date: Thu, 3 Oct 2024 10:53:36 +0300 Subject: [PATCH 2/2] fix: use correct import --- packages/rich-text-editor/test/attach.common.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rich-text-editor/test/attach.common.js b/packages/rich-text-editor/test/attach.common.js index b39f210d82..78d2c30749 100644 --- a/packages/rich-text-editor/test/attach.common.js +++ b/packages/rich-text-editor/test/attach.common.js @@ -1,4 +1,4 @@ -import { expect } from '@vaadin/chai-plugins'; +import { expect } from '@esm-bundle/chai'; import { fixtureSync, nextRender, nextUpdate } from '@vaadin/testing-helpers'; import sinon from 'sinon';