Closes #28342: change the ui direction after a locale change #28343
Conversation
| @@ -229,6 +231,10 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { | |||
|
|
|||
| components.publicSuffixList.prefetch() | |||
|
|
|||
| // Changing a language on the Language screen restarts the activity, but the activity keeps | |||
| // the old layout direction. We have to update the direction manually. | |||
| window.decorView.layoutDirection = TextUtils.getLayoutDirectionFromLocale(Locale.getDefault()) | |||
There was a problem hiding this comment.
Drive by: This seems like a last resort location to do an initialization step from a setting. Would it be possible to include this in the locale settings flow when updating the language? DefaultLocaleSettingsController might be a better alternative.
There was a problem hiding this comment.
That was my first though as well, but this approach has an issue – invalidating the layout and recreating the activity happens not synchronously and looks like a glitch.
FILE.2023-01-09.13.00.00.mp4
What concerned did you have in mind here? Applying proper orientation during onCreate looks okay to me. setLayoutDirection has also a safety check to invalidate the layout only if it is needed.
There was a problem hiding this comment.
Where is the orientation being set in HomeActivity?
My primary concern is that this was even needed at all and might be a heavy hammer, but I did notice that HomeActivity extends from LocaleAwareAppCompatActivity which has this same line in its own onCreate. Based on the adjacent comment in HomeActivity, the super.onCreate only gets called after some disk read violations, but it was obviously needed.
There was a problem hiding this comment.
The use-case I am working on is DefaultLocaleSettingsController changing the Locale to something that has a different text orientation and restarts the activity. Changing the locale should also change the layout orientation, but it doesn't and the activity is recreated with the old orientation. To avoid glitching, enforcing changed orientation looks like a good idea to me. What do you think?
There was a problem hiding this comment.
Judging by the conversation in #9413, I think I'm OK with this approach, as it's a natural side effect of us having to take care of all of the configuration changes. Maybe it won't be necessary in a 100% Compose world 🤞
|
Thank you, Noah! @MozillaNoah |
mavduevskiy
added
the
pr:needs-landing
FILE.2022-12-29.11.01.07.mp4
Pull Request checklist
QA
To download an APK when reviewing a PR (after all CI tasks finished running):
Checksat the top of the PR page.firefoxci-taskclustergroup on the left to expand all tasks.build-debugtask.View task in Taskclusterin the newDETAILSsection.GitHub Automation
Fixes #28342