Add additional config flag to allow forcing RTL on LTR locales #28129
Conversation
|
The Pull Request introduced fingerprint changes against the base commit: 9782fb3 Fingerprint diff[
{
"type": "dir",
"filePath": "../../packages/expo-localization/android",
"reasons": [
"expoAutolinkingAndroid"
],
"hash": "d4279285f5a3737985209267bde65337cf36189b"
}
]Generated by PR labeler 🤖 |
aleqsio
force-pushed
the
@aleqsio/add-force-rtl-flag
branch
from
6304141
to
2b2c36e
Compare
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
| @@ -400,7 +400,7 @@ open class ExperienceActivity : BaseExperienceActivity(), StartReactInstanceDele | |||
| ) | |||
| showOrReconfigureManagedAppSplashScreen(optimisticManifest) | |||
| setLoadingProgressStatusIfEnabled() | |||
| ExperienceRTLManager.setSupportsRTLFromManifest(this, optimisticManifest) | |||
| ExperienceRTLManager.setRTLPreferencesFromManifest(this, optimisticManifest) | |||
There was a problem hiding this comment.
It may be necessary to implement something similar in the dev client as it also configures the app based on the manifest. Not very important right now, but it would be nice to have that there too.
| it.apply() | ||
| } | ||
| } else { | ||
| if (supportsRTL == "true" || supportsRTL == "false") { |
There was a problem hiding this comment.
| if (supportsRTL == "true" || supportsRTL == "false") { | |
| if (supportsRTL != null) { |
There was a problem hiding this comment.
Not really, since we set those keys in the strings.xml config template as unset, we need this exact check.
| .getSharedPreferences(SHARED_PREFS_NAME, Context.MODE_PRIVATE) | ||
| .edit() | ||
| .also { | ||
| it.putBoolean(KEY_FOR_PREFS_ALLOWRTL, supportsRTL == "true") |
There was a problem hiding this comment.
Should we also set KEY_FOR_PREFS_FORCERTL to false? This key may already exist in the shared preferences. We don't want to use the old value.
There was a problem hiding this comment.
I'll do that but only if it's explicitly false
if (forcesRTL == "false") {
it.putBoolean(KEY_FOR_PREFS_FORCERTL, false)
}
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
Why and how
There's this issue that has been going on for some time 😅
#26532
It's due to two things:
New key is:
Test Plan
Performed the following tests:
LTR/RTL depend on calling I18nManager.(force/allow)RTL – built in react-native behavior
if extra.forcesRTL true:
app RTL, I18nManager.isRTL true
if extra.supportsRTL true:
LTR/RTL depend on locale
app LTR, I18nManager.isRTL false
if extra.forcesRTL true:
app RTL, I18nManager.isRTL true
if extra.supportsRTL true:
LTR/RTL depend on locale
LTR/RTL depend on calling I18nManager.(force/allow)RTL – built in react-native behavior
if extra.forcesRTL true:
app RTL, I18nManager.isRTL true
if extra.supportsRTL true:
LTR/RTL depend on locale
app LTR, I18nManager.isRTL false
if extra.forcesRTL true:
app RTL, I18nManager.isRTL true
if extra.supportsRTL true:
LTR/RTL depend on locale
Checklist
npx expo prebuild& EAS Build (eg: updated a module plugin).