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

Code documentations and new event for confirm remove the image #1414

Merged
merged 4 commits into from
Sep 29, 2023
Merged

Code documentations and new event for confirm remove the image #1414

merged 4 commits into from
Sep 29, 2023

Conversation

EchoEllet
Copy link
Collaborator

@EchoEllet EchoEllet commented Sep 29, 2023

Related to #1413 and
#1412

Made new changes

  • Documented the code of flutter_quill_extensions
  • Added a new event to allow the developer to confirm or not confirm removing the image that is necessary for production, in my opinion

… delete unused dependencies and upgrade all packages and plugins and remove gallery_saver which has not been updated for more than 23 months, it was a great plugin but it old now, and I also add some simple documentation and other minor improvements
…new event to allow the user to confirm removing the image before actually remove it, translated some text in Arabic languague since it was incorrect or missing
@singerdmx
Copy link
Owner

You have conflict
Please rebase

@EchoEllet
Copy link
Collaborator Author

You have conflict Please rebase

Done

@EchoEllet
Copy link
Collaborator Author

All the tests have passed:

image

So why it all checks fail in GitHub?? if is this because of the conflict

please rerun the tests with the latest commit.

@singerdmx
Copy link
Owner

info • The line length exceeds the 80-character limit • flutter_quill_extensions/lib/flutter_quill_extensions.dart:85:81 • lines_longer_than_80_chars

@EchoEllet
Copy link
Collaborator Author

info • The line length exceeds the 80-character limit • flutter_quill_extensions/lib/flutter_quill_extensions.dart:85:81 • lines_longer_than_80_chars

I forgot to run flutter analyze

Now I have fixed the merge conflicts as well the test

@singerdmx singerdmx merged commit b722475 into singerdmx:master Sep 29, 2023
1 check passed
ImageEmbedBuilder({required this.afterRemoveImageFromEditor});
ImageEmbedBuilder({
required this.afterRemoveImageFromEditor,
required this.shouldRemoveImageFromEditor,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be nullable, instead of passing the default parameter of the function that just returns true.

@@ -15,13 +15,13 @@ dependencies:
flutter_colorpicker: ^1.0.3
flutter_keyboard_visibility: ^5.4.1
quiver: ^3.2.1
url_launcher: ^6.1.12
url_launcher: ^6.1.14
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't bump minimum plugin versions unless there is a major version/breaking change or we need to use a api feature that's only in a newer plugin

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