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

Add paste options (bool plainTextOnly and a callback to view/modify a delta prior to pasting) #2350

Open
1 task done
mtallenca opened this issue Nov 5, 2024 · 20 comments · Fixed by #2338
Open
1 task done
Assignees
Labels
enhancement New feature or request moderate Issues that are important for improving functionality or user experience.

Comments

@mtallenca
Copy link
Contributor

mtallenca commented Nov 5, 2024

Is there an existing issue for this?

Use case

Problem: On iOS when pasting HTML, it is converted into a quill delta object and then performs a controller replaceText(). The conversion assumes the full functionality of the editor and keeps all converted attributes. If the QuillEditor implementation isn't using some of the Quill features, it can be inserted into the document and not be editable. i.e. fonts, colors, images, etc.

I propose adding two changes to the controller.

  1. Whenever HTML is pasted, pass the converted delta object to a callback that would allow the delta to be edited. The callback would return the edited delta object and that would be used with the replaceText controller method.

  2. Provide a bool that would convert any pasted HTML into plain text prior to calling replaceText().

Alternative solutions: Currently in the onReplaceText controller callback, I'm examining the delta, removing any unsupported attributes, and then calling a 2nd replace text with the modified delta.

Proposal

quill_controller_rich_paste.dart

final htmlText = await getHTML();
    if (htmlText != null) {
      final htmlBody = html_parser.parse(htmlText).body?.outerHtml;
      // ignore: deprecated_member_use_from_same_package
      var deltaFromClipboard = DeltaX.fromHtml(htmlBody ?? htmlText);
      
      //Add an optional callback...
      if (verifyPasteDelta != null) {
          deltaFromClipboard = verifyPasteDelta!(deltaFromClipboard);
       }
       // end of add

      _pasteUsingDelta(deltaFromClipboard);

      return true;
    }
@mtallenca mtallenca added the enhancement New feature or request label Nov 5, 2024
@EchoEllet EchoEllet added the moderate Issues that are important for improving functionality or user experience. label Nov 6, 2024
@EchoEllet
Copy link
Collaborator

EchoEllet commented Nov 6, 2024

This was originally discussed at flutter_quill_delta_from_html #12.

This issue will be addressed in #2338 (release/v11) as there are related changes in there (see migration guide for list of breaking changes).

I'm thinking about moving the QuillControllerConfig.onClipboardPaste to a separate config class related to the clipboard (QuillClipboardConfig) before addressing this issue.

What to address:

  • Encapsulating clipboard-related properties such as onClipboardPaste, onPasteDelta (not added yet, same as the suggested verifyPasteDelta), and enableRichTextPaste (not added yet) in one config class (QuillClipboardConfig).
  • Whether the QuillClipboardConfig should be in the controller's config (QuillControllerConfig) instead of the editor's config (QuillEditorConfig).
  • Based on the current state of the rich text paste, should we disable it by default and provide docs to enable it? In either case, the developer has onClipboardPaste to control the implementation.
  • Verify the functionality of the callback onClipboardPaste, see if we should change how it works, or provide some parameters, any possible issues, to avoid possible breaking changes after the release of 11.0.0.

Any suggestions?

@mtallenca
Copy link
Contributor Author

mtallenca commented Nov 6, 2024

  • Since there are multiple clipboard config options, makes sense to group them into their own config class
  • I think clipboard control belongs wherever onReplaceText lives - currently the controller
  • I'm not sure how quickly apps update to the latest quill. Whenever possible our team updates quickly. The enabling of rich text paste while not a breaking change, probably should have been classified as one. If most devs aren't already using the latest, it would make sense to disable by default - or make it required.
  • For the onClipboardPaste, I'm thinking the clipboard retrieval and processing is beyond most use cases when working with the editor. In the 1st example add a comment // Do something with the clipboardData before the return true i.e. onReplaceText. Maybe flip the order of the examples b/c the difficulty of writing a custom handler for all types of clipboard data is non trivial.
  • Maybe change onPasteDelta to onPaste(Object? deltaOrString) so any type of paste can be intercepted / modified. If this was the case, I'm guessing onClipboardPaste would not be used very often if at all

@EchoEllet
Copy link
Collaborator

EchoEllet commented Nov 7, 2024

  • In the 1st example add a comment // Do something with the clipboardData before the return true

