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

chore: drop support for HTML, PDF, and Markdown converting functions #1997

Merged
merged 9 commits into from
Jul 11, 2024

Conversation

EchoEllet
Copy link
Collaborator

@EchoEllet EchoEllet commented Jul 9, 2024

Description

Drop support for the converters to convert to HTML/PDF.

Reasons

Take a look at Conversion to HTML README section, PR #1990, some discussions at related issues, PRs, discussions, and Slack. We discussed more details there.

Here is the key list of reasons:

  1. Lack of Standardization in Quill JS: Those two features are not a standard features in Quill JS, Our implementation was a workaround rather than a core part of the Quill ecosystem. We decided to focus more on the existing core code without adding more code that is not tested and doesn't adhere to high-quality standards, We’ve been receiving a high volume of reports across Issue Tracker, GitHub discussions, Slack, and private messages regarding this, we prefer to focus on the primary goal of the project, improve existing code, prioritize other things, critical bugs that make the editor unusable, performance issues, tests, inconsistency, code quality, extensibility, bugs, feature requests, architecture, docs, usability. There are other alternatives that focus on this functionality, the only use case where you might want those two features (converting Delta to HTML/PDF) is to add the Save as feature. The project focuses on the editor itself.
  2. Dependency on Third-Party Projects: The conversion features relied heavily on third-party packages, We didn't implement our own implementation designed from the ground up for the project, instead we used existing packages, slightly modifying them.
  3. Quality and Stability: The implementation is far from stable, with bugs and inconsistencies issues., We found that the experimental support did not meet the high standards we strive for in our primary package.
  4. Differences: There are structural and functional differences between HTML and Quill Delta that often result in inconsistencies and incomplete, as we keep adding more features, most of them are broken when converting to HTML and then back to Delta, requiring storing custom attributes in HTML elements, which make it incompatible with existing HTML from other systems, storing custom HTML attributes might cause some issues in case the system validate the syntax, attributes of the HTML. See Quill JS #1551 for more details.
  5. Increased Issue Reports: This made it more difficult for contributors and maintainers to concentrate on improving the primary functionality of the editor package and addressing critical bugs and feature requests. We don't have custom labels to filter the issues and this would require extra input when creating an issue, some users might not select the correct input.
  6. Limited Usefulness: We have reports from users regarding bugs and feature requests who are storing the Delta as HTML in the database, you usually don't need to store it as HTML in case you use Web, you can use Quill JS or Flutter web with flutter_quill, there are some reasons where you might want to convert Delta to HTML for sending it as an email or publish it somewhere else, or for migration from the existing system, there are already existing solutions like vsc_quill_delta_to_html or flutter_quill_delta_from_html, see Conversion to HTML section for more details, it's not recommended to store the Document as HTML or other data formats in the database, it causes other issues regarding performance other than bugs and doesn't support all features.
  7. Project Structure: Add two more directories, that will also need to be maintained in CI, docs, scripts, the example, and other places, which consume more resources when building the project for each commit, and release.

While we're grateful to the library authors, we prefer to minimize the dependencies, when a new release of Flutter comes out, this will make it harder to update the package with the new release of Flutter, requiring more around, sometimes forking the package and edit them manually, reverting the changes to use the original and maintained package, most developers will update Flutter to use them with most projects, they might have some projects that use flutter_quill, which will cause build failure and require either temporary workarounds or downloading different versions of Flutter, See Comment #2114954661 as an example.

Why breaking change?

The quill_pdf_converter and quill_html_converter are experimental packages.

Even though the version is not experimental (to allow automated publishing and centralize the version) We have marked them as experimental in the package README.md for the two packages in GitHub and Pub.dev, README.md of the repository, in CHANGELOG.md, the docs of the code of all flutter_quill, quill_html_converter and quill_pdf_converter for all functions and classes, marked them as experimental using meta, mentioned that in GitHub issues, discussions and in Slack.

We have already mentioned that the support can be dropped at anytime

This library is experimental and the support might be dropped at any time.

You can still use the old version of those packages for now. Take a look at the existing quill_html_converter and quill_pdf_converter

We haven't dropped the support for the experimental functions DeltaX.fromMarkdown and DeltaX.fromHtml in flutter_quill, which work well for the Rich Text Pasting Feature only, we do plan soon, we will deprecate them first for a while then avoid exposing them and only using them internally for this feature which also requires super_clipboard, we recommend to avoid using those APIs as soon as possible.

Suggested alternatives

There might be other alternatives for other data formats, you might want to explore them on pub.dev.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the package version in pubspec.yaml files.
  • All existing and new tests are passing.
  • I have run the commands in ./scripts/before_push.sh and it all passed successfully

