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

revert!: magnifier #2413

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

revert!: magnifier #2413

wants to merge 8 commits into from

Conversation

EchoEllet
Copy link
Collaborator

@EchoEllet EchoEllet commented Dec 11, 2024

Description

Remove the magnifier due to buggy behavior with many regressions.

See #2406 for a list of issues, reasons, and details.

This reverts the two related PRs using:

git revert af691e69ed185b99899aa2dd9bb9a57620abbf05  # https://github.com/singerdmx/flutter-quill/pull/2026
git revert 2937dc8c955d02f97cf2c254da1db69799483dfc  # https://github.com/singerdmx/flutter-quill/pull/2128

And then removed anything related to the magnifier that was made in related PRs manually.

To try this branch:

dependency_overrides:
  flutter_quill:
    git:
      url: https://github.com/singerdmx/flutter-quill.git
      ref: revert/magnifier-feat

Related Issues

Type of Change

  • Feature: New functionality without breaking existing features.
  • 🛠️ Bug fix: Resolves an issue without altering current behavior.
  • 🧹 Refactor: Code reorganization, no behavior change.
  • Breaking: Alters existing functionality and requires updates.
  • 🧪 Tests: New or modified tests
  • 📝 Documentation: Updates or additions to documentation.
  • 🗑️ Chore: Routine tasks, or maintenance.
  • Build configuration change: Build/configuration changes.

@EchoEllet EchoEllet changed the title Revert/magnifier feat revert!: magnifier Dec 11, 2024
…is in editor_keyboard_shortcut_actions_manager.dart and added back due to git revert
@EchoEllet
Copy link
Collaborator Author

EchoEllet commented Dec 11, 2024

This PR is not ready yet for merge. Should double-check the changes, and manually compare the changes.

Also, need to manually test the changes for possible regressions, test it before the magnifier, after it was added, and after reverting it (with this branch).

Before magnifier

$ git clone --branch v9.5.23 --depth 1 git@github.com:singerdmx/flutter-quill.git flutter-quill-before-magnifier
$ (cd flutter-quill-before-magnifier/example && flutter run)

After the magnifier with all changes and fixes

$ git clone --branch v11.0.0-dev.14 --depth 1 git@github.com:singerdmx/flutter-quill.git flutter-quill-latest-with-magnifier
$ (cd flutter-quill-latest-with-magnifier/example && flutter run)

After reverting the magnifier

$ git clone --branch revert/magnifier-feat --depth 1 git@github.com:singerdmx/flutter-quill.git flutter-quill-without-magnifier
$ (cd flutter-quill-without-magnifier/example && flutter run)
Videos

No issues, and seem to work as before:

AndroidApi35Test.mov
iOS16Test.mov

A buggy behavior reproduced after the revert; need to test it before and after the magnifier too:

AndroidBuggyBehavior.mov

@EchoEllet EchoEllet requested a review from CatHood0 December 11, 2024 14:59
@EchoEllet EchoEllet mentioned this pull request Dec 12, 2024
1 task
@EchoEllet EchoEllet added the breaking change This change introduces an incompatibility that requires adjustments in dependent code or packages, a label Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This change introduces an incompatibility that requires adjustments in dependent code or packages, a
Projects
None yet
1 participant