The ClipboardData is a Flutter API that only supports plain text, in this example, it was used to check if the clipboard has plain text and then remove the plain text from the system clipboard to disable plain text pasting. Not a very useful example though.

The enabling of rich text paste while not a breaking change, probably should have been classified as one

I agree, even though I introduced it in 9.0.0 it was buggy at the time, and didn't mention this as a breaking change in CHANGELOG.md.

Maybe change onPasteDelta to onPaste(Object? deltaOrString) so any type of paste can be intercepted / modified. If this was the case

How about adding both onPasteDelta and onPastePlainText for type safety (this name suggestion is not final), it's also a bit more clear.

Object? deltaOrString

It can be an image.

I'm thinking about moving onImagePaste and onGifPaste from the editor's config to the QuillClipboardConfig since v11 is still published as pre-release (dev).

Since the clipboardPaste method is in the controller and pasteText (in QuillRawEditorState) still uses those callbacks.

@EchoEllet
Copy link
Collaborator

This might be a good feature (#2354) to add or at least provide a way to customize the copy/cut behavior.

@EchoEllet
Copy link
Collaborator

EchoEllet commented Nov 8, 2024

Whenever HTML is pasted, pass the converted delta object to a callback that would allow the delta to be edited. The callback would return the edited delta object and that would be used

Added the onDeltaPaste callback.

To try the latest pre-release version (#2338):

dependencies:
  flutter_quill: ^11.0.0-dev.6

See the migration guide.

Use the QuillClipboardConfig:

QuillController.basic(
  config: QuillControllerConfig(
    clipboardConfig: QuillClipboardConfig(
      onDeltaPaste: (delta) async {
        // TODO: Your custom implementation or return the Delta directly
        return delta;
      },
      onPlainTextPaste: (plainText) async {
        // TODO: Your custom implementation or return the plain text directly
        return plainText;
      },
     enableExternalRichPaste: false, // Currently default to true
    ),
  ),
);

EchoEllet added a commit that referenced this issue Nov 9, 2024
* fix!: remove super_clipboard from flutter_quill_extensions and move it to quill_super_clipboard (#2322)

* chore!: remove the controller from the configuration class, remove the quill toolbar and editor provider widgets, and other minor breaking changes

* chore!: remove SimpleSpellCheckerService from flutter_quill_extensions

* chore!: remove experimental support for spell checking, remove the deprecated support for YouTube in flutter_quill_extensions, and other minor breaking changes

* chore(deps): remove equatable

* chore!: remove quill shared configuration and toolbar shared configuration

* chore!: remove QuillController.setContents()

* chore!: remove QuillController.editorFocusNode

* chore: remove outdated comments

* chore: always call setState() in _markNeedsBuild() in QuillRawEditorState even if dirty is already true (revert to old behavior)

* chore: extract code from _requestKeyboard() and add docs comment for _requireEditorCurrentState

* chore!: remove classes related to editor element options, minor docs updates in editor config, rename isOnTapOutsideEnabled to onTapOutsideEnabled

* chore!: rename 'Configurations' to 'Config'

* chore!: refactor build method of the embed block interface

* chore!: remove the experimental table support from flutter_quill_extensions

* chore!: remove old deprecated properties

* chore!: rename rawItemsMap to items for font family and font size options

* chore!: remove deprecated formula embed support from flutter_quill_extensions

* chore!: remove deprecated class SuperClipboardService, rename the directory models to config in flutter_quill_extensions

* chore: add commnet in imageProviderBuilder code docs comment in flutter_quill_extensions

* docs: fix typos in migration

* docs(readme): replace deprecated flutter_quill_internal.dart with internal.dart

* docs(readme): improve README

* docs(readme): add the GitHub flutter_quill code snippet back

* docs: add more details to the migration guide

* fix(ios): use the localized strings for 'open', 'copy', and 'remove'

* chore: rename the file quill_controller_configurations.dart to quill_controller_config.dart

* chore!: avoid storing quill editor config inside Document

* chore: restore search within embed objects feature (revert removal of editor config inside the QuillController)

* chore: add @experimental to some APIs

* docs: add emojis to the migration guide, add the migration guide link in README.md

* chore: removes quill controller web files, updates QuillControllerConfig.onClipboardPaste to allow overriding the default paste handling

* docs: fix typos in the migration guide

* chore: minor change in the migration guide

* docs: update link of QuillControllerConfig.onClipboardPaste in the migration guide

* chore: fix dart analysis issues

* chore: mark QuillEditorConfig.customLeadingBlockBuilder as experimental

* chore!: avoid exporting OptionalSize

* chore: rename _restoreToolbar to _restoreToolbarAfterMagnifier in text_selection.dart

* chore: annotate QuillEditorConfig.magnifierConfiguration as experimental

* chore: minor cleanup to magnifier feature

* chore: export missing class, fix #2333

* chore: fix dart analysis

* chore(release): temp changes to publish 11.0.0 (will revert changes of this commit)

* chore: add temp dependency_overrides to fix CI failure

* chore: temp changes to publish 11.0.0-dev.1

* ci(publish): temp change to fix CI failure

* chore: restore previous publish workflow (revert), update min version of flutter_quill in test and extensions packages, remove pubspec_overrides.yaml

* chore: revert CHANGELOG.md and publish.yml changes

* chore: revert a change in #2026 (see comment https://github.com/singerdmx/flutter-quill/pull/2026/files#r1679744497)

* chore: revert change of reverting the removal of _handles check introduced by #2026

* docs(migration): clarify the removal of the QuillToolbar widget

* docs(migration): improve removal of the QuillToolbar section

* docs: add the custom toolbar page link in: removal of the QuillToolbar

* docs: add important info at the top of the migration guide

* feat(toolbar): add the base button options feature back, supports flutter_quill_extensions's buttons too.

* chore: fix analysis warnings

* docs: add more details in the migration guide in the breaking behavior with code snippets

* chore: minor change to the 'Breaking behavior' section

* chore(deps): improve dependencies constraints for compatibility

- Fix #2341
- Fix #2347

* docs(migration): explain that QuillToolbar is not a visual widget like QuillSimpleToolbar

* docs: minor changes to README.md and migration guide

* feat: clipboard paste callbacks, partial fix to #2350

* docs: update outdated link in the migration guide

* chore: rename deltaToPaste() to getDeltaToPaste()

* docs: improve CHANGELOG.md format and quality, fixing #2211

* ci: pass the GitHub token to an action

* docs: fix format of CHANGELOG.md

* ci: use cider for CHANGELOG.md format validation

* ci: add a TODO to improve CHANGELOG.md validation

* chore: remove flutter_quill_extensions from publishing

* chore: publish flutter_quill_extensions and add 'insertVideo' in quill_en.arb

* chore(release): prepare to publish 11.0.0-dev.3

* ci: increase _expectedTranslationKeysLength due to 'insertVideo'

* ci: use a GitHub action to update the release notes

* chore(release): publish flutter_quill_extensions 11.0.0-dev.3

* chore(release): prepare to publish 11.0.0-dev.4

* ci: remove the release notes file creation

* chore(example): delete the current example to recreate

* chore: recreate the example (fix #2249), minor changes to flutter_quill_extensions

* docs(readme): update the screenshots of the example app

* docs(readme): update sample page link, remove 'breaking changes' from table of contents only

* feat: add the option to disable rich text paste feature, partial fix to #2350

* chore(release): prepare to publish 11.0.0-dev.5

* chore: regenerate translations to reflect #2358

* chore: ignore deprecations

* docs: fix a minor issue in the Contributing guide

* chore(example): add file read access for macOS

* feat(l10n): localize "insert video" for Khmer language

Source: #2358 (comment)

* chore: simplify PR template

* docs: update development notes

* docs(readme): use images from GitHub repo instead of relative path to load on pub.dev
@EchoEllet EchoEllet reopened this Nov 9, 2024
@EchoEllet
Copy link
Collaborator

@mtallenca Do the parameters onDeltaPaste, onPlainTextPaste, and enableExternalRichPaste fix the issue?

What is missing is Paste as plain text (for the end-user) in the context menu which is supported by default on native Android, we can add it on all platforms or Android only, it's similar to native Android though not the same.

@mtallenca
Copy link
Contributor Author

mtallenca commented Nov 11, 2024 via email

@CatKevin
Copy link

CatKevin commented Dec 3, 2024

Thank you very much for your reply!
However, I only see string2string and delta2delta related conversions. But we may copy a string with emojis somewhere (not in quill), such as "Hello[smile]Mike![cheer][cheer][cheer]", and users can copy and paste it on any APP without exposing the emoji protocol in this APP. After we paste this string in quill, we actually hope to parse the strings "[smile]" and "[cheer]" into corresponding emojis and display them in quill.
Therefore, I think we may also need a method to convert strings to Delta. Do you think so?

@EchoEllet
Copy link
Collaborator

You can use onClipboardPaste for custom implementation, though you're right, I don't see a strong reason why onPlainTextPaste shouldn't expect Delta? instead of String?.

  /// Callback triggered when pasting plain text into the editor.
  ///
-  /// Returns modified text to override the pasted content, or `null` to use the default.
+  /// Returns a [Delta] to override the pasted content, or `null` to use the default.
  @experimental
-  final Future<String?> Function(String plainText)? onPlainTextPaste;
+  final Future<Delta?> Function(String plainText)? onPlainTextPaste;

The API design needs more consideration.

@CatKevin

This comment was marked as resolved.

@EchoEllet
Copy link
Collaborator

The Delta data structure is a common data structure in flutter_quill. No matter what kind of data it is,

I do agree with that as stated in my previous comment.

The onClipboardPaste function will change the data in the user's clipboard

The behavior slightly changed in v11 (see breaking changes), it allows to override the default with a custom implementation, it can be used to check if the clipboard as plain text, process the paste and then return true (see example code). I do plan on changing it to return Delta instead.

@CatKevin

This comment was marked as resolved.

@EchoEllet

This comment was marked as off-topic.

@xalanq

This comment was marked as resolved.

@EchoEllet
Copy link
Collaborator

The issue has already been resolved in the pre-release. See this comment.

The v11 is already stable. We didn't publish it as stable since we have one breaking change, which is removing the magnifier feature (see #2413 for more details).

@xalanq
Copy link

xalanq commented Dec 23, 2024

Thank you for your reply. But I meant the following one

-  final Future<String?> Function(String plainText)? onPlainTextPaste;
+  final Future<Delta?> Function(String plainText)? onPlainTextPaste;

@EchoEllet
Copy link
Collaborator

Function(String plainText)? onPlainTextPaste;

We might need to change plainText to Delta too since the QuillController has an internal clipboard (not the same as the system clipboard) that contains the copied content as rich text. If we always provide plain text, the text attributes might be lost if the user copies rich text inside the editor and then pastes it.

  • Changing it to Delta makes it no longer plain text, maybe onTextPaste would be a better name, or perhaps a second callback?
  • Currently the onDeltaPaste is used only for the external rich text paste, however using it for the internal rich text paste is also valid.

Note

  • The internal rich text paste occurs when the user copies and pastes content inside the editor from the internal clipboard.
  • The external rich text paste occurs when content is pasted from other apps while preserving styles using the system clipboard.

Here is an example after introducing the change:

onPlainTextPaste: (String plainText) async {
          return Delta()
            ..insert('The plain text from the clipboard', {'bold': true})
            ..insert(':\n')
            ..insert(plainText);
        },
IssueAfterUnmergedChange.mov

@xalanq
Copy link

xalanq commented Dec 24, 2024

Hi, thx for your reply! But the latest code is still the old version

final Future<String?> Function(String plainText)? onPlainTextPaste;

Is that change still in your local repo? When will it be merged into the master? THX!

@mtallenca
Copy link
Contributor Author

I'm only interested in altering pasted text when it's coming from the system clipboard (so I can ensure it's only using attributes that the current editor config has). I would like anything copied from the editor then pasted back into the editor to remain unmodified. Need a bool to indicate system clipboard?

@EchoEllet
Copy link
Collaborator

EchoEllet commented Dec 24, 2024

Is that change still in your local repo? When will it be merged into the master?

Yes, it needs more consideration before merging.

I'm only interested in altering pasted text

You're right. That is a separate topic and should be in #2156.

I would like anything copied from the editor then pasted back into the editor to remain unmodified.

Currently, the onDeltaPaste is only used when pasting a Delta from the system clipboard, but that's subject to change.
The API is not final.

The onDeltaPaste is generic and might not be very clear.

I'm considering onExternalRichTextPaste and onInternalRichTextPaste to separate them. The external paste is when it's coming from the system clipboard.

Need a bool to indicate system clipboard?

Having onRichTextPaste with a boolean is also valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request moderate Issues that are important for improving functionality or user experience.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants