-
Notifications
You must be signed in to change notification settings - Fork 835
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
feat: Use quill_native_bridge as default impl in DefaultClipboardService, fix related bugs in the extensions package #2230
Conversation
…dHTML() method channel for Android and iOS, example and Android platform directory
… the example (initial impl)
9ceb38d
to
e3cc4c5
Compare
e3cc4c5
to
e6ff099
Compare
…eds further fixes)
… example, set supported platforms in pubspec.yaml, configure quill_native_bridge example
…ipboard on Android for quill_native_bridge
…mon issues in the copy image button
ClipboardService
for Android, macOS, and iOS
7eb67dc
to
9743d47
Compare
ClipboardService
for Android, macOS, and iOSClipboardService
for Android, macOS, iOS and web
…rvice, slightly update Android, disable copyImageToClipboard() for web (temporarily). WIP
…sions.useSuperClipboardPlugin()
…vice.copyImage() (non-breaking change)
This PR took longer than expected due to adding a feature to customize clipboard behavior, allowing users to disable the rich clipboard feature (see this comment). This feature was unrelated to this PR and was a pre-existing issue. I will finalize this PR for merging and move the related issues to the FlutterQuill/quill-native-bridge repo. @CatHood0 @AtlasAutocode If you have time, can you review this PR? I have moved quill_native_bridge to FlutterQuill/quill-native-bridge to minimize changes in this repo.. |
ClipboardService
for Android, Linux, macOS, iOS, Windows and Web
I usually prefer more code review though we need to prioritize other issues. |
…_extensions, remove pubspec_overrides, part of #2230
Our current design is inconsistent. There is also a bug (unrelated to this PR), The solution is to either:
In either case, we will need to refactor some things in version @AtlasAutocode When you have the time, can you explain the design and why |
pasteText is in the editor and should only call the controller's clipboardPaste function. I see no reason for pasteText to call clipboard image functions directly. Presumably this is historical code left so as not to introduce a breaking change. |
Thanks for the reply. The image paste was supported initially with |
I'm still not sure how will we handle this in This feature can still be implemented without having to provide the |
…ice, fix related bugs in the extensions package (singerdmx#2230) * feat(quill_native_bridge): add initial implementation for getClipboardHTML() method channel for Android and iOS, example and Android platform directory * chore(example): use quill_native_bridge from path in dependency_overrides * feat: use quill_native_bridge for HTML Clipboard in flutter_quill and the example (initial impl) * feat(quill_native_bridge): extend the support for macOS. * chore(example): clear todos in app module build.gradle * chore(android): additional check before accessing the clip data item * fix(flutter_quill_extensions): unrelated bug for copying an image (needs further fixes) * feat: add copyImageToClipboard() method channel, an asset file in the example, and set supported platforms in pubspec.yaml, configure quill_native_bridge example * docs(readme): add an optional step to configure copying images to the clipboard on Android for quill_native_bridge * feat: use copyImageToClipboard() in flutter_quill_extensions, fix common issues in the copy image button * chore: correct name of _loadImageBytesFromImageProvider() * feat: add web support for copyImageToClipboard() * feat: (WIP) retrieving image files from the clipboard, simplifying ClipboardService, slightly updating Android, disabling copyImageToClipboard() for web (temporarily). WIP * chore(example): rename _flutterQuillAssetImage to _kFlutterQuillAssetImage * chore(example): update error messages in quill_native_bridge example * feat: add support for gif, fix unhandled exception on Android only when getting an image from the clipboard before closing the app on a new app start, clean up the platform support check * chore(example): configure the Android example for copying images to the system clipboard, update README.md * feat: support for web (WIP, will be updated soon) * chore: annotate ClipboardService and related classes as experimental * chore: temporarily add dependency_overrides to fix build failure - revert this change later * feat: separate web implementation from quill_native_bridge into quill_native_bridge_web * chore: update description of quill_native_bridge_web * chore(web): avoid using jsify() when possible * chore: publish and use quill_native_bridge_web in quill_native_bridge, update to the newly published quill_native_bridge in flutter_quill * docs(readme): use pub.dev link of quill_native_bridge in quill_native_bridge_web * chore: use .toJS in clipboardItem map (minor change) * fix: use hosted quill_native_bridge in dependencies and override with the local path in dependency_overrides * fix: update minimum required version of Flutter/Dart by quill_native_bridge * test: write basic integration tests for getClipboardImage() and copyImageToClipboard() * feat: add copyHTMLToClipboard(), add integration tests for copyHTMLToClipboard() and getClipboardHTML(), minor cleanup in the example * chore: cleanup QuillNativeBridgeWeb, extract mime constants, organize the order of implemented methods * fix(web): issue caused by previous commit * docs(web): update methods of QuillNativeBridge to reflect the web support * chore(android): cleanup the check for HTML from the clipboard in getClipboardHTML * test: add a test for getClipboardHTML to ensure it's not null only when the HTML item is the last/primary * ci: attempt to fix CI failure by updating the outdated path of quill_native_bridge after splitting the web implementation * docs: document why platform implementations of QuillNativeBridgePlatform should extend it rather than implements * chore(example): add Windows platform runner example using Flutter CLI (change is automated) * feat: (WIP) add windows experimental support for getClipboardHTML(), new integration test to ensure the HTML is parsable * fix(docs): update incorrect description of quill_native_bridge_windows in README * refactor: move plugin platform interface to quill_native_bridge_platform_interface * chore: (WIP): publish quill_native_bridge 10.7.5 * chore: (WIP) publish quill_native_bridge_web to use quill_native_bridge_platform_interface * chore(example): add Linux platform runner example using Flutter CLI (change is automated) * feat(linux): (WIP) highly experimental Linux support using xclip * fix(windows): non case sensitive check in stripWin32HtmlDescription() suggested by @AtlasAutocode * chore(example): unrelated change for windows in Flutter Quill example due to using flutter_inappwebview_windows without running the example on windows (outside of this PR) * chore(android): cleanup onMethodCall() in QuillNativeBridgePlugin * docs(readme): update docs to reflect the changes * chore: use hasWebSupport instead of hardcoding the check directly in QuillNativeBridgePlatformFeature.isSupported * feat: rename MethodChannelQuillNativeBridge.methodChannel to testMethodChannel, check in development mode to ensure that testMethodChannel can be only used in unit tests for non-web platforms * refactor: move android implementation from quill_native_bridge to quill_native_bridge_android, use pigeon for Android * chore: update QuillNativeBridgeAndroid.registerWith() assert message * refactor(apple): move iOS and macOS implementation from quill_native_bridge to quill_native_bridge_ios and quill_native_bridge_macos, use pigeon for iOS and macOS, clean the code and update QuillNativeBridge to QuillNativeBridgeImpl on Android * chore: add quill_native_bridge_platform_interface in pubspec_overides.yaml of quill_native_bridge * refactor: rename copyHTMLToClipboard() and getClipboardHTML() to copyHtmlToClipboard() and getClipboardHtml() * feat: more advanced method to check the platform support check allowing each platform to have its own check, publish a new experimental version of quill_native_version * chore(analysis): remove unused imports in quill_native_bridge and quill_native_bridge_platform_interface * fix(ci): temporarily pub get quill_native_bridge packages in main workflow * chore: format dart pigeon generated files as a workaround to CI failure * chore(deps): update flutter_lints to 5.0.0 in quill_native_bridge * refactor(windows): move CloseClipboard() to finally block in getClipboardHtml() * chore(windows): clear todo related to previous commit * chore(readme): remove description from the platform support table, use emojis instead of text for readability * chore(readme): use different emoji for platforms that don't support a feature or are inapplicable in the platform support table * style(windows): use TEXT and free from win32 instead of toNativeUtf16() and calloc.free(pointer) from ffi * refactor(windows): only register HTML format id once * feat(windows): (WIP) highly experimental support for copyHtmlToClipboard(), minor cleanup (need more) * refactor: extract supported platforms into Set for isSupported() * chore(windows): add error code in errors using GetLastError() * chore(windows): avoid passing the locked memory pointer to SetClipboardData() in copyHtmlToClipboard() * docs(readme): document android platform configuration in quill_native_bridge * fix(windows): (WIP) GlobalUnlock before SetClipboardData and GlobalFree on SetClipboardData failure * docs(readme): slightly improve platform configuration docs in quill_native_bridge * fix(windows): GlobalUnlock() the handle instead of pointer before calling GlobalUnlock() * chore(android): add reference to Flutter #63533 * chore(android): define package in GeneratedMessages.kt, update related imports * chore(android): ignore analysis warning for the generated code messages.g.dart * feat: (WIP) add initial support for getClipboardFiles() * chore(ci): format generated messages.g.dart file to bypass CI failure * feat(linux): add support for getClipboardFiles() * docs: update the old reference of QuillNativeBridgeFeature, add doc comment for the library * chore(readme): add link for quill_native_bridge, correct minor typo * chore(docs): minor changes to update the doc comment in QuillController * chore: move quill_native_bridge to https://github.com/FlutterQuill/quill-native-bridge * docs(readme): improve the 'Rich Text Paste' section, drop the TODO for providing a way to customize the paste behavior and customize the clipboard operations * chore: update deprecation message of FlutterQuillExtensions.useSuperClipboardPlugin() to reflect the change * chore(example): remove the comment of deprecated method FlutterQuillExtensions.useSuperClipboardPlugin() * chore: rename ClipboardService.copyImageToClipboard() to ClipboardService.copyImage() (non-breaking change) * chore: add a link to a TODO in DefaultClipboardService._getClipboardFile()
…_extensions, remove pubspec_overrides, part of singerdmx#2230
Description
Add default native implementation of
ClipboardService
for Android, macOS, iOS, Windows, and Web.Note
The package quill_native_bridge has been moved into a separate repo (FlutterQuill/quill-native-bridge).
The rich text-pasting feature now works without any additional plugins, no need for bridges or installing Rust.
This feature is introduced to allow moving super_clipboard into quill_super_clipboard (
super_clipboard
will be no longer a dependency offlutter_quill
orflutter_quill_extensions
) without introducing a breaking change.Rich Text Paste Flutter Quill Example
The example above doesn't call this method from
flutter_quill_extensions
:Videos
QuillExampleAndroid.mov
QuillExampleIos.mov
QuillExampleMacOS.mov
Manually tested on both real device and emulator/simulator.
Related Issues
quill_native_bridge
onlyType of Change
TODOS
Cleanup the code for native platformsDone only for Kotlin and Swift implementations, web, Linux, and Windows still need more cleanup.MethodChannelQuillNativeBridge
and related classes)Remove optional template assets and files for the example of quill_native_bridgeWill leave them as they don't update often.Retrieving gif images from the Clipboard on Android, iOS, and macOS.Was not able to support macOS since not natively supported same as iOS. Supported only on Android and iOS.See and discuss ifShould be separated, gif is not supported on desktop and web.getClipboardImage
should or should not return a Gif file, if not, shouldgetClipboardImage
fallback to returning the Gif image in case of unsupported operating systems? Related feat: add paste gif from clipboard #1788.See if there is a need for moving the web implementation into its own package (Web implementation has been separated into its own package.quill_native_bridge_web
andquill_native_bridge_platform_interface
), also we might consider throwing assert failure on all methods in case of web inMethodChannelQuillNativeBridge.dart
since will useQuillNativeBridgeWeb
for the web.flutter_quill
exampleAndroidManifest.xml
to support copying images to the clipboard.See if retrieving Markdown from the clipboard is a standard feature on all platforms or if there is a use-case to it.Doesn't seem to be a standard feature on most platforms.See if retrieving HTML and Markdown files is a standard feature on all platforms or if it's a desktop-specific featureSeems to be a desktop-specific feature, AFAIK is not supported on Android desktop mode.getClipboardImage
andgetGifImage
but they can be in the same function and will always return the image regardless of what it was. We probably do want to separate them so that users can have separate implementations foronImagePaste
andonGifPaste
Review the Clipboard Impl for Android, and see where we would useThe main point of retrieving HTML is to have rich text that can be converted and pasted into the editor, having plain text wrapped to HTML is not what we expect, this functionality is not available built-in on other platforms and we're trying to have those methods unified when possible. This feature of rich pasting will be only supported if the app you're copying content from supports it.coerceToHtmlText()
, currently we're always retrieving the first item.Might implement a new feature which is copyingTheDelta
asHTML
to the system clipboard to paste it for other apps (might not do it for now or only for example).copyHTMLToClipboard
has been added for integration tests but is not used inClipboardService
.ClipboardService
Fix issue Rich Text Paste Duplicates Content via HTML & Plain Text on Web #2220 for webThis is a separate issue. Will focus on the current issue for now.Might update Fix safari clipboard bug #1722 only to fix the bug for Safari instead, discuss the change first.Seems to be unrelated.See which features can be implemented on the web and the restrictions, we will probably use the Clipboard API on all browsers that support it and fall back to events (such as paste event) for Firefox.Retrieving GIF images from the clipboard is not supported on the web. Was able to use the Clipboard API instead of fallback to Clipboard events on Firefox without any issues. See [Web] Pasting HTML Doesn't Work Propertly #1998Update docs and references toUpdated, still need minor adjustments once we make it possible to disable those experimental features.super_clipbaord
influtter_quill
andflutter_quill_extensions
.Discuss if we need to introduce a breaking change forOnce Windows support is complete, removingflutter_quill_extensions
to movesuper_clipboard
into its own package or provide an example code instead.super_clipboard
shouldn't be a breaking change since thequill_native_bridge
all currently used features bysuper_clipboard
except getting Markdown and HTML files on the desktop.Support Linux Wayland, currently only support x11Moved into a separate issue in quill-native-bridge repo.See Flutter #127396 and decide if we should use theSince no decision has made by the Flutter team yet, and haven't discussed this change, decided to stick to plugin_platform_interface for now.base
keyword (introduced in Dart 3) or stick toplugin_platform_interface
forquill_native_bridge
(will probably do)quill_native_bridge
andflutter_quill_extensions
influtter_quill
once done before publishing a new release.TheWe may consider removingClipboardService
and related classes should be experimental for now by adding a comment andexperimental
annotation.ClipboardServiceProvider
and accessing theinstance
directly fromClipboardService
if being experimental is the only issue.Add the option to not use some of the features in this PR.This is a separate issue and might be solved in a separate PR, this PR focuses on replacing the implementation. See this comment, Feat: support for customize paste function #2156 and The option to disable the Rich clipboard feature or customize the clipboard behavior #2320.Check the format of Kotlin, and Swift in GitHub workflow.Will be in a separate GitHub workflow once we move theMoved into a separate issue in quill-native-bridge.quill_native_bridge
into its own package. Currently, Swift and Kotlin code is formatted.Currently on Android, will return the HTML/Image from the primary clip item (the last item has been copied), if the user copies HTML, then an image, and ifThe current behavior is the expected, at least on most applications. However, we should confirm if the current solution is consistent across different platforms when possible.getClipboardHTML
was called after that then will benull
since that's not the last/primary item (which is the image in this case). We should consider this and if it's the behavior that we expect.quill_native_bridge
QuillNativeBridgePlatform
(outside ofquill_native_bridge
) can decide if its unsupported feature, if we update a package implementation (e.g.quill_native_bridge_windows
) and add support for a method, updatingquill_native_bridge
orquill_native_bridge_platform_interface
is not required. Allowing to check if Clipboard API is supported inQuillNativeBridgeWeb
.Improve the error handling in general, throwing exceptions instead of using asserts inMoved to a separate issue in quill-native-bridge repoquill_native_bridge_windows
, andquill_native_bridge_linux
. Also, HandleProcess
exception on Linux.MethodChannelQuillNativeBridge
(quill_native_bridge_platform_interface
). We're handling Android-specific logic in theMethodChannelQuillNativeBridge
.Separate Android and Apple (iOS, macOS) implementation into their package.At first decided to have macOS and iOS in one packagequill_native_bridge_apple
.Update flutter_lints to latest versionAddressed in chore(deps): update dev dependency flutter_lints to 5.0.0, solve warnings #22925.0.0
and might update the lints forquill_native_bridge
TestMoved into a separate issue in quill-native-bridge repo.copyImageToClipboard
andgetClipboardImage
with common image formats on all platforms.OnceLack of support forgetClipboardFiles
supports web, avoid usingdart:io
inDefaultClipboardService
forgetHtmlFile
andgetMarkdownFile
methods (solve the TODO, currently returnnull
for web).getClipboardFiles
on the web blocks the fix, Moved to a separate issue in quill-native-bridge repo.ClipboardService
offlutter_quill
Support for Windows is very limited and highly experimental.