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

feat: add android open app language settings #288

Merged

Conversation

itsramiel
Copy link
Contributor

@itsramiel itsramiel commented Jan 8, 2025

Summary

The changes adds the capability to navigate users of the app to the language settings of the app for Android. It is not possible on iOS, macOS, and web afaik. An alternative solution would be use react native's Linking.openSettings() but this only opens the app settings and then the user has to look for the Language option which can take some time and be frustrating to the user. Instead, this function directly leads to the language selection.

The solution is implemented following the docs.

Test Plan

The changes can be tested using the example project in the repo. Below is a screen recording showing how it behaves.

Note that the per app language is available on api >= 33 and the app has to be setup to support multiple language as was done for the example project with the locales_config.xml file and the changes made to the manifest.

Screen.Recording.2025-01-08.at.12.31.54.PM.mov

What's required for testing (prerequisites)?

Android device with api level >= 33 and has custom languages set

What are the steps to test it (after prerequisites)?

Call the newly exposed method

Compatibility

OS Implemented
iOS
Android

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I added a sample use of the API in the example project (example/src/App.js)

@itsramiel itsramiel requested a review from zoontek as a code owner January 8, 2025 11:35
@itsramiel
Copy link
Contributor Author

@zoontek Please let me know if you welcome this addition before changing the readme

@zoontek
Copy link
Owner

zoontek commented Jan 8, 2025

@itsramiel Thanks for the PR!

LGTM, except maybe one small thing: Could you update the method to return a Promise<void> instead (and a possible rejection)? Here, if the user didn't read the doc correctly and try to use it on Android < 13, iOS or the web, it will do nothing and stay silent, not the best user experience. It also allows you to catch potential errors, like how it's done in react-native-permissions openSettings.

For unsupported platforms / versions, it should reject with an error: Only supported by Android 13 and above, similar to those. This way, someone naively using it will be informed. Or, if he tests only on 13+ devices, it will be reported in Sentry or equivalent, allowing him to wrap the button in a condition.

@itsramiel
Copy link
Contributor Author

@zoontek

I just updated it throw an error similar to react-native-permissions without having to make it async. I think there is no need for it to be a promise. Lmk what you think.

@zoontek
Copy link
Owner

zoontek commented Jan 9, 2025

@itsramiel I'm not a fan of throwing exceptions, if those are not handled, the app will crash, so it's actually even worse 😅

In a perfect world, everyone would use a Result type, but here, a Promise stays the best option (especially thanks to linters that would force the user to handle it)

Someone not reading the documentation (trust me, this happen a lot) will naturally add a catch block and display a generic error, which is better than a crash or no user feedback.

@itsramiel
Copy link
Contributor Author

@zoontek so basically make the function async to turn it into a promise and thats it?

@zoontek
Copy link
Owner

zoontek commented Jan 9, 2025

@itsramiel Yes

@itsramiel
Copy link
Contributor Author

Done

@itsramiel itsramiel force-pushed the feat/add-android-open-app-language-settings branch from b87692a to 029c15e Compare January 9, 2025 09:52
@zoontek zoontek merged commit c3819c4 into zoontek:master Jan 9, 2025
@zoontek
Copy link
Owner

zoontek commented Jan 9, 2025

Perfect, thanks!

@zoontek
Copy link
Owner

zoontek commented Jan 9, 2025

@itsramiel Released with v3.4.0

@itsramiel
Copy link
Contributor Author

@itsramiel Released with v3.4.0

Thank you 🙏

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.

2 participants