Breaking Change

  • Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
  • No, this is not a breaking change.

@EchoEllet EchoEllet changed the title chore: drop support converters chore: drop support for HTML/PDF converter packages Jul 9, 2024
@EchoEllet EchoEllet marked this pull request as ready for review July 9, 2024 19:58
@EchoEllet EchoEllet marked this pull request as draft July 9, 2024 19:59
@EchoEllet EchoEllet changed the title chore: drop support for HTML/PDF converter packages chore: drop support for HTML, PDF, and Markdown converting functions Jul 9, 2024
@EchoEllet EchoEllet marked this pull request as ready for review July 10, 2024 10:07
@EchoEllet EchoEllet force-pushed the chore/drop-support-converters branch from ec174de to cc22e00 Compare July 10, 2024 10:16
@EchoEllet EchoEllet force-pushed the chore/drop-support-converters branch from f972dcf to f50f172 Compare July 10, 2024 10:42
@EchoEllet
Copy link
Collaborator Author

EchoEllet commented Jul 10, 2024

@CatHood0

Maybe we should move the changes to vsc_quill_delta_to_html if the change is suitable and works for most use cases though it has some specific features that do not conform to standard Quill Delta, such as line-height or maybe to a new package that's specifically designed for HTML conversion with Quill in general or Flutter Quill?

The line-height property is specific to CSS and is not natively supported in HTML. In the context of Flutter Quill, it appears we are attempting to convert the line-height attribute to convert HTML back to Delta, it's also not supported in standard Quill Delta. As we discussed previously in #1990, this might not be a valid use case.

We could also consider moving quill_html_converter and quill_pdf_converter to the example Application with the tests.

Unrelated bug in `flutter_quill_delta_from_html`

It seems that the package flutter_quill_delta_from_html, which is used for the rich text pasting feature, might not support nested list or sublist while using the rich text pasting feature:

v9.5.14:
image

v9.4.0:

image

The previous approach supports nested lists while causing some other issues, the new flutter_quill_delta_from_html package solved some of those issues though it converted the entries of the nested list as an entry to the list itself and mixed some of the items in the same entry.

@CatHood0
Copy link
Collaborator

CatHood0 commented Jul 10, 2024

It seems that the package flutter_quill_delta_from_html which is used for the rich text pasting feature, might not support nested list or sublist, tested while using the rich text pasting feature

Can you provide me with the text you used as an example (I mean if you used HTML or if you are directly using the list that comes from the README)?

I saw the README, that has the nested list with Markdown syntax, which the latter is not supported. I could be misunderstanding something, so if you provide me with the text you used I could test about this problem, while I take advantage and find a solution to add the HTML indent attribute to Delta

@EchoEllet
Copy link
Collaborator Author

EchoEllet commented Jul 10, 2024

Can you provide me with the text you used as an example (I mean if you used HTML or if you are directly using the list that comes from the README)?

Copying the following from README in the Table of contents section:

