Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Closes #28342: change the ui direction after a locale change #28343

Merged
merged 2 commits into from
Jan 20, 2023
Merged

Closes #28342: change the ui direction after a locale change #28343

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
261a6f8
Closes #28342: change the ui direction after a locale change
Dec 29, 2022
49bd6f8
Merge branch 'main' into change_ui_direction_after_changing_locale
mergify[bot] Jan 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/src/main/java/org/mozilla/fenix/HomeActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import android.os.Build
import android.os.Bundle
import android.os.StrictMode
import android.os.SystemClock
import android.text.TextUtils
import android.text.format.DateUtils
import android.util.AttributeSet
import android.view.ActionMode
Expand Down Expand Up @@ -143,6 +144,7 @@ import org.mozilla.fenix.trackingprotection.TrackingProtectionPanelDialogFragmen
import org.mozilla.fenix.utils.BrowsersCache
import org.mozilla.fenix.utils.Settings
import java.lang.ref.WeakReference
import java.util.Locale

/**
* The main activity of the application. The application is primarily a single Activity (this one)
Expand Down Expand Up @@ -238,6 +240,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())
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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 🤞


binding = ActivityHomeBinding.inflate(layoutInflater)
setContentView(binding.root)
ProfilerMarkers.addListenerForOnGlobalLayout(components.core.engine, this, binding.root)
Expand Down