Skip to content

Conversation

@LuchoTurtle
Copy link
Member

@LuchoTurtle LuchoTurtle commented Aug 17, 2023

closes #1

This adds an implementation example using https://github.com/singerdmx/flutter-quill and documents the options available in order to fulfill the requirements stated in #1.

@LuchoTurtle LuchoTurtle added documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality flutter Flutter related issues labels Aug 17, 2023
@LuchoTurtle LuchoTurtle self-assigned this Aug 17, 2023
@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Aug 21, 2023

Kinda frustrating having to manually bump the SDK to version 21 because flutter_quill_extensions is using a gallery_saver library that is tossing the error just so I can test the app on a physical device.

Launching lib/main.dart on ONEPLUS A6010 in debug mode...
main.dart:1
/Users/lucho/Documents/dwyl/flutter-wysiwyg-editor-tutorial/android/app/src/debug/AndroidManifest.xml Error:
	uses-sdk:minSdkVersion 19 cannot be smaller than version 21 declared in library [:gallery_saver] /Users/lucho/Documents/dwyl/flutter-wysiwyg-editor-tutorial/build/gallery_saver/intermediates/merged_manifest/debug/AndroidManifest.xml as the library might be using APIs not available in 19
	Suggestion: use a compatible library with a minSdk of at most 19,
		or increase this project's minSdk version to at least 21,
		or use tools:overrideLibrary="carnegietechnologies.gallery_saver" to force usage (may lead to runtime failures)

They should either upgrade or use another library, as stated in singerdmx/flutter-quill#1350 :\

@nelsonic
Copy link
Member

That is frustrating. 🤦‍♂️
Are you able to Fork it and make the necessary updates then create a PR to update upstream repo?

@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Aug 21, 2023

Perhaps, but I think I can work with having minSdkVersion set to 21 (I just needed to upgrade from 20 to 21, so it's not THAT big of a deal.

I'll see to it after sorting through this epic.

@LuchoTurtle
Copy link
Member Author

Been trying to paste images directly into the clipboard and not from the gallery (as an optional route) but it doesn't work in Android devices (solely, it works on other platforms).
This is because of this line:

https://github.com/singerdmx/flutter-quill/blob/5dca6008cde707beb46291b6fbf71af6dbbf7bfb/lib/src/widgets/raw_editor.dart#L1534

Unfortunately, this is not straightforward in Android devices. I'll keep doing the "opening gallery and choosing" route.

@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Aug 21, 2023

While image upload works on mobile, it does not work on web. I've explained one route that might get to work in singerdmx/flutter-quill#976. 🙆‍♂️

@nelsonic
Copy link
Member

@LuchoTurtle please consider opening a separate issue for pasting images from clipboard for Android so that you can ship the basic version of this. I know you want to get everything on the requirements working. But it's better to ship something basic than get stuck on a "nice to have" feature like this. 👌

Saw your comment on using universal_io. Seems like a no-brainer to use it. 💭

@LuchoTurtle
Copy link
Member Author

Opened a PR that addresses an issue that I had embedding an image -> singerdmx/flutter-quill#1367.

Although it works, I'm testing this with an image URL, not a local file. I don't believe this is actually possible, so we'd need to upload an image to our S3 bucket.

@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Aug 22, 2023

I've dismissed the previous PR because it doesn't properly work on mobile devices.

Ugh, having this working on both web and mobile is utterly frustrating.

If I follow a basic Quill editor, embedding image on the editor works fine on mobile. However, to have it work for the web, I need to:

  • add define a FlutterQuillEmbed and add a webImagePickImpl and onImagePickCallback callback functions attributes to the toolbar initialization, like so:
    final embedButtons = FlutterQuillEmbeds.buttons(

	 // ......
      // `onImagePickCallback` is called after image (from any platform) is picked
      onImagePickCallback: _onImagePickCallback,

      // `webImagePickImpl` is called after image (from web) is picked and then `onImagePickCallback` is called
      webImagePickImpl: _webImagePickImpl,

      // ........
    );

This is only to make it compilable and workable on the web with a fixed image URL address.
While the image is resizable on mobile, it's apparently not by default on the web, as shown in the source code:

    ImageUniversalUI().platformViewRegistry.registerViewFactory(imageUrl, (viewId) {
      return html.ImageElement()
        ..src = imageUrl
        ..style.height = 'auto'
        ..style.width = 'auto';
    });

This will insert an image with the original dimensions into the editor.

image

Now, we've reached a point where (as you've said on standup), is easier (and ultimately better for us) to upload the image to our imgup and use the public URL.
Now we can:

  • use the compressed URL to downsize the image.
  • override these settings in Flutter and put a hard cap on the web image embed (e.g. have 300px on width).
  • both.