- [Flutter Quill](#flutter-quill)
  - [📚 Table of contents](#-table-of-contents)
  - [📸 Screenshots](#-screenshots)
  - [📦 Installation](#-installation)
  - [🛠 Platform Specific Configurations](#-platform-specific-configurations)
  - [🚀 Usage](#-usage)
  - [🔄 Migration](#-migration)
  - [🔤 Input / Output](#-input--output)
    - [🔗 Links](#-links)
  - [⚙️ Configurations](#️-configurations)
    - [🔗 Links](#-links-1)
    - [🖋 Font Family](#-font-family)
  - [📦 Embed Blocks](#-embed-blocks)
    - [🛠️ Using the embed blocks from `flutter_quill_extensions`](#️-using-the-embed-blocks-from-flutter_quill_extensions)
    - [🔗 Links](#-links-2)
  - [🔄 Conversion to HTML](#-conversion-to-html)
  - [🌐 Translation](#-translation)
  - [🧪 Testing](#-testing)
  - [👥 Contributors](#-contributors)

Which is in Markdown, and rendered in GitHub which uses HTML, calling platform-specific code
to access the Clipboard (withsuper_clipboard plugin) that will return this HTML:

<body><ul dir="auto" style="box-sizing: border-box; padding-left: 2em; margin-top: 0px; margin-bottom: var(--base-size-16); color: rgb(230, 237, 243); font-family: -apple-system, &quot;system-ui&quot;, &quot;Segoe UI&quot;, &quot;Noto Sans&quot;, Helvetica, Arial, sans-serif, &quot;Apple Color Emoji&quot;, &quot;Segoe UI Emoji&quot;; font-size: 16px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(13, 17, 23); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><li style="box-sizing: border-box;"><a href="https://github.com/singerdmx/flutter-quill#flutter-quill" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">Flutter Quill</a><ul dir="auto" style="box-sizing: border-box; padding-left: 2em; margin-top: 0px; margin-bottom: 0px;"><li style="box-sizing: border-box;"><a href="https://github.com/singerdmx/flutter-quill#-table-of-contents" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">📚 Table of contents</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-screenshots" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">📸 Screenshots</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-installation" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">📦 Installation</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-platform-specific-configurations" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🛠 Platform Specific Configurations</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-usage" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🚀 Usage</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-migration" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🔄 Migration</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-input--output" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🔤 Input / Output</a><ul dir="auto" style="box-sizing: border-box; padding-left: 2em; margin-top: 0px; margin-bottom: 0px;"><li style="box-sizing: border-box;"><a href="https://github.com/singerdmx/flutter-quill#-links" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🔗 Links</a></li></ul></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#%EF%B8%8F-configurations" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">⚙️ Configurations</a><ul dir="auto" style="box-sizing: border-box; padding-left: 2em; margin-top: 0px; margin-bottom: 0px;"><li style="box-sizing: border-box;"><a href="https://github.com/singerdmx/flutter-quill#-links-1" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🔗 Links</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-font-family" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🖋 Font Family</a></li></ul></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-embed-blocks" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">📦 Embed Blocks</a><ul dir="auto" style="box-sizing: border-box; padding-left: 2em; margin-top: 0px; margin-bottom: 0px;"><li style="box-sizing: border-box;"><a href="https://github.com/singerdmx/flutter-quill#%EF%B8%8F-using-the-embed-blocks-from-flutter_quill_extensions" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🛠️ Using the embed blocks from<span>&nbsp;</span><code style="box-sizing: border-box; font-family: var(--fontStack-monospace, ui-monospace, SFMono-Regular, SF Mono, Menlo, Consolas, Liberation Mono, monospace); font-size: 13.6px; padding: 0.2em 0.4em; margin: 0px; white-space: break-spaces; background-color: var(--bgColor-neutral-muted, var(--color-neutral-muted)); border-radius: 6px;">flutter_quill_extensions</code></a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-links-2" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🔗 Links</a></li></ul></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-conversion-to-html" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🔄 Conversion to HTML</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-translation" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🌐 Translation</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-testing" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">🧪 Testing</a></li><li style="box-sizing: border-box; margin-top: 0.25em;"><a href="https://github.com/singerdmx/flutter-quill#-contributors" style="box-sizing: border-box; background-color: transparent; color: var(--fgColor-accent, var(--color-accent-fg)); text-decoration: underline; text-underline-offset: 0.2rem;">👥 Contributors</a></li></ul></li></ul></body>

The HTML will be different depending on the platform, it can return <body></body> as the root element or the content directly though we're already parsing the required part of HTML outside of DeltaX.

@CatHood0
Copy link
Collaborator

The HTML will be different depending on the platform, it can return as the root element or the content directly although we're already parsing the required part of HTML outside of DeltaX

This in itself is not a problem, flutter_quill_delta_from_html does validations to make sure we always get the nodes we need (the ones that contain the content to be parsed to Delta).

Anyway, I'm trying a solution for this right now, because it seemed simple, but it's not so simple in practice.

Which is in Markdown, and rendered in GitHub which uses HTML, calling platform-specific code
to access the Clipboard (withsuper_clipboard plugin) that will return this HTML

Thanks for the example, I will take it into account while I resolve the issue of nested lists.

@CatHood0
Copy link
Collaborator

CatHood0 commented Jul 10, 2024

It seems that the package flutter_quill_delta_from_html which is used for the rich text pasting feature, might not support nested list or sublist, tested while using the rich text pasting feature

This already was fixed at the flutter_quill_delta_from_html version 1.3.0

@singerdmx
Copy link
Owner

Not sure why it is closed

@EchoEllet
Copy link
Collaborator Author

EchoEllet commented Jul 11, 2024

Not sure why it is closed

It has been referenced on the PR that has been merged:

Related Issues
Fix #1997 (comment)

We have discussed an issue in a comment on another issue that's not directly related.

@EchoEllet EchoEllet merged commit 0a7f503 into master Jul 11, 2024
2 checks passed
@EchoEllet EchoEllet deleted the chore/drop-support-converters branch July 11, 2024 07:35
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.

3 participants