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

fix: use immediate state for reloading the tray where possible #1498

Merged
merged 5 commits into from
Nov 7, 2024

Conversation

XavierChanth
Copy link
Member

- What I did

  • The only case where we may see old values is from ProfileRunningState, but ProfileState should also trigger a reload, so this is likely not an issue at all. Will observe to confirm.

- How I did it

- How to verify it

- Description for the changelog
fix: use immediate state for reloading the tray where possible

Comment on lines +29 to 46
void reloadTray(BuildContext context, Loggable state) async {
var cubit = context.read<TrayCubit>();
switch (state) {
case FavoritesState _:
cubit.reload(favoriteState: state);
case ProfileListState _:
cubit.reload(profileListState: state);
case ProfilesRunningState _:
cubit.reload(profilesRunningState: state);
case SettingsLoadedState _:
var localizations = await AppLocalizations.delegate.load(state.settings.language.locale);
cubit.reload(localizations: localizations);
case ProfileState _:
cubit.reload(profileState: state);
default:
cubit.reload(localizations: AppLocalizations.of(context));
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is where the magic happens... we conditionally pass the state to the appropriate field to ensure that TrayCubit.reload uses immediate values wherever possible, rather than pulling from possibly outdated context.

}

@override
Widget build(BuildContext context) {
var trayCubit = context.read<TrayCubit>();
if (trayCubit.state is TrayInitial) {
trayCubit.initialize();
trayCubit.initialize(localizations: AppLocalizations.of(context));
Copy link
Member Author

Choose a reason for hiding this comment

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

Tray cubit also takes localizations as input in the initialize function, so that the it can immediately set the tray with the desired language on startup.

Comment on lines +77 to +87
BlocListener<SettingsBloc, SettingsState>(
listener: reloadTray,
// Only call listener when the language changes in settings
listenWhen: (prev, next) {
if (prev is SettingsLoadedState && next is SettingsLoadedState) {
return prev.settings.language != next.settings.language;
}
// This may cause some extra reloading (very occasionally, settings shouldn't change often)
// but it should catch all of the edge cases
return prev is SettingsLoadedState || next is SettingsLoadedState;
}),
Copy link
Member Author

Choose a reason for hiding this comment

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

Added settings bloc listener for the locale, but it only should trigger a reload when the language changes (listenWhen function)

@@ -1,4 +1,5 @@
{

Copy link
Member Author

Choose a reason for hiding this comment

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

Extracted these localizations from your branch @CurtlyCritchlow

@CurtlyCritchlow CurtlyCritchlow merged commit de0fd70 into trunk Nov 7, 2024
9 checks passed
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