From 2d690736e36dc2fcbb36cb99c60eb370a76f2945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elias=20M=C3=BCller?= Date: Sat, 1 Aug 2026 18:16:22 +0200 Subject: [PATCH] implemented connectivity debouncing and refetch throttling for loadable state UI components --- .../bloc/loadable_state_bloc.dart | 73 ++++++++---- .../loadable_state_indicators.dart | 21 ++++ .../view/loadable_state_consumer.dart | 5 +- .../view/loadable_state_error_bar.dart | 11 +- .../files/search/files_search_results.dart | 3 +- pubspec.yaml | 2 +- .../state/loadable_state_visibility_test.dart | 107 ++++++++++++++++++ 7 files changed, 194 insertions(+), 28 deletions(-) create mode 100644 lib/state/app/infrastructure/loadable_state/loadable_state_indicators.dart create mode 100644 test/state/loadable_state_visibility_test.dart diff --git a/lib/state/app/infrastructure/loadable_state/bloc/loadable_state_bloc.dart b/lib/state/app/infrastructure/loadable_state/bloc/loadable_state_bloc.dart index 9c07d3b..195f920 100644 --- a/lib/state/app/infrastructure/loadable_state/bloc/loadable_state_bloc.dart +++ b/lib/state/app/infrastructure/loadable_state/bloc/loadable_state_bloc.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; import '../../../../../extensions/date_time.dart'; +import '../loadable_state_indicators.dart'; import 'loadable_state_event.dart'; import 'loadable_state_state.dart'; @@ -13,48 +14,75 @@ class LoadableStateBloc extends Bloc late StreamSubscription> _updateStream; void Function()? reFetch; - /// Last time [reFetch] was triggered by an [AppLifecycleState.resumed] - /// event. Used to coalesce rapid foreground/background flips so we don't - /// spam the network when the user briefly checks notifications. - DateTime _lastResumeRefetch = DateTime.fromMillisecondsSinceEpoch(0); + static const Duration _refetchThrottle = Duration(seconds: 10); + + /// Last time [reFetch] was triggered automatically (resume or reconnect). + /// Used to coalesce rapid foreground/background flips so we don't spam the + /// network when the user briefly checks notifications. + DateTime _lastRefetch = DateTime.fromMillisecondsSinceEpoch(0); + + /// Delays committing a transition *into* the offline state. On resume + /// connectivity_plus frequently emits a transient `[none]` before the real + /// result lands; debouncing it prevents the offline bar from flashing. + Timer? _offlineDebounce; LoadableStateBloc() : super(const LoadableStateState(connections: null)) { on((event, emit) { emit(event.state); - if (connectivityStatusKnown() && isConnected()) { - if (reFetch == null) return; - reFetch!(); - } + if (connectivityStatusKnown() && isConnected()) _triggerRefetch(); }); - void emitConnectivity(List result) { + Connectivity().checkConnectivity().then(_emitConnectivity); + _updateStream = Connectivity().onConnectivityChanged.listen( + _emitConnectivity, + ); + WidgetsBinding.instance.addObserver(this); + } + + void _emitConnectivity(List result) { + _offlineDebounce?.cancel(); + // Commit a connected reading immediately (snappy recovery); only debounce + // the drop to offline so a transient tick doesn't surface as offline. + if (!result.contains(ConnectivityResult.none)) { // The initial checkConnectivity() future is not cancellable and may // resolve after the bloc was disposed, so guard against a closed sink. if (isClosed) return; add(ConnectivityChanged(LoadableStateState(connections: result))); + return; } + _offlineDebounce = Timer(const Duration(milliseconds: 1200), () { + if (isClosed) return; + add(ConnectivityChanged(LoadableStateState(connections: result))); + }); + } - Connectivity().checkConnectivity().then(emitConnectivity); - _updateStream = Connectivity().onConnectivityChanged.listen( - emitConnectivity, - ); - WidgetsBinding.instance.addObserver(this); + void _triggerRefetch() { + final now = DateTime.now(); + if (!shouldRefetch( + _lastRefetch, + now, + _refetchThrottle, + hasReFetch: reFetch != null, + )) { + return; + } + _lastRefetch = now; + reFetch!(); } @override void didChangeAppLifecycleState(AppLifecycleState state) { if (state != AppLifecycleState.resumed) return; - final now = DateTime.now(); - if (now.difference(_lastResumeRefetch) < const Duration(seconds: 10)) { - return; - } - _lastResumeRefetch = now; - // Re-check connectivity so the resulting [ConnectivityChanged] handler - // clears a stale error bar and triggers [reFetch] once reachable again. + // Trigger the refetch synchronously so [RefetchStarted] clears any stale + // error and enters the loading state before the first resume frame paints, + // instead of only after the async connectivity round-trip below. + _triggerRefetch(); + // Still re-check connectivity so the (debounced) offline bar reflects the + // real network state after resume. unawaited( Connectivity().checkConnectivity().then((result) { if (isClosed) return; - add(ConnectivityChanged(LoadableStateState(connections: result))); + _emitConnectivity(result); }), ); } @@ -92,6 +120,7 @@ class LoadableStateBloc extends Bloc @override Future close() { WidgetsBinding.instance.removeObserver(this); + _offlineDebounce?.cancel(); _updateStream.cancel(); return super.close(); } diff --git a/lib/state/app/infrastructure/loadable_state/loadable_state_indicators.dart b/lib/state/app/infrastructure/loadable_state/loadable_state_indicators.dart new file mode 100644 index 0000000..fadf162 --- /dev/null +++ b/lib/state/app/infrastructure/loadable_state/loadable_state_indicators.dart @@ -0,0 +1,21 @@ +// Pure decision helpers for the shared loadable-state indicators. Kept free of +// Flutter/connectivity imports so they can be unit-tested in isolation. + +/// Whether the offline bar should be shown over cached content. Suppressed while +/// a fetch is in flight so a transient connectivity dip does not flash the bar +/// during the resume/refresh window. +bool shouldShowOfflineBar({ + required bool hasContent, + required bool loading, + required bool connectivityKnown, + required bool connected, +}) => hasContent && !loading && connectivityKnown && !connected; + +/// Whether an automatic refetch may run: only when a refetch callback exists and +/// the throttle [window] since the [last] trigger has elapsed. +bool shouldRefetch( + DateTime last, + DateTime now, + Duration window, { + required bool hasReFetch, +}) => hasReFetch && now.difference(last) >= window; diff --git a/lib/state/app/infrastructure/loadable_state/view/loadable_state_consumer.dart b/lib/state/app/infrastructure/loadable_state/view/loadable_state_consumer.dart index 7ddfa98..a90711a 100644 --- a/lib/state/app/infrastructure/loadable_state/view/loadable_state_consumer.dart +++ b/lib/state/app/infrastructure/loadable_state/view/loadable_state_consumer.dart @@ -62,8 +62,8 @@ class LoadableStateConsumer< final showPrimaryLoading = isLoading && !hasContent; final showBackgroundLoading = isLoading && hasContent; - final showError = hasError && !hasContent; - final showErrorBar = hasError && hasContent; + final showError = hasError && !hasContent && !isLoading; + final showErrorBar = hasError && hasContent && !isLoading; // Keep the wrapper hierarchy stable across refresh cycles: reFetch flips to // null mid-refetch, and toggling the RefreshIndicator on that signal would @@ -102,6 +102,7 @@ class LoadableStateConsumer< LoadableStateErrorBar( visible: showErrorBar, hasContent: hasContent, + loading: isLoading, message: loadableState.error?.message, technicalDetails: loadableState.error?.technicalDetails, lastUpdated: loadableState.lastFetch, diff --git a/lib/state/app/infrastructure/loadable_state/view/loadable_state_error_bar.dart b/lib/state/app/infrastructure/loadable_state/view/loadable_state_error_bar.dart index 417af24..670ab16 100644 --- a/lib/state/app/infrastructure/loadable_state/view/loadable_state_error_bar.dart +++ b/lib/state/app/infrastructure/loadable_state/view/loadable_state_error_bar.dart @@ -5,16 +5,19 @@ import 'package:flutter_bloc/flutter_bloc.dart'; import '../../../../../widget/info_dialog.dart'; import '../bloc/loadable_state_bloc.dart'; +import '../loadable_state_indicators.dart'; class LoadableStateErrorBar extends StatelessWidget { final bool visible; final bool hasContent; + final bool loading; final String? message; final String? technicalDetails; final int? lastUpdated; const LoadableStateErrorBar({ required this.visible, this.hasContent = false, + this.loading = false, this.message, this.technicalDetails, this.lastUpdated, @@ -26,8 +29,12 @@ class LoadableStateErrorBar extends StatelessWidget { @override Widget build(BuildContext context) { final bloc = context.watch(); - final isOfflineWithCache = - hasContent && bloc.connectivityStatusKnown() && !bloc.isConnected(); + final isOfflineWithCache = shouldShowOfflineBar( + hasContent: hasContent, + loading: loading, + connectivityKnown: bloc.connectivityStatusKnown(), + connected: bloc.isConnected(), + ); final shouldShow = visible || isOfflineWithCache; return AnimatedSize( diff --git a/lib/view/pages/files/search/files_search_results.dart b/lib/view/pages/files/search/files_search_results.dart index ff9b611..0c08d7a 100644 --- a/lib/view/pages/files/search/files_search_results.dart +++ b/lib/view/pages/files/search/files_search_results.dart @@ -55,7 +55,7 @@ class FilesSearchResults extends StatelessWidget { final showPrimaryLoading = isLoading && !hasContent; final showBackgroundLoading = isLoading && hasContent; final showErrorScreen = hasError && !hasContent && !isLoading; - final showErrorBar = hasError && hasContent; + final showErrorBar = hasError && hasContent && !isLoading; final showEmpty = !hasContent && !hasError && !isLoading; final errorMessage = hasError ? errorToUserMessage(controller.serverError) : null; @@ -65,6 +65,7 @@ class FilesSearchResults extends StatelessWidget { LoadableStateErrorBar( visible: showErrorBar, hasContent: hasContent, + loading: isLoading, message: errorMessage, ), // Background loading sits *outside* the result Stack so the linear diff --git a/pubspec.yaml b/pubspec.yaml index c8783ed..0c9fab6 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -3,7 +3,7 @@ description: Mobile client for Webuntis and Nextcloud with Talk integration publish_to: 'none' -version: 1.4.0+59 +version: 1.5.0+60 environment: sdk: ">=3.8.0 <4.0.0" diff --git a/test/state/loadable_state_visibility_test.dart b/test/state/loadable_state_visibility_test.dart new file mode 100644 index 0000000..15e9542 --- /dev/null +++ b/test/state/loadable_state_visibility_test.dart @@ -0,0 +1,107 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:marianum_mobile/state/app/infrastructure/loadable_state/loadable_state_indicators.dart'; + +void main() { + group('shouldShowOfflineBar', () { + test('shows when offline with cache and not loading', () { + expect( + shouldShowOfflineBar( + hasContent: true, + loading: false, + connectivityKnown: true, + connected: false, + ), + isTrue, + ); + }); + + test('is suppressed while loading even if offline with cache', () { + expect( + shouldShowOfflineBar( + hasContent: true, + loading: true, + connectivityKnown: true, + connected: false, + ), + isFalse, + ); + }); + + test('is hidden without cached content', () { + expect( + shouldShowOfflineBar( + hasContent: false, + loading: false, + connectivityKnown: true, + connected: false, + ), + isFalse, + ); + }); + + test('is hidden while connectivity is unknown', () { + expect( + shouldShowOfflineBar( + hasContent: true, + loading: false, + connectivityKnown: false, + connected: false, + ), + isFalse, + ); + }); + + test('is hidden when connected', () { + expect( + shouldShowOfflineBar( + hasContent: true, + loading: false, + connectivityKnown: true, + connected: true, + ), + isFalse, + ); + }); + }); + + group('shouldRefetch', () { + final window = const Duration(seconds: 10); + final base = DateTime(2026, 1, 1, 12, 0, 0); + + test('is false within the throttle window', () { + expect( + shouldRefetch( + base, + base.add(const Duration(seconds: 9)), + window, + hasReFetch: true, + ), + isFalse, + ); + }); + + test('is true once the window has elapsed', () { + expect( + shouldRefetch( + base, + base.add(const Duration(seconds: 10)), + window, + hasReFetch: true, + ), + isTrue, + ); + }); + + test('is false without a refetch callback', () { + expect( + shouldRefetch( + base, + base.add(const Duration(minutes: 5)), + window, + hasReFetch: false, + ), + isFalse, + ); + }); + }); +}