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

Bug fix: Removing check not allowing spell check on web #2252

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

joeserhtf
Copy link
Contributor

Description

Fix: Enable spell check for Web.

For some reason, this call was locked for web builds.
Removing this lock, and running multiple tests without problems.
Using the spell check package adds a lot to the size, but it's not a problem.

Related Issues

Found: #2164

Type of Change

  • New feature: Adds new functionality without breaking existing features.
  • [ X ] 🛠️ Bug fix: Resolves an issue without altering current behavior.
  • 🧹 Code refactor: Code restructuring that does not affect behavior.
  • Breaking change: Alters existing functionality and requires updates.
  • 🧪 Tests: Adds new tests or modifies existing tests.
  • 📝 Documentation: Updates or additions to documentation.
  • 🗑️ Chore: Routine tasks, or maintenance.
  • Build configuration change: Changes to build or deploy processes.

@EchoEllet
Copy link
Collaborator

EchoEllet commented Sep 20, 2024

Thank you for your contributions. If you do plan on using Spell Checker in your own app, please take a look at #2246 first. We do plan on breaking the SpellCheckerService completely soon to fix some issues. Which is the main reason I moved it into the example instead of own package. It will be experimental even after those changes.

@singerdmx I will review this change soon.

@CatHood0 Is this a reason for this check, or maybe it's for preventing an issue for the web?

@CatHood0
Copy link
Collaborator

@CatHood0 Is this a reason for this check or maybe it's for preventing an issue for the web?

Was this why it didn't work? I understand it now. Believe me, I completely forgot that I added that condition which, now that I see it, did nothing. Thanks for that quick fix. I already thought it would be something else.

And to answer you, no, I really doubt it did anything, and I don't think I remember why I added it.

Copy link
Collaborator

@CatHood0 CatHood0 left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing the code and fixing it. I didn't remember (even when I reviewed it I didn't notice this for some reason) that I added such a condition.

@singerdmx singerdmx merged commit da3ed41 into singerdmx:master Sep 20, 2024
2 checks passed
@stvhrs
Copy link

stvhrs commented Sep 20, 2024

thanks bro

CatHood0 pushed a commit to CatHood0/flutter-quill that referenced this pull request Oct 28, 2024
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.

5 participants