-
-
Notifications
You must be signed in to change notification settings - Fork 209
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
offline mode improvements #546
offline mode improvements #546
Conversation
@@ -106,6 +108,12 @@ class BottomNavScaffold extends ConsumerWidget { | |||
final currentTab = ref.watch(currentBottomTabProvider); | |||
final tabs = ref.watch(tabsProvider); | |||
|
|||
bool isOnline = true; | |||
|
|||
ref.listen(connectivityChangesProvider, (_, connectivity) { |
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.
You should not use ref.listen
here but fetch connectivity value directly in _onItemTapped
.
final curTab = ref.read(currentBottomTabProvider); | ||
final tappedTab = BottomTab.values[index]; | ||
|
||
if (!isOnline && tappedTab == BottomTab.watch) { |
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 logic would be better placed in the else
of
if (tappedTab == curTab) {
...
} else {
// put it here because we want it to apply only if the tapped tab is not the current tab
}
@@ -749,20 +749,33 @@ class _GamePreview<T> extends StatelessWidget { | |||
} | |||
|
|||
class _RelationButton extends ConsumerWidget { | |||
const _RelationButton(); | |||
const _RelationButton(this.wasOnline); |
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.
The wasOnline
is weird here. We want to disable the button if currently not online, the wasOnline
serves another purpose.
|
||
@override | ||
Widget build(BuildContext context, WidgetRef ref) { | ||
return AppBarIconButton( | ||
icon: const Icon(Icons.people), | ||
icon: Icon( |
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.
You shouldn't change the button color. In flutter you disable a button by passing a null
callback to the onPressed
parameter. Flutter will disable and grey out the button automatically then.
fullscreenDialog: true, | ||
); | ||
if (!wasOnline) { | ||
showPlatformSnackbar( |
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.
Not necessary to show a snackbar, just disabling the button is enough.
@veloce thanks for the review, i will do the changes when i have a bit of time |
Closing as #645 was merged. |
Closes: #462
Recap
In puzzle tab:
remove error messages for dashboard and history that could not load: cannot reproduce the error message, even offline, the history and dashboards are displayedIn play tab:
In the bottom navigation bar: