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

Remove init checks for locales #344

Merged
merged 2 commits into from
Oct 24, 2023
Merged

Remove init checks for locales #344

merged 2 commits into from
Oct 24, 2023

Conversation

figengungor
Copy link
Contributor

This enables to access locales before initializing SpeechToText.

Reason: Speech to text feature is optional in my app and I don't want to initialize unless user wants to use this feature. However I would like to show the current locale and locale list to user even though s/he didn't start to use this feature.

Copy link
Contributor

@sowens-csd sowens-csd left a comment

Choose a reason for hiding this comment

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

This looks like a reasonable change but have you checked whether it will work on iOS? We can't remove the initialization check in the plugin without having it work properly on both platforms. That could lead to incorrect behaviour on iOS if it is called without initialization.

Also, when you are listing locales is it acceptable that they are speech only locales? This method doesn't list all locales available for the device, only those that have speech recognition available for them.

@sowens-csd
Copy link
Contributor

I should have also said, thanks for the PR! I do appreciate the contribution.

@figengungor
Copy link
Contributor Author

Nope, I don't have iOS environment to test this right now. Can you test this for iOS?

Yes, it is acceptable they are speech only locales. I'll only use them for this feature.

If you want I can skip the init check for Android only. Or if you don't have time to test other platforms, that's ok, you can close this merge request and I'll use my fork.

Have a nice day!

@JekaNS
Copy link

JekaNS commented Jun 27, 2023

Hi there,

I'm also interested in this feature. So i checked this in my IOS project.

Знімок екрана 2023-06-27 о 23 06 17

In the screenshot, you can see that the execution is stopped at the breakpoint before initialization. And you can see the state of the internal properties _ availableLocales and _currentLocale , but sttInitialized is set to false.

I tested this on an IOS simulator (16.4) and a physical device (16.5.1).

So, for iOS is enough to remove the initialization check in the plugin (Dart part). And there is no need to make any changes to the iOS platform plugin part.

Copy link
Contributor

@sowens-csd sowens-csd left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the PR. I had to make one change to support the new Android 33+ check for locales. It means that the locales list will be empty until the user has responded to the permission dialog in the first initialize check.

@sowens-csd sowens-csd merged commit ecfd9a9 into csdcorp:main Oct 24, 2023
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