Skip to content

refactor!: update to Quill v2, drop outdated Firefox patch #9007

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

web-padawan
Copy link
Member

@web-padawan web-padawan commented Apr 25, 2025

Description

Fixes #1680

WIP, all tests passed locally but this still needs some manual testing especially for htmlValue logic.

Type of change

  • Breaking change

@TatuLund
Copy link
Contributor

TatuLund commented Apr 25, 2025

Updated some tests where htmlValue contains   instead of regular whitespace (is this a bug?)

@web-padawan Not a bug, but by design how some Quill internal tags were removed to clean the html value: #6434 So I think change is valid as Quill has now method to get html value instead of our own hack.

@web-padawan web-padawan force-pushed the proto/quill-v2 branch 3 times, most recently from a4ebd07 to d025045 Compare April 25, 2025 13:44
@web-padawan
Copy link
Member Author

web-padawan commented Apr 25, 2025

Not a bug, but by design how some Quill internal tags were removed to clean the html value: #6434

I'm talking about a different logic added in #6651 and #6652 as a fix for vaadin/flow-components#5533.
So the following test now fails as getSemanticHTML() returns ' ' for preserved whitespaces.

it('should not lose extra space characters from the resulting htmlValue', () => {
const htmlWithExtraSpaces = '<p>Extra spaces</p>';
rte.dangerouslySetHtmlValue(htmlWithExtraSpaces);
flushValueDebouncer();
expect(rte.htmlValue).to.equal(htmlWithExtraSpaces);
});

UPD: this was introduced in slab/quill#4502 which landed in v2.0.3 patch release and a number of users were upset and had to pin to v2.0.2 - see discussion at slab/quill#4502 (comment) and this issue: slab/quill#4509

@web-padawan web-padawan force-pushed the proto/quill-v2 branch 4 times, most recently from 955c282 to a93b758 Compare April 26, 2025 07:30
@web-padawan
Copy link
Member Author

Updated the Quill fork to apply the workaround from slab/quill#4509 (comment), see vaadin/quill@76a4568.
This replaces all but one consecutive space character with &nbsp;, which should be less intrusive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider updating to Quill 2.0 once stable is released
2 participants