-
Notifications
You must be signed in to change notification settings - Fork 6
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
Refactor of language change listener #52
Conversation
Stops it from flickering.
Resolved conflicts in order to unblock PR.
void onChanged(@NonNull Language newLangauge) { | ||
Intent intent = getIntent(); | ||
finish(); | ||
startActivity(intent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is what makes the screen stop flickering. Instead of restarting the same activity and causing a re-render the code gracefully finishing then starts a new instance of the activity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting, so my understanding was that finish()
leads to onDestroy()
, which may be cleaning up state that was created in onCreate()
. and then startActivity()
runs onStart()
, but not necessarily onCreate()
, which seems like it would be a problem, but i guess that finish()
+ startActivity()
results in onCreate()
being called? idk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
finish()
eventually leads to onDestroy()
. I'm starting a new instance of the same activity and relying on the normal activity transitions to provide a nicer UX than before. Instead, I could have notified the adapter of the changes, but since the selection of a new language is effectively an Android Config change I wanted the whole activity (incl NavBar) to re-pull resources.
i'll go ahead and merge. |
No description provided.