search

singerdmx / flutter-quill

Rich text editor for Flutter
https://pub.dev/packages/flutter_quill
MIT License
2.52k stars 806 forks source link

Make spell checker package optional #2142

Closed jonafeucht closed 2 weeks ago

jonafeucht commented 2 weeks ago

Is there an existing issue for this?

Flutter Quill version

Latest

Steps to reproduce

The simple_spell_checker package should be optional, it has ballooned my app from sub 70mb to >150mb, adding around 80mb more.

From what I can tell this is because of all the language dictionaries:

Screenshot 2024-08-23 at 16 50 54

My suggestion would be to just make platform specific api calls. Apple for example provides https://developer.apple.com/documentation/uikit/uitextchecker for iOS. Android and other platforms should have equivalent apis.

Expected results

Small app size

Actual results

Large app size

Code sample

No response

Additional Context

No response

CatHood0 commented 2 weeks ago

The idea of ​​this spell checker is to be a client-side service. I'm interested in knowing how we can make it optional, can you explain about it?

jonafeucht commented 2 weeks ago

The idea of ​​this spell checker is to be a client-side service. I'm interested in knowing how we can make it optional, can you explain about it?

Both Apple and Google apis are client side. I would expect Microsoft to have an equivalent api for Windows. No idea about the web, but having the package as a hard dependency there wouldn't be a problem, as the code lives server side.

My problem, when it primarily comes to mobile devices, is that the package is trying to reinvent the wheel. That's why I would prefer direct api calls to the platform provided apis.

My suggestion for a temporary solution would be to separate the package and make it standalone without it being a hard dependency of flutter_quill_extensions. And just point developers to the package if they need spell checking.

CatHood0 commented 2 weeks ago

My problem, when it primarily comes to mobile devices, is that the package is trying to reinvent the wheel. That's why I would prefer direct api calls to the platform provided

You have a point. The package exists mostly because I couldn't get Flutter's native SpellCheckerService to work. That is to say, no matter how much it did, it did not show the words that were misspelled (probably my bad configuration but I have no idea where that would be).

However, although it reinvents the wheel, the package is for those who want to manage the state of the language within the service without use the language that comes from the system, since in Flutter's native SpellCheckerService it is not possible (not that I have seen) in addition to allowing any language. personalized.

If we get into it a little more, MultiSpellChecker was created recently, which comes from the same instance as SimpleSpellChecker but unlike it, it can contain several languages ​​at the same time to check words.

An example of this is Obsidian, which in its Desktop version (I don't know if it will be for the web) allows you to select several languages ​​at the same time for the spell checker. This functionality is not natively available in the native API, and it would have to be reinvented a bit to create it using native API (which I do not rule out, I only created this option for anyone who prefers it or that suits their needs).

I will not deny that the main problem of the package is its size due to the dictionaries. I'm trying to fix it.

@EchoEllet @singerdmx what do you think? Should we move any implementation of simple_spell_checker to the example app to avoud this?

EchoEllet commented 2 weeks ago

do you think? what do you think? Should we move any implementation of simple_spell_checker to the example app to avoud this?

Yes, we could separate this feature in its own package that depends on simple_spell_checker, which has Flutter Quill specific code, or you could make the package only for flutter_quill which will require more steps to use the package without flutter_quill as a dependency.

Or we could make it specific for the example app.

As for the bundle size, we have already discussed this. However, we should have mentioned this in the CHANGELOG regarding the bundle size, users already using the package and don't expect such change in a minor update, we can't make another major release either.

@CatHood0 I can handle this since it doesn't require changing much or leaving it up to you to decide.

I prefer to keep the flutter_quill_spell_checker package (which hasn't been created yet) in the same repository as simple_spell_checker so you can get more control and publish releases without submitting PRs to this repository. Creating a new package in this repository is an option as well.

A solution is to use platform-specific API provided by iOS and Android as mentioned by @jonafeuch

The project is a dart package and doesn't have any platform-specific code for the native platform (Swift or Kotlin). Some platforms might not support this feature natively. We probably want to make this a plugin as well at some point.

Dart native interoperability image
CatHood0 commented 2 weeks ago

@EchoEllet then I leave you in your command. Thank you!

EchoEllet commented 2 weeks ago

I will start as soon as I can. It might be 1-2 days before I do.

EchoEllet commented 1 week ago

I have tried SpellCheckerSession on Android (platform-specific API) and in my case, it didn't work quite well, on the emulator it takes longer than expected to return the result, on a real device (I have used only one that's Android 14) and it returns an empty list.

The text: Ths is an exmple.

image

Flutter DefaultSpellCheckService didn't work at all for the emulator and real device.

I haven't looked into this issue much however we can probably implement it only on iOS and Android using system API.

the package is for those who want to manage the state of the language within the service without use the language that comes from the system

Can you clarify what you mean by this?

in Flutter's native SpellCheckerService it is not possible (not that I have seen)

Did you try it on Android or iOS?

CatHood0 commented 1 week ago

I noticed that this could be useful. I was made a similar suggestion to have simple_spell_checker also have an instance that uses the native system services (in case of IOS or Android). I haven't taken the time to do this yet, but it's on my list of things I could do

Can you clarify what you mean by this?

This part refers to the fact that we can handle languages ​​not only created by the users themselves (through the LanguageIdentifier class) but it also allows us to change at any time to a language that the operating system is not using. You can see it as if in Word, we changed the spell checker from English to Italian, even if the system is not using these languages. i clarify that i don't know if this same thing is already allowed with Flutter's native SpellCheckService service, although as far as I looked, there was nothing like that.

Did you try it on Android or iOS?

I tried it only on Android. Unfortunately I don't have the availability to use an iOS device, but still at no time was i able to get either a TextField/TextFormField or an EditableTextLine to work as I expected. I tried with or without an internet connection, and at no time did it give me results. But, as i said above, maybe it was some bad configuration on my part.

EchoEllet commented 1 week ago

This part refers to the fact that we can handle languages ​​not only created by the users themselves (through the LanguageIdentifier class) but it also allows us to change at any time to a language that the operating system is not using. You can see it as if in Word, we changed the spell checker from English to Italian, even if the system is not using these languages.

It's already possible to choose a different local than the system default, I haven't tried it out before though.

Flutter's native SpellCheckService service, although as far as I looked, there was nothing like that.

I tried DefaultSpellCheckService and wasn't able to get it working at all while it uses the same API that I used in native Android using MethodChannel.

CatHood0 commented 1 week ago

It's already possible to choose a different local than the system default, I haven't tried it out before though.

Thanks for letting me know. I'll take a look at it as soon as i can, as i might be able to make some kind of weird monster between the native and the implementation i created to eliminate the problem i have with the package's heaviness.

CatHood0 commented 1 week ago

I tried DefaultSpellCheckService and wasn't able to get it working at all while it uses the same API that I used in native Android using MethodChannel.

I really think it might be a Flutter issue that this aspect is not working properly. Could it be that internally the service is not being used even if we pass it in the parameters?

EchoEllet commented 1 week ago

Could it be that internally the service is not being used even if we pass it in the parameters?

I forgot to mention that I used fetchSpellCheckSuggestions(locale, text) with an instance of DefaultSpellCheckService directly without TextField or EditableTextLine.

EchoEllet commented 1 week ago

I think we need to refactor our SpellCheckService a bit. We probably want to extend it from Flutter SpellCheckService.

This will require breaking changes which is a bit unfortunate, we need to make it experimental first by with @experimental, adding docs comment, and in README for a while as I expect more breaking changes.

We don't need to make it a singleton. We can probably make it similar to how it works in TextField if we can use DefaultSpellCheckService.