I'll try overriding these settings after documenting and fully testing what I have right now. I'll see if there's any web widget in Flutter that is easily resizeable and doesn't completely break the editor 🤔

@codecov
Copy link

codecov bot commented Sep 5, 2023

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@LuchoTurtle
Copy link
Member Author

I've added the rest of the documentation and added all of the details into the README file.
After this, I've tried going through platform channels in a last attempt to override the behaviour from image-picker inside flutter-quill by using setMockMethodCallHandler and although I got some progress by running the mocked overridden version, I keep getting this following error:

The following PlatformException was thrown running a test:
PlatformException(error, Invalid argument: Instance of 'XFile', null, null)

When the exception was thrown, this was the stack:
#0      StandardMethodCodec.decodeEnvelope (package:flutter/src/services/message_codecs.dart:652:7)
#1      MethodChannel._invokeMethod (package:flutter/src/services/platform_channel.dart:310:18)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)

I feel like I've spent too much already on trying to get this to work, so I wager it makes sense to tackle uploading to S3 and rendering on the web properly.

@LuchoTurtle LuchoTurtle assigned nelsonic and unassigned LuchoTurtle Sep 6, 2023
@LuchoTurtle LuchoTurtle marked this pull request as ready for review September 6, 2023 09:36
@LuchoTurtle
Copy link
Member Author

Following #1 (comment), I'm submitting this for review.

It does not yet upload images to S3, as I believe this should be contained on a different PR and because this is already a big one to review.

@nelsonic nelsonic added the awaiting-review An issue or pull request that needs to be reviewed label Sep 8, 2023
@nelsonic

This comment was marked as outdated.

@nelsonic

This comment was marked as outdated.

@nelsonic
Copy link
Member

Flutter 3.13.4 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 367f9ea16b (3 days ago) • 2023-09-12 23:27:53 -0500
Engine • revision 9064459a8b
Tools • Dart 3.1.2 • DevTools 2.25.0

Running flutter doctor...
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.13.4, on macOS 13.5.1 22G90 darwin-arm64, locale en-GB)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
[✓] Xcode - develop for iOS and macOS (Xcode 14.3.1)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2021.3)
[✓] VS Code (version 1.82.0)
[✓] Connected device (2 available)
    ! Error: Failed to prepare the device for development. Please unlock and reconnect the device. (code 806)
[✓] Network resources

• No issues found!

Then:

flutter test

Output:

...
> vm_service 11.7.1 (was 11.3.0) (11.10.0 available)
+ web 0.1.4-beta
  win32 5.0.6 (5.0.7 available)
  xdg_directories 1.0.2 (1.0.3 available)
  xml 6.3.0 (6.4.2 available)
Changed 8 dependencies!
00:11 +3: All tests passed!

All good. ✅

image

🙄

@nelsonic
Copy link
Member

Is it not possible to test this file? 💭
image

@nelsonic
Copy link
Member

Converted your demo video to GIF and double-speed: https://ezgif.com/optimize/ezgif-2-26cec1fc55.gif

dwyl-wysiwyg-demo-optimised

@nelsonic
Copy link
Member

Image uploads don't work on my localhost in Chrome:
image

This is "OK" as it will be fixed by using visual-editor in the follow-up. ✅

Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LuchoTurtle :shipit:

@nelsonic nelsonic merged commit ce6c758 into main Sep 16, 2023
@nelsonic nelsonic deleted the wysiwyg-#1 branch September 16, 2023 10:13
nelsonic added a commit that referenced this pull request Sep 16, 2023
@nelsonic nelsonic mentioned this pull request Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review An issue or pull request that needs to be reviewed documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality flutter Flutter related issues

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

Feat: Write a comprehensive WYSIWYG Tutorial for Flutter 📝

3 participants