From 5eec91d02e60ddca18432cf8f630aa628e335140 Mon Sep 17 00:00:00 2001 From: Seenu K Date: Sat, 10 Oct 2026 10:10:13 +0530 Subject: [PATCH 01/10] fix: show the empty group list at once instead of a sync spinner With no saved groups, the list hid its empty state behind a spinner until the first full sync finished. That flag lives only in memory, so every cold start waited on the network, and offline users never saw the empty state. The empty state now renders as soon as the local database answers. A thin progress bar under the app bar shows while this session's first full sync runs, and the sync status banner now appears for an empty list too. DestinationScaffold gains a `bottom` slot for the bar. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YSJh8YFmryHU41unFvrxSu --- .../screens/group_list_screen.dart | 9 +++- lib/presentation/widgets/page_body.dart | 7 +++ .../widgets/sync_status_notice.dart | 28 +++++++++++ test/presentation/session_sync_test.dart | 2 +- test/presentation/sync_refresh_test.dart | 47 +++++++++++++++++++ 5 files changed, 90 insertions(+), 3 deletions(-) diff --git a/lib/presentation/screens/group_list_screen.dart b/lib/presentation/screens/group_list_screen.dart index 638801e..1271bab 100644 --- a/lib/presentation/screens/group_list_screen.dart +++ b/lib/presentation/screens/group_list_screen.dart @@ -54,6 +54,11 @@ class GroupListScreen extends ConsumerWidget { return DestinationScaffold( titleWidget: const BrandLockup(), actions: [if (kIsWeb) const SyncRefreshButton.everything()], + // The empty state is shown at once rather than held back until the + // server confirms it, so an empty list says when it may yet fill. + bottom: groups.isEmpty + ? const InitialSyncProgress(label: 'Checking for your groups') + : null, // Disabled until the device has learned what a currency is, which is only // ever true during a brand-new install's first sweep or on a rebuilt // device with no connection. @@ -102,12 +107,12 @@ abstract final class _GroupList { const UnsyncedChangesBanner(padding: _noticeGap), const ConflictingEditBanner(padding: _noticeGap), const LinkAccountPrompt(padding: _noticeGap), - if (groups.isNotEmpty) const SyncStatusBanner(padding: _noticeGap), + const SyncStatusBanner(padding: _noticeGap), ], ), ), if (groups.isEmpty) - const SliverToBoxAdapter(child: InitialSyncGate(child: _EmptyState())) + const SliverToBoxAdapter(child: _EmptyState()) else SliverPadding( padding: const EdgeInsets.only(top: 8), diff --git a/lib/presentation/widgets/page_body.dart b/lib/presentation/widgets/page_body.dart index 438294d..7738b33 100644 --- a/lib/presentation/widgets/page_body.dart +++ b/lib/presentation/widgets/page_body.dart @@ -34,6 +34,7 @@ class DestinationScaffold extends StatelessWidget { this.titleWidget, required this.slivers, this.actions, + this.bottom, this.floatingActionButton, this.wrap, this.maxWidth = 760, @@ -49,6 +50,11 @@ class DestinationScaffold extends StatelessWidget { final Widget? titleWidget; final List? actions; + + /// A strip along the app bar's lower edge, such as a progress bar, that + /// stays put while the page scrolls under it. + final PreferredSizeWidget? bottom; + final Widget? floatingActionButton; /// Wraps the scroll view, for the destination that pulls to sync. @@ -78,6 +84,7 @@ class DestinationScaffold extends StatelessWidget { SliverAppBar.large( title: titleWidget ?? Text(title!), actions: actions, + bottom: bottom, // Collapses into the small bar and stays there, which is what // Material 3 specifies for a large top app bar: the headline is // worth the height when you arrive and worth none of it while you diff --git a/lib/presentation/widgets/sync_status_notice.dart b/lib/presentation/widgets/sync_status_notice.dart index 6f7449b..8a966e3 100644 --- a/lib/presentation/widgets/sync_status_notice.dart +++ b/lib/presentation/widgets/sync_status_notice.dart @@ -48,6 +48,34 @@ class InitialSyncGate extends ConsumerWidget { } } +/// A thin bar under the app bar while this session's first full refresh runs. +/// +/// Saved data is shown as soon as the device has read it; this only says that +/// more may still arrive from the server. Later refreshes stay quiet, so a +/// write or a return to the foreground does not flash it. The bar's height is +/// reserved even while idle, so content does not shift when it disappears. +class InitialSyncProgress extends ConsumerWidget + implements PreferredSizeWidget { + const InitialSyncProgress({super.key, required this.label}); + + /// What is being checked, for screen readers. + final String label; + + @override + Size get preferredSize => const Size.fromHeight(4); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final status = ref.watch(syncControllerProvider); + final checking = + status.enabled && status.isSyncing && !status.hasCompletedFullSync; + return SizedBox.fromSize( + size: preferredSize, + child: checking ? LinearProgressIndicator(semanticsLabel: label) : null, + ); + } +} + /// Keeps saved data visible while explaining a failed refresh or upload. class SyncStatusBanner extends ConsumerWidget { const SyncStatusBanner({super.key, this.padding = EdgeInsets.zero}); diff --git a/test/presentation/session_sync_test.dart b/test/presentation/session_sync_test.dart index 54007f2..00aeb06 100644 --- a/test/presentation/session_sync_test.dart +++ b/test/presentation/session_sync_test.dart @@ -198,7 +198,7 @@ void main() { await tester.pump(const Duration(milliseconds: 10)); } expect(container.read(syncControllerProvider).isSyncing, isTrue); - expect(find.text('No groups yet'), findsNothing); + expect(find.bySemanticsLabel('Checking for your groups'), findsOneWidget); expect(tester.takeException(), isNull); auth.currentUser = _otherAccount; diff --git a/test/presentation/sync_refresh_test.dart b/test/presentation/sync_refresh_test.dart index a2efb05..9e86ef8 100644 --- a/test/presentation/sync_refresh_test.dart +++ b/test/presentation/sync_refresh_test.dart @@ -196,6 +196,53 @@ void main() { expect(find.byType(GroupListSkeleton), findsNothing); }); + testWidgets('an empty list shows at once while the first refresh runs', ( + tester, + ) async { + final sync = _TestSync(initial: const SyncStatus(isSyncing: true)); + await _mount( + tester, + sync, + const GroupListScreen(), + groups: Stream.value(const []), + ); + await tester.pump(); + expect(find.text('No groups yet'), findsOneWidget); + expect(find.byType(LinearProgressIndicator), findsOneWidget); + + sync.finish(); + await tester.pumpAndSettle(); + expect(find.text('No groups yet'), findsOneWidget); + expect(find.byType(LinearProgressIndicator), findsNothing); + }); + + testWidgets('an empty list keeps its empty state through a failed refresh', ( + tester, + ) async { + final sync = _TestSync(initial: const SyncStatus(isSyncing: true)); + await _mount( + tester, + sync, + const GroupListScreen(), + groups: Stream.value(const []), + ); + await tester.pump(); + sync.show( + SyncStatus( + lastReport: SyncReport( + pushed: 0, + pulled: 0, + failed: 0, + error: StateError('connection unavailable'), + ), + ), + ); + await tester.pumpAndSettle(); + expect(find.text('No groups yet'), findsOneWidget); + expect(find.text('Could not refresh'), findsOneWidget); + expect(find.byType(LinearProgressIndicator), findsNothing); + }); + testWidgets('the group list keeps its saved rows through refresh and error', ( tester, ) async { From 67b0491e85e344181f45df6b540f191ddd578ed5 Mon Sep 17 00:00:00 2001 From: Seenu K Date: Sat, 10 Oct 2026 10:42:01 +0530 Subject: [PATCH 02/10] feat: open on saved data, and say offline quietly Launch: the first frame is deferred (WidgetsBinding.deferFirstFrame) until the landing screen has no local-data placeholder on it, capped at 800ms. The Android splash and the web loader stay up meanwhile, so a cold start goes straight to the saved groups and their balances. Placeholders opt in via LaunchPlaceholder; anything waiting on the network never holds launch. Offline: a sync that failed because no server answered now shows a single quiet line instead of the "Could not refresh" card, and a failed sync is retried as soon as the network comes back rather than after its back-off. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YSJh8YFmryHU41unFvrxSu --- lib/application/sync_coordinator.dart | 14 ++- lib/data/sync/api_client.dart | 26 ++++- lib/main.dart | 5 + lib/presentation/launch_hold.dart | 99 +++++++++++++++++++ lib/presentation/screens/activity_screen.dart | 3 +- .../screens/group_list_screen.dart | 3 +- lib/presentation/widgets/group_skeleton.dart | 38 +++---- .../widgets/sync_status_notice.dart | 63 ++++++++++-- test/application/sync_coordinator_test.dart | 29 ++++++ test/data/api_client_test.dart | 14 +++ test/presentation/launch_hold_test.dart | 51 ++++++++++ test/presentation/sync_refresh_test.dart | 33 +++++++ 12 files changed, 351 insertions(+), 27 deletions(-) create mode 100644 lib/presentation/launch_hold.dart create mode 100644 test/presentation/launch_hold_test.dart diff --git a/lib/application/sync_coordinator.dart b/lib/application/sync_coordinator.dart index 7dc64c1..c86f53d 100644 --- a/lib/application/sync_coordinator.dart +++ b/lib/application/sync_coordinator.dart @@ -4,6 +4,7 @@ import 'dart:math' as math; import 'package:flutter/foundation.dart'; +import '../data/sync/api_client.dart'; import '../data/sync/sync_engine.dart'; @immutable @@ -25,6 +26,10 @@ class SyncStatus { final DateTime? retryAt; Object? get error => lastReport?.error; + + /// Whether the last attempt failed because no server could be reached, + /// which is the ordinary state of an offline device rather than a fault. + bool get isOffline => isUnreachable(error); } /// Decides when to sync, runs one sync at a time, and reports status. @@ -72,7 +77,14 @@ class SyncCoordinator extends ChangeNotifier { _subscriptions ..add( _online.listen((isOnline) { - if (isOnline) resumed(); + if (!isOnline) return; + // A failed attempt is retried as soon as the network is back, not + // when its back-off ends or the usual gap has passed. + if (_status.error == null) { + resumed(); + } else { + unawaited(syncAll()); + } }), ) ..add(_writes.listen((_) => _writeQueued())); diff --git a/lib/data/sync/api_client.dart b/lib/data/sync/api_client.dart index c44b380..2b8f71c 100644 --- a/lib/data/sync/api_client.dart +++ b/lib/data/sync/api_client.dart @@ -28,7 +28,12 @@ api.OpensplitApi buildApiClient({ /// A request the server refused, or that never got an answer. class ApiFailure implements Exception { - const ApiFailure(this.message, {required this.retry, this.code}); + const ApiFailure( + this.message, { + required this.retry, + this.code, + this.unreachable = false, + }); /// Reads the `{error: {code, message, retry}}` envelope off a failed request. factory ApiFailure.from(DioException error) { @@ -54,6 +59,7 @@ class ApiFailure implements Exception { retry: status == null || status >= 500 ? api.Retry.transient : api.Retry.permanent, + unreachable: _neverAnswered(error), ); } @@ -63,6 +69,10 @@ class ApiFailure implements Exception { /// Null when no server answered in the expected shape. final api.ErrorCode? code; + /// Whether the request never reached a server that answered: the device is + /// offline, or the server is out of reach from where it is. + final bool unreachable; + static api.ErrorError? _decode(Map body) { try { return api.Error.fromJson(body).error; @@ -99,3 +109,17 @@ Future send(Future> request) async { throw ApiFailure.from(error); } } + +/// Whether [error] means no server answered at all, as opposed to a server +/// that answered and refused. +/// +/// Judged by the request itself rather than by the device's network +/// interfaces, which report a captive portal or a dead Wi-Fi as connected. +bool isUnreachable(Object? error) => switch (error) { + ApiFailure(:final unreachable) => unreachable, + DioException() => _neverAnswered(error), + _ => false, +}; + +bool _neverAnswered(DioException error) => + error.response == null && error.type != DioExceptionType.cancel; diff --git a/lib/main.dart b/lib/main.dart index 67c06ee..5748595 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -14,10 +14,15 @@ import 'application/preferences_providers.dart'; import 'config.dart'; import 'data/auth/session_store.dart'; import 'presentation/app.dart'; +import 'presentation/launch_hold.dart'; Future main() async { WidgetsFlutterBinding.ensureInitialized(); + // The splash stays up while the first screen reads what is saved, so a launch + // goes straight to the groups instead of through placeholder cards. + LaunchHold.begin(); + // google_fonts falls back to downloading a face it cannot find in the bundle. GoogleFonts.config.allowRuntimeFetching = false; diff --git a/lib/presentation/launch_hold.dart b/lib/presentation/launch_hold.dart new file mode 100644 index 0000000..0fb7d90 --- /dev/null +++ b/lib/presentation/launch_hold.dart @@ -0,0 +1,99 @@ +import 'dart:async'; + +import 'package:flutter/scheduler.dart'; +import 'package:material_ui/material_ui.dart'; + +/// Keeps the platform's launch screen up until the first screen can show what +/// is saved on this device, rather than a placeholder for it. +/// +/// Flutter takes down the Android splash, and fires the `flutter-first-frame` +/// event that removes the web loader, when it renders its first frame. [begin] +/// defers that frame. Widgets still build and lay out underneath, so each +/// screen's own queries run exactly as they would otherwise; only the pixels +/// wait. The frame is let through once a built frame contains no +/// [LaunchPlaceholder], or after a time limit, so a slow disk falls back to the +/// placeholder instead of a long splash. +/// +/// Only placeholders for local data take part. Anything waiting on the network +/// draws as usual, because a launch screen held on a connection is the slow, +/// online-only start this exists to avoid. +abstract final class LaunchHold { + static int _placeholders = 0; + static bool _holding = false; + static Timer? _limit; + + /// Whether the first frame is still being held back. + static bool get isHolding => _holding; + + /// Defers the first frame. Call once, before `runApp`. + /// + /// The [limit] is how long a launch may wait on the local database. Opening + /// it usually takes a fraction of that; the limit is for a cold disk or a + /// debug build. + static void begin({Duration limit = const Duration(milliseconds: 800)}) { + if (_holding) return; + _holding = true; + WidgetsBinding.instance.deferFirstFrame(); + _limit = Timer(limit, _release); + _checkAfterNextFrame(settled: false); + } + + /// Releases the frame once two frames in a row have had nothing to wait for. + /// + /// Two, because the router builds its first page a frame after the app + /// itself, and that page may be the one with a placeholder in it. + static void _checkAfterNextFrame({required bool settled}) { + SchedulerBinding.instance.addPostFrameCallback((_) { + if (!_holding) return; + if (_placeholders > 0) return _checkAfterNextFrame(settled: false); + if (settled) return _release(); + _checkAfterNextFrame(settled: true); + SchedulerBinding.instance.scheduleFrame(); + }); + } + + static void _release() { + if (!_holding) return; + _holding = false; + _limit?.cancel(); + _limit = null; + WidgetsBinding.instance.allowFirstFrame(); + } + + /// Forgets any hold, for tests that begin one. + @visibleForTesting + static void reset() { + _release(); + _placeholders = 0; + } +} + +/// Marks [child] as standing in for saved data that is still being read. +/// +/// While one of these is on screen, [LaunchHold] keeps the launch screen up. +/// After launch it has no effect. Works in both box and sliver positions. +class LaunchPlaceholder extends StatefulWidget { + const LaunchPlaceholder({super.key, required this.child}); + + final Widget child; + + @override + State createState() => _LaunchPlaceholderState(); +} + +class _LaunchPlaceholderState extends State { + @override + void initState() { + super.initState(); + LaunchHold._placeholders++; + } + + @override + void dispose() { + LaunchHold._placeholders--; + super.dispose(); + } + + @override + Widget build(BuildContext context) => widget.child; +} diff --git a/lib/presentation/screens/activity_screen.dart b/lib/presentation/screens/activity_screen.dart index 0e77b37..ee916e5 100644 --- a/lib/presentation/screens/activity_screen.dart +++ b/lib/presentation/screens/activity_screen.dart @@ -11,6 +11,7 @@ import '../../domain/models/group_event.dart'; import '../navigation.dart'; import '../widgets/empty_state.dart'; import '../widgets/page_body.dart'; +import '../widgets/sync_status_notice.dart'; /// What has happened to this group's expenses, and who did it. class ActivityScreen extends ConsumerWidget { @@ -33,7 +34,7 @@ class ActivityScreen extends ConsumerWidget { ), body: PageBody( child: switch (events) { - null => const Center(child: CircularProgressIndicator()), + null => const SavedDataLoading(label: 'Loading activity'), [] => const _Empty(), _ => ListView.separated( padding: const EdgeInsets.symmetric(vertical: 8), diff --git a/lib/presentation/screens/group_list_screen.dart b/lib/presentation/screens/group_list_screen.dart index 1271bab..1ab40d1 100644 --- a/lib/presentation/screens/group_list_screen.dart +++ b/lib/presentation/screens/group_list_screen.dart @@ -8,6 +8,7 @@ import '../../application/sync_providers.dart'; import '../../data/local/database.dart'; import '../../data/repositories/drift_group_repository.dart'; import '../../data/web/boot_hint.dart'; +import '../launch_hold.dart'; import '../widgets/avatar_view.dart'; import '../widgets/brand_mark.dart'; import '../widgets/conflicting_edit_banner.dart'; @@ -180,7 +181,7 @@ class _GroupTile extends ConsumerWidget { title: Text(group.name, overflow: TextOverflow.ellipsis), titleTextStyle: Theme.of(context).textTheme.titleMedium, subtitle: ledger == null - ? const SizedBox(height: 20) + ? const LaunchPlaceholder(child: SizedBox(height: 20)) : GroupStanding(ledger: ledger, currencies: currencies), ); } diff --git a/lib/presentation/widgets/group_skeleton.dart b/lib/presentation/widgets/group_skeleton.dart index f481f9f..d5e4c6f 100644 --- a/lib/presentation/widgets/group_skeleton.dart +++ b/lib/presentation/widgets/group_skeleton.dart @@ -1,6 +1,8 @@ import 'package:material_ui/material_ui.dart'; import 'package:flutter/widget_previews.dart'; +import '../launch_hold.dart'; + /// The group list as it looks before the local database has answered. class GroupListSkeleton extends StatefulWidget { const GroupListSkeleton({super.key, this.cards = 3}); @@ -44,23 +46,25 @@ class _GroupListSkeletonState extends State // turned off is exactly the kind of motion this setting exists to stop. final still = MediaQuery.disableAnimationsOf(context); - return SliverPadding( - padding: const EdgeInsets.fromLTRB(0, 8, 0, 96), - sliver: SliverList.separated( - itemCount: widget.cards, - separatorBuilder: (_, _) => const SizedBox(height: 8), - itemBuilder: (context, index) { - final (title, detail) = _widths[index % _widths.length]; - return SkeletonGroupCard( - breathe: _breathe, - cycles: _cycles, - // 0.15s apart at 1.8s a cycle. - phase: still ? 0 : (index % _widths.length) * (0.15 / 1.8), - animate: !still, - titleWidth: title, - detailWidth: detail, - ); - }, + return LaunchPlaceholder( + child: SliverPadding( + padding: const EdgeInsets.fromLTRB(0, 8, 0, 96), + sliver: SliverList.separated( + itemCount: widget.cards, + separatorBuilder: (_, _) => const SizedBox(height: 8), + itemBuilder: (context, index) { + final (title, detail) = _widths[index % _widths.length]; + return SkeletonGroupCard( + breathe: _breathe, + cycles: _cycles, + // 0.15s apart at 1.8s a cycle. + phase: still ? 0 : (index % _widths.length) * (0.15 / 1.8), + animate: !still, + titleWidth: title, + detailWidth: detail, + ); + }, + ), ), ); } diff --git a/lib/presentation/widgets/sync_status_notice.dart b/lib/presentation/widgets/sync_status_notice.dart index 8a966e3..651cba2 100644 --- a/lib/presentation/widgets/sync_status_notice.dart +++ b/lib/presentation/widgets/sync_status_notice.dart @@ -3,6 +3,7 @@ import 'package:flutter/widget_previews.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../application/sync_providers.dart'; +import '../launch_hold.dart'; /// Indicates that the local database has not produced its first result yet. class SavedDataLoading extends StatelessWidget { @@ -12,10 +13,12 @@ class SavedDataLoading extends StatelessWidget { final String label; @override - Widget build(BuildContext context) => Center( - child: Padding( - padding: const EdgeInsets.all(32), - child: CircularProgressIndicator(semanticsLabel: label), + Widget build(BuildContext context) => LaunchPlaceholder( + child: Center( + child: Padding( + padding: const EdgeInsets.all(32), + child: CircularProgressIndicator(semanticsLabel: label), + ), ), ); } @@ -77,6 +80,10 @@ class InitialSyncProgress extends ConsumerWidget } /// Keeps saved data visible while explaining a failed refresh or upload. +/// +/// Being offline gets a single quiet line rather than a card: for an app that +/// works offline it is a normal state, not a problem to act on, and it fixes +/// itself when the network comes back. class SyncStatusBanner extends ConsumerWidget { const SyncStatusBanner({super.key, this.padding = EdgeInsets.zero}); @@ -86,10 +93,16 @@ class SyncStatusBanner extends ConsumerWidget { Widget build(BuildContext context, WidgetRef ref) { final status = ref.watch(syncControllerProvider); final refreshFailed = status.error != null; - if (!status.enabled || - (!refreshFailed && status.lastReport?.nextPushAt == null)) { + final changesWaiting = status.lastReport?.nextPushAt != null; + if (!status.enabled || (!refreshFailed && !changesWaiting)) { return const SizedBox.shrink(); } + if (status.isOffline) { + return Padding( + padding: padding, + child: _OfflineNote(changesWaiting: changesWaiting), + ); + } return Padding( padding: padding, child: _SyncProblem( @@ -120,6 +133,39 @@ class _CheckingGroups extends StatelessWidget { ); } +class _OfflineNote extends StatelessWidget { + const _OfflineNote({required this.changesWaiting}); + + final bool changesWaiting; + + @override + Widget build(BuildContext context) { + final theme = Theme.of(context); + final color = theme.colorScheme.onSurfaceVariant; + return Semantics( + liveRegion: true, + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 4), + child: Row( + children: [ + Icon(Icons.cloud_off_outlined, size: 18, color: color), + const SizedBox(width: 8), + Expanded( + child: Text( + changesWaiting + ? 'Offline. Your changes will sync when you are back ' + 'online.' + : 'Offline. Showing what is saved on this device.', + style: theme.textTheme.bodySmall?.copyWith(color: color), + ), + ), + ], + ), + ), + ); + } +} + class _SyncProblem extends StatelessWidget { const _SyncProblem({ required this.title, @@ -165,6 +211,11 @@ Widget syncFailurePreview() => MaterialApp( ), ); +/// Previews the quiet line shown while the device is offline. +@Preview(name: 'Offline', group: 'Sync', size: Size(360, 80)) +Widget offlinePreview() => + const MaterialApp(home: Scaffold(body: _OfflineNote(changesWaiting: true))); + /// Previews the local database loading state before any saved rows are ready. @Preview(name: 'Opening saved groups', group: 'Sync', size: Size(360, 240)) Widget savedDataLoadingPreview() => const MaterialApp( diff --git a/test/application/sync_coordinator_test.dart b/test/application/sync_coordinator_test.dart index 9586ff6..6ecf54a 100644 --- a/test/application/sync_coordinator_test.dart +++ b/test/application/sync_coordinator_test.dart @@ -218,6 +218,35 @@ void main() { expect(coordinator.status.retryAt, isNull); }); + testWidgets('a failed attempt is retried as soon as the network returns', ( + tester, + ) async { + final online = StreamController(); + addTearDown(online.close); + var fullRuns = 0; + final coordinator = SyncCoordinator( + syncAll: () async => ++fullRuns == 1 ? _failed : _clean, + syncGroups: (_) async => _clean, + online: online.stream, + ); + addTearDown(coordinator.dispose); + + coordinator.start(); + await tester.pump(); + expect(coordinator.status.error, isNotNull); + + // Well inside both the back-off and the gap between automatic syncs. + online.add(true); + await tester.pump(); + expect(fullRuns, 2); + expect(coordinator.status.error, isNull); + + // A healthy device coming back online still respects the gap. + online.add(true); + await tester.pump(); + expect(fullRuns, 2); + }); + testWidgets('permanent refusals do not schedule automatic retries', ( tester, ) async { diff --git a/test/data/api_client_test.dart b/test/data/api_client_test.dart index f57702b..7c6eb5d 100644 --- a/test/data/api_client_test.dart +++ b/test/data/api_client_test.dart @@ -31,6 +31,20 @@ void main() { expect(failure.message, 'Something went wrong.'); }); + test('a request no server answered reads as unreachable', () { + final request = RequestOptions(path: '/api/groups/g'); + final offline = DioException( + requestOptions: request, + type: DioExceptionType.connectionError, + ); + + expect(ApiFailure.from(offline).unreachable, isTrue); + expect(isUnreachable(ApiFailure.from(offline)), isTrue); + expect(isUnreachable(offline), isTrue); + expect(isUnreachable(ApiFailure.from(_refusal(500, 'transient'))), false); + expect(isUnreachable(StateError('not a request')), isFalse); + }); + test('a refusal keeps the retry the server gave it', () { expect(ApiFailure.from(_refusal(409, 'stale')).retry, api.Retry.stale); expect( diff --git a/test/presentation/launch_hold_test.dart b/test/presentation/launch_hold_test.dart new file mode 100644 index 0000000..e91b31e --- /dev/null +++ b/test/presentation/launch_hold_test.dart @@ -0,0 +1,51 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:material_ui/material_ui.dart'; +import 'package:opensplit/presentation/launch_hold.dart'; + +void main() { + tearDown(LaunchHold.reset); + + testWidgets('holds the first frame until saved data replaces placeholders', ( + tester, + ) async { + final loaded = ValueNotifier(false); + addTearDown(loaded.dispose); + + LaunchHold.begin(limit: const Duration(seconds: 5)); + await tester.pumpWidget( + ValueListenableBuilder( + valueListenable: loaded, + builder: (context, isLoaded, _) => isLoaded + ? const Text('Saved home group', textDirection: TextDirection.ltr) + : const LaunchPlaceholder(child: SizedBox()), + ), + ); + await tester.pump(); + expect(LaunchHold.isHolding, isTrue); + + loaded.value = true; + await tester.pump(); + await tester.pump(); + expect(LaunchHold.isHolding, isFalse); + }); + + testWidgets('releases a screen with nothing to wait for', (tester) async { + LaunchHold.begin(limit: const Duration(seconds: 5)); + await tester.pumpWidget( + const Text('Welcome', textDirection: TextDirection.ltr), + ); + await tester.pump(); + expect(LaunchHold.isHolding, isFalse); + }); + + testWidgets('gives up at the limit rather than holding the splash', ( + tester, + ) async { + LaunchHold.begin(limit: const Duration(milliseconds: 800)); + await tester.pumpWidget(const LaunchPlaceholder(child: SizedBox())); + await tester.pump(const Duration(milliseconds: 799)); + expect(LaunchHold.isHolding, isTrue); + await tester.pump(const Duration(milliseconds: 1)); + expect(LaunchHold.isHolding, isFalse); + }); +} diff --git a/test/presentation/sync_refresh_test.dart b/test/presentation/sync_refresh_test.dart index 9e86ef8..2ed1a4c 100644 --- a/test/presentation/sync_refresh_test.dart +++ b/test/presentation/sync_refresh_test.dart @@ -11,6 +11,8 @@ import 'package:opensplit/application/sync_coordinator.dart'; import 'package:opensplit/application/sync_providers.dart'; import 'package:opensplit/data/local/database.dart'; import 'package:opensplit/data/repositories/drift_group_repository.dart'; +import 'package:dio/dio.dart' + show DioException, DioExceptionType, RequestOptions; import 'package:opensplit/data/sync/sync_engine.dart'; import 'package:opensplit/presentation/screens/group_detail_screen.dart'; import 'package:opensplit/presentation/screens/group_list_screen.dart'; @@ -243,6 +245,37 @@ void main() { expect(find.byType(LinearProgressIndicator), findsNothing); }); + testWidgets('offline is a quiet line, not a problem card', (tester) async { + final sync = _TestSync(); + await _mount(tester, sync, const GroupListScreen()); + await tester.pumpAndSettle(); + + sync.show( + SyncStatus( + hasCompletedFullSync: true, + lastReport: SyncReport( + pushed: 0, + pulled: 0, + failed: 0, + error: DioException( + requestOptions: RequestOptions(path: '/api/sync'), + type: DioExceptionType.connectionError, + ), + nextPushAt: DateTime.utc(2026, 10, 10), + ), + ), + ); + await tester.pumpAndSettle(); + expect(find.text(_group.name), findsOneWidget); + expect( + find.text('Offline. Your changes will sync when you are back online.'), + findsOneWidget, + ); + expect(find.text('Could not refresh'), findsNothing); + expect(find.text('Changes waiting to sync'), findsNothing); + expect(find.text('Try again'), findsNothing); + }); + testWidgets('the group list keeps its saved rows through refresh and error', ( tester, ) async { From e4be442dd5e31e01d95a7cd6b54a87786eeac287 Mon Sep 17 00:00:00 2001 From: Seenu K Date: Sat, 10 Oct 2026 10:51:49 +0530 Subject: [PATCH 03/10] refactor: one launch wait, and an app bar icon for offline The splash (Android, and now a matching one on the web) stays up until the landing screen has real data, so a launch is one wait instead of a loader, then placeholders, then content. - web/index.html draws only the brand mark, replacing the HTML skeleton of app bar, rail, navigation bar and cards; the boot hints it read and GroupListSkeleton go with it. - Local loading states draw nothing. LaunchPlaceholder stays as the signal that holds the splash. - The launch limit is now a 10s safety net against a hung query, not an 800ms tuning value. - Offline is a cloud-off icon in the group list and group app bars, with a tooltip, instead of a line in the content. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YSJh8YFmryHU41unFvrxSu --- lib/data/web/boot_hint.dart | 4 - lib/data/web/boot_hint_stub.dart | 4 - lib/data/web/boot_hint_web.dart | 33 - lib/main.dart | 2 +- lib/presentation/app.dart | 7 - lib/presentation/launch_hold.dart | 15 +- lib/presentation/router.dart | 6 +- .../screens/group_detail_screen.dart | 2 + .../screens/group_list_screen.dart | 21 +- lib/presentation/widgets/group_skeleton.dart | 195 ------ .../widgets/offline_indicator.dart | 56 ++ .../widgets/sync_status_notice.dart | 72 +-- test/presentation/sync_refresh_test.dart | 28 +- test/presentation/theme_test.dart | 163 +---- web/index.html | 599 +----------------- 15 files changed, 139 insertions(+), 1068 deletions(-) delete mode 100644 lib/data/web/boot_hint.dart delete mode 100644 lib/data/web/boot_hint_stub.dart delete mode 100644 lib/data/web/boot_hint_web.dart delete mode 100644 lib/presentation/widgets/group_skeleton.dart create mode 100644 lib/presentation/widgets/offline_indicator.dart diff --git a/lib/data/web/boot_hint.dart b/lib/data/web/boot_hint.dart deleted file mode 100644 index cc616f1..0000000 --- a/lib/data/web/boot_hint.dart +++ /dev/null @@ -1,4 +0,0 @@ -/// What the web loader is allowed to assume before any Dart has run. -library; - -export 'boot_hint_stub.dart' if (dart.library.js_interop) 'boot_hint_web.dart'; diff --git a/lib/data/web/boot_hint_stub.dart b/lib/data/web/boot_hint_stub.dart deleted file mode 100644 index ef42328..0000000 --- a/lib/data/web/boot_hint_stub.dart +++ /dev/null @@ -1,4 +0,0 @@ -/// Android has no pre-engine loader to inform, so these do nothing. -void recordHasGroups(bool hasGroups) {} - -void recordSignedIn(bool signedIn) {} diff --git a/lib/data/web/boot_hint_web.dart b/lib/data/web/boot_hint_web.dart deleted file mode 100644 index 5deb4ce..0000000 --- a/lib/data/web/boot_hint_web.dart +++ /dev/null @@ -1,33 +0,0 @@ -import 'package:web/web.dart' as web; - -/// The key `web/index.html` reads to decide whether to draw group cards. -const _groupsKey = 'opensplit.hasGroups'; - -/// The key `web/index.html` reads to decide whether to draw the app at all. -const _sessionKey = 'opensplit.signedIn'; - -/// Tells the next cold start whether this browser has any groups to show. -void recordHasGroups(bool hasGroups) { - _write(_groupsKey, hasGroups); -} - -/// Tells the next cold start whether to draw the app or the welcome screen. -void recordSignedIn(bool signedIn) { - _write(_sessionKey, signedIn); - if (!signedIn) _write(_groupsKey, false); -} - -/// localStorage is the only store readable synchronously on the first frame of -/// the document. Losing it costs nothing: every key here is absent by default -/// and the loader's fallback is the quieter drawing, not the louder one. -void _write(String key, bool value) { - try { - if (value) { - web.window.localStorage.setItem(key, '1'); - } else { - web.window.localStorage.removeItem(key); - } - } catch (_) { - // Private mode, or site data blocked. The skeleton has a safe default. - } -} diff --git a/lib/main.dart b/lib/main.dart index 5748595..7591e98 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -20,7 +20,7 @@ Future main() async { WidgetsFlutterBinding.ensureInitialized(); // The splash stays up while the first screen reads what is saved, so a launch - // goes straight to the groups instead of through placeholder cards. + // is one wait that ends on real content. LaunchHold.begin(); // google_fonts falls back to downloading a face it cannot find in the bundle. diff --git a/lib/presentation/app.dart b/lib/presentation/app.dart index 3136cd3..b0b741a 100644 --- a/lib/presentation/app.dart +++ b/lib/presentation/app.dart @@ -12,7 +12,6 @@ import '../application/router_provider.dart'; import '../application/session_providers.dart'; import '../application/sync_providers.dart'; import '../data/platform/app_update_service.dart'; -import '../data/web/boot_hint.dart'; import '../data/web/release_updates.dart'; import '../domain/auth_service.dart'; import '../l10n/app_localizations.dart'; @@ -66,12 +65,6 @@ class _OpenSplitAppState extends ConsumerState { // Built for each account, which starts its automatic sync. ref.listenManual(syncControllerProvider, (_, _) {}, fireImmediately: true); - ref.listenManual(signedInProvider, (_, signedIn) { - // Leaves a note for the next cold start, so the web loader draws the - // layout this session will actually land on. - recordSignedIn(signedIn); - }, fireImmediately: true); - // A Google flow that left the page finishes here, on the one launch that is // a return from it. unawaited(_finishIdentityRedirect()); diff --git a/lib/presentation/launch_hold.dart b/lib/presentation/launch_hold.dart index 0fb7d90..7fb34c4 100644 --- a/lib/presentation/launch_hold.dart +++ b/lib/presentation/launch_hold.dart @@ -7,12 +7,11 @@ import 'package:material_ui/material_ui.dart'; /// is saved on this device, rather than a placeholder for it. /// /// Flutter takes down the Android splash, and fires the `flutter-first-frame` -/// event that removes the web loader, when it renders its first frame. [begin] +/// event that removes the web splash, when it renders its first frame. [begin] /// defers that frame. Widgets still build and lay out underneath, so each /// screen's own queries run exactly as they would otherwise; only the pixels /// wait. The frame is let through once a built frame contains no -/// [LaunchPlaceholder], or after a time limit, so a slow disk falls back to the -/// placeholder instead of a long splash. +/// [LaunchPlaceholder], so a launch is one wait however long the disk takes. /// /// Only placeholders for local data take part. Anything waiting on the network /// draws as usual, because a launch screen held on a connection is the slow, @@ -27,10 +26,12 @@ abstract final class LaunchHold { /// Defers the first frame. Call once, before `runApp`. /// - /// The [limit] is how long a launch may wait on the local database. Opening - /// it usually takes a fraction of that; the limit is for a cold disk or a - /// debug build. - static void begin({Duration limit = const Duration(milliseconds: 800)}) { + /// The [limit] is not a tuning knob. It is there so that a local query that + /// never answers shows the app, with its navigation and settings, instead of + /// a splash that cannot be told apart from a hang. It is longer than SQLite's + /// own five-second busy timeout, so a locked database reports its own error + /// first. + static void begin({Duration limit = const Duration(seconds: 10)}) { if (_holding) return; _holding = true; WidgetsBinding.instance.deferFirstFrame(); diff --git a/lib/presentation/router.dart b/lib/presentation/router.dart index b52d9ab..d78f744 100644 --- a/lib/presentation/router.dart +++ b/lib/presentation/router.dart @@ -234,7 +234,7 @@ class AdaptiveNavigation extends StatelessWidget { /// Material 3's own default destination width, and the rail's width outright: /// a destination is padded 8dp either side *within* this, and only pushes /// past it if a label needs more. None of the three labels is anywhere near, - /// so the rail is a fixed 80dp and the skeleton can count on it. + /// so the rail is a fixed 80dp. static const double _railWidth = 80; final StatefulNavigationShell shell; @@ -262,10 +262,6 @@ class AdaptiveNavigation extends StatelessWidget { NavigationRail( selectedIndex: shell.currentIndex, labelType: NavigationRailLabelType.all, - // Material's own default, stated rather than inherited because the - // web loading skeleton draws a rail of exactly this width before - // Flutter starts — see the 840px block in web/index.html, and the - // test that holds the two numbers together. minWidth: _railWidth, onDestinationSelected: _select, destinations: [ diff --git a/lib/presentation/screens/group_detail_screen.dart b/lib/presentation/screens/group_detail_screen.dart index 1333325..e23d15c 100644 --- a/lib/presentation/screens/group_detail_screen.dart +++ b/lib/presentation/screens/group_detail_screen.dart @@ -21,6 +21,7 @@ import '../widgets/empty_state.dart'; import '../widgets/group_cover.dart'; import '../widgets/group_pane.dart'; import '../widgets/group_standing.dart'; +import '../widgets/offline_indicator.dart'; import '../widgets/page_body.dart'; import '../widgets/segmented_list.dart'; import '../widgets/sync_refresh_button.dart'; @@ -145,6 +146,7 @@ class _GroupAppBar extends StatelessWidget { leading: BackButton(onPressed: () => goBack(context, '/')), title: Text(ledger.group.name), actions: [ + const OfflineIndicator(), if (kIsWeb) SyncRefreshButton.group(groupId), IconButton( tooltip: 'Settle up', diff --git a/lib/presentation/screens/group_list_screen.dart b/lib/presentation/screens/group_list_screen.dart index 1ab40d1..2db7d50 100644 --- a/lib/presentation/screens/group_list_screen.dart +++ b/lib/presentation/screens/group_list_screen.dart @@ -7,16 +7,15 @@ import '../../application/ledger_providers.dart'; import '../../application/sync_providers.dart'; import '../../data/local/database.dart'; import '../../data/repositories/drift_group_repository.dart'; -import '../../data/web/boot_hint.dart'; import '../launch_hold.dart'; import '../widgets/avatar_view.dart'; import '../widgets/brand_mark.dart'; import '../widgets/conflicting_edit_banner.dart'; import '../widgets/create_group_sheet.dart'; import '../widgets/empty_state.dart'; -import '../widgets/group_skeleton.dart'; import '../widgets/group_standing.dart'; import '../widgets/link_account_prompt.dart'; +import '../widgets/offline_indicator.dart'; import '../widgets/page_body.dart'; import '../widgets/pull_to_sync.dart'; import '../widgets/segmented_list.dart'; @@ -35,15 +34,6 @@ class GroupListScreen extends ConsumerWidget { // where. final source = groupListingsProvider; - // Leaves a note for the next cold start, so the web loader knows whether to - // draw group cards or just the chrome. - ref.listen(source, (_, next) { - final loaded = next.value; - if (loaded != null) { - recordHasGroups(loaded.any((listing) => listing.isCurrent)); - } - }); - final groupsAsync = ref.watch(source); final all = groupsAsync.value ?? const []; final groups = [ @@ -54,7 +44,10 @@ class GroupListScreen extends ConsumerWidget { return DestinationScaffold( titleWidget: const BrandLockup(), - actions: [if (kIsWeb) const SyncRefreshButton.everything()], + actions: [ + const OfflineIndicator(), + if (kIsWeb) const SyncRefreshButton.everything(), + ], // The empty state is shown at once rather than held back until the // server confirms it, so an empty list says when it may yet fill. bottom: groups.isEmpty @@ -85,7 +78,9 @@ class GroupListScreen extends ConsumerWidget { groups: groups, archivedCount: archived, ), - _ => const [GroupListSkeleton()], + // Nothing, rather than placeholder cards: at launch the splash covers + // this, and afterwards a local read is too quick to be worth drawing. + _ => const [LaunchPlaceholder(child: SliverToBoxAdapter())], }, ); } diff --git a/lib/presentation/widgets/group_skeleton.dart b/lib/presentation/widgets/group_skeleton.dart deleted file mode 100644 index d5e4c6f..0000000 --- a/lib/presentation/widgets/group_skeleton.dart +++ /dev/null @@ -1,195 +0,0 @@ -import 'package:material_ui/material_ui.dart'; -import 'package:flutter/widget_previews.dart'; - -import '../launch_hold.dart'; - -/// The group list as it looks before the local database has answered. -class GroupListSkeleton extends StatefulWidget { - const GroupListSkeleton({super.key, this.cards = 3}); - - /// Enough to fill a phone without implying a number. - final int cards; - - @override - State createState() => _GroupListSkeletonState(); -} - -class _GroupListSkeletonState extends State - with SingleTickerProviderStateMixin { - /// How many times the bars breathe before coming to rest. - static const _cycles = 5; - - /// One controller for every bar on screen rather than one per card. The - /// per-card offset is applied to the value, not to the clock, which is what - /// CSS `animation-delay` does and costs two tickers fewer. - late final AnimationController _breathe = AnimationController( - vsync: this, - duration: const Duration(milliseconds: 1800 * _cycles), - )..forward(); - - @override - void dispose() { - _breathe.dispose(); - super.dispose(); - } - - /// The title and detail widths, per card. - static const _widths = <(double, double)>[ - (0.62, 0.40), - (0.44, 0.56), - (0.55, 0.40), - ]; - - @override - Widget build(BuildContext context) { - // Honoured for the same reason the CSS honours it: a pulse that cannot be - // turned off is exactly the kind of motion this setting exists to stop. - final still = MediaQuery.disableAnimationsOf(context); - - return LaunchPlaceholder( - child: SliverPadding( - padding: const EdgeInsets.fromLTRB(0, 8, 0, 96), - sliver: SliverList.separated( - itemCount: widget.cards, - separatorBuilder: (_, _) => const SizedBox(height: 8), - itemBuilder: (context, index) { - final (title, detail) = _widths[index % _widths.length]; - return SkeletonGroupCard( - breathe: _breathe, - cycles: _cycles, - // 0.15s apart at 1.8s a cycle. - phase: still ? 0 : (index % _widths.length) * (0.15 / 1.8), - animate: !still, - titleWidth: title, - detailWidth: detail, - ); - }, - ), - ), - ); - } -} - -/// One card of the skeleton. -class SkeletonGroupCard extends StatelessWidget { - const SkeletonGroupCard({ - super.key, - required this.breathe, - required this.cycles, - required this.phase, - required this.animate, - required this.titleWidth, - required this.detailWidth, - }); - - final Animation breathe; - final int cycles; - final double phase; - final bool animate; - final double titleWidth; - final double detailWidth; - - @override - Widget build(BuildContext context) { - final theme = Theme.of(context); - final scheme = theme.colorScheme; - - return ExcludeSemantics( - child: Card.outlined( - clipBehavior: Clip.antiAlias, - child: ListTile( - // Not a placeholder: there is no version of this row without an - // avatar, so it is drawn in its final colour. - leading: CircleAvatar(backgroundColor: scheme.secondaryContainer), - title: _Line( - style: theme.textTheme.titleMedium, - barHeight: 16, - widthFactor: titleWidth, - breathe: breathe, - cycles: cycles, - phase: phase, - animate: animate, - ), - subtitle: _Line( - style: theme.textTheme.bodyMedium, - barHeight: 12, - widthFactor: detailWidth, - breathe: breathe, - cycles: cycles, - phase: phase, - animate: animate, - ), - ), - ), - ); - } -} - -/// A bar standing in for a line of text. -class _Line extends StatelessWidget { - const _Line({ - required this.style, - required this.barHeight, - required this.widthFactor, - required this.breathe, - required this.cycles, - required this.phase, - required this.animate, - }); - - final TextStyle? style; - final double barHeight; - final double widthFactor; - final Animation breathe; - final int cycles; - final double phase; - final bool animate; - - @override - Widget build(BuildContext context) { - final scheme = Theme.of(context).colorScheme; - - return Stack( - children: [ - // Reserves the line box. Never painted, never read aloud. - Opacity(opacity: 0, child: Text('​', style: style)), - Positioned.fill( - child: Align( - alignment: Alignment.centerLeft, - child: AnimatedBuilder( - animation: breathe, - builder: (context, child) { - final done = breathe.isCompleted; - final t = (breathe.value * cycles + phase) % 1; - // 0%,100% -> 1; 50% -> 0.45, eased both ways. - final curved = Curves.easeInOut.transform( - t < 0.5 ? t * 2 : (1 - t) * 2, - ); - // Rests at full opacity rather than wherever the last frame - // happened to land, which for the offset cards is not 1. - final opacity = (animate && !done) ? 1 - (0.55 * curved) : 1.0; - return Opacity(opacity: opacity, child: child); - }, - child: FractionallySizedBox( - widthFactor: widthFactor, - child: Container( - height: barHeight, - decoration: BoxDecoration( - color: scheme.surfaceContainerHighest, - borderRadius: BorderRadius.circular(4), - ), - ), - ), - ), - ), - ), - ], - ); - } -} - -/// Previews the skeleton beside what it stands in for. -@Preview(name: 'Group list skeleton', group: 'Empty', size: Size(360, 320)) -Widget groupListSkeletonPreview() => const MaterialApp( - home: Scaffold(body: CustomScrollView(slivers: [GroupListSkeleton()])), -); diff --git a/lib/presentation/widgets/offline_indicator.dart b/lib/presentation/widgets/offline_indicator.dart new file mode 100644 index 0000000..e45fdf4 --- /dev/null +++ b/lib/presentation/widgets/offline_indicator.dart @@ -0,0 +1,56 @@ +import 'package:flutter/widget_previews.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:material_ui/material_ui.dart'; + +import '../../application/sync_providers.dart'; + +/// A cloud-off icon for the app bar while no server can be reached. +/// +/// Tapping or hovering it says what that means. It goes away by itself once a +/// sync gets through, which the coordinator tries as soon as the network +/// returns. +class OfflineIndicator extends ConsumerWidget { + const OfflineIndicator({super.key}); + + @override + Widget build(BuildContext context, WidgetRef ref) { + final status = ref.watch(syncControllerProvider); + if (!status.enabled || !status.isOffline) return const SizedBox.shrink(); + return _OfflineIcon(changesWaiting: status.lastReport?.nextPushAt != null); + } +} + +class _OfflineIcon extends StatelessWidget { + const _OfflineIcon({required this.changesWaiting}); + + final bool changesWaiting; + + @override + Widget build(BuildContext context) => Tooltip( + message: changesWaiting + ? 'Offline. Your changes are saved on this device and will sync when ' + 'you are back online.' + : 'Offline. Showing what is saved on this device.', + // A tap, not a long press, because there is nothing else a tap could do. + triggerMode: TooltipTriggerMode.tap, + child: Padding( + // The 48dp an IconButton would occupy, so the actions stay aligned. + padding: const EdgeInsets.all(12), + child: Icon( + Icons.cloud_off_outlined, + color: Theme.of(context).colorScheme.onSurfaceVariant, + ), + ), + ); +} + +/// Previews the indicator without a backend. +@Preview(name: 'Offline', group: 'Sync', size: Size(360, 80)) +Widget offlineIndicatorPreview() => MaterialApp( + home: Scaffold( + appBar: AppBar( + title: const Text('Groups'), + actions: const [_OfflineIcon(changesWaiting: true)], + ), + ), +); diff --git a/lib/presentation/widgets/sync_status_notice.dart b/lib/presentation/widgets/sync_status_notice.dart index 651cba2..6ab0970 100644 --- a/lib/presentation/widgets/sync_status_notice.dart +++ b/lib/presentation/widgets/sync_status_notice.dart @@ -5,21 +5,20 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import '../../application/sync_providers.dart'; import '../launch_hold.dart'; -/// Indicates that the local database has not produced its first result yet. +/// Stands in, invisibly, for saved content the local database has not +/// returned yet. +/// +/// At launch the splash covers it (see [LaunchHold]); after launch a local +/// read finishes before a spinner would be worth drawing. class SavedDataLoading extends StatelessWidget { const SavedDataLoading({super.key, required this.label}); - /// The saved content being opened, distinct from a network refresh. + /// The saved content being opened, for screen readers. final String label; @override Widget build(BuildContext context) => LaunchPlaceholder( - child: Center( - child: Padding( - padding: const EdgeInsets.all(32), - child: CircularProgressIndicator(semanticsLabel: label), - ), - ), + child: Semantics(label: label, child: const SizedBox.expand()), ); } @@ -81,9 +80,9 @@ class InitialSyncProgress extends ConsumerWidget /// Keeps saved data visible while explaining a failed refresh or upload. /// -/// Being offline gets a single quiet line rather than a card: for an app that -/// works offline it is a normal state, not a problem to act on, and it fixes -/// itself when the network comes back. +/// Says nothing while offline: for an app that works offline that is a normal +/// state, not a problem to act on, and [OfflineIndicator] in the app bar +/// already says it. class SyncStatusBanner extends ConsumerWidget { const SyncStatusBanner({super.key, this.padding = EdgeInsets.zero}); @@ -97,12 +96,7 @@ class SyncStatusBanner extends ConsumerWidget { if (!status.enabled || (!refreshFailed && !changesWaiting)) { return const SizedBox.shrink(); } - if (status.isOffline) { - return Padding( - padding: padding, - child: _OfflineNote(changesWaiting: changesWaiting), - ); - } + if (status.isOffline) return const SizedBox.shrink(); return Padding( padding: padding, child: _SyncProblem( @@ -133,39 +127,6 @@ class _CheckingGroups extends StatelessWidget { ); } -class _OfflineNote extends StatelessWidget { - const _OfflineNote({required this.changesWaiting}); - - final bool changesWaiting; - - @override - Widget build(BuildContext context) { - final theme = Theme.of(context); - final color = theme.colorScheme.onSurfaceVariant; - return Semantics( - liveRegion: true, - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 4), - child: Row( - children: [ - Icon(Icons.cloud_off_outlined, size: 18, color: color), - const SizedBox(width: 8), - Expanded( - child: Text( - changesWaiting - ? 'Offline. Your changes will sync when you are back ' - 'online.' - : 'Offline. Showing what is saved on this device.', - style: theme.textTheme.bodySmall?.copyWith(color: color), - ), - ), - ], - ), - ), - ); - } -} - class _SyncProblem extends StatelessWidget { const _SyncProblem({ required this.title, @@ -210,14 +171,3 @@ Widget syncFailurePreview() => MaterialApp( ), ), ); - -/// Previews the quiet line shown while the device is offline. -@Preview(name: 'Offline', group: 'Sync', size: Size(360, 80)) -Widget offlinePreview() => - const MaterialApp(home: Scaffold(body: _OfflineNote(changesWaiting: true))); - -/// Previews the local database loading state before any saved rows are ready. -@Preview(name: 'Opening saved groups', group: 'Sync', size: Size(360, 240)) -Widget savedDataLoadingPreview() => const MaterialApp( - home: Scaffold(body: SavedDataLoading(label: 'Loading saved groups')), -); diff --git a/test/presentation/sync_refresh_test.dart b/test/presentation/sync_refresh_test.dart index 2ed1a4c..9133193 100644 --- a/test/presentation/sync_refresh_test.dart +++ b/test/presentation/sync_refresh_test.dart @@ -16,7 +16,8 @@ import 'package:dio/dio.dart' import 'package:opensplit/data/sync/sync_engine.dart'; import 'package:opensplit/presentation/screens/group_detail_screen.dart'; import 'package:opensplit/presentation/screens/group_list_screen.dart'; -import 'package:opensplit/presentation/widgets/group_skeleton.dart'; +import 'package:opensplit/presentation/launch_hold.dart'; +import 'package:opensplit/presentation/widgets/offline_indicator.dart'; import 'package:opensplit/presentation/widgets/pull_to_sync.dart'; import 'package:opensplit/presentation/widgets/sync_refresh_button.dart'; import 'package:opensplit/presentation/widgets/sync_status_notice.dart'; @@ -175,7 +176,7 @@ void main() { await tester.pumpAndSettle(); }); - testWidgets('opening saved groups has a loading state, not a blank body', ( + testWidgets('opening saved groups holds the launch, not a placeholder', ( tester, ) async { final groups = StreamController>(); @@ -186,16 +187,14 @@ void main() { const GroupListScreen(), groups: groups.stream, ); - // The skeleton, not a spinner and not a blank body: the point of the - // assertion is that something group-shaped is on screen before the database - // has answered, which is what the web loader has already been drawing for - // the whole of the engine download. - expect(find.byType(GroupListSkeleton), findsOneWidget); + // Nothing group-shaped is drawn while the database has not answered: the + // splash covers that wait, and this marker is what keeps it up. + expect(find.byType(LaunchPlaceholder, skipOffstage: false), findsOneWidget); expect(find.text('No groups yet'), findsNothing); groups.add([_group]); await tester.pumpAndSettle(); expect(find.text(_group.name), findsOneWidget); - expect(find.byType(GroupListSkeleton), findsNothing); + expect(find.byType(LaunchPlaceholder, skipOffstage: false), findsNothing); }); testWidgets('an empty list shows at once while the first refresh runs', ( @@ -245,7 +244,7 @@ void main() { expect(find.byType(LinearProgressIndicator), findsNothing); }); - testWidgets('offline is a quiet line, not a problem card', (tester) async { + testWidgets('offline is an app bar icon, not a problem card', (tester) async { final sync = _TestSync(); await _mount(tester, sync, const GroupListScreen()); await tester.pumpAndSettle(); @@ -267,13 +266,22 @@ void main() { ); await tester.pumpAndSettle(); expect(find.text(_group.name), findsOneWidget); + expect(find.byType(OfflineIndicator), findsOneWidget); + expect(find.byIcon(Icons.cloud_off_outlined), findsOneWidget); expect( - find.text('Offline. Your changes will sync when you are back online.'), + find.byTooltip( + 'Offline. Your changes are saved on this device and will sync when ' + 'you are back online.', + ), findsOneWidget, ); expect(find.text('Could not refresh'), findsNothing); expect(find.text('Changes waiting to sync'), findsNothing); expect(find.text('Try again'), findsNothing); + + sync.finish(); + await tester.pumpAndSettle(); + expect(find.byIcon(Icons.cloud_off_outlined), findsNothing); }); testWidgets('the group list keeps its saved rows through refresh and error', ( diff --git a/test/presentation/theme_test.dart b/test/presentation/theme_test.dart index 37a9aa4..81b7380 100644 --- a/test/presentation/theme_test.dart +++ b/test/presentation/theme_test.dart @@ -5,7 +5,6 @@ import 'dart:math' as math; import 'package:material_ui/material_ui.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:opensplit/presentation/theme.dart'; -import 'package:opensplit/presentation/widgets/group_skeleton.dart'; /// Relative luminance, per WCAG 2.1. double _luminance(Color c) { @@ -78,150 +77,8 @@ void main() { }); }); - group('the web loading skeleton', () { - test('uses the theme\'s own colours', _skeletonMatchesTheme); - - testWidgets('uses the theme\'s own type', (tester) async { - late TextTheme resolved; - await tester.pumpWidget( - MaterialApp( - theme: buildTheme(Brightness.light), - // Text geometry is applied by Theme.of, not by the ThemeData - // constructor, so the real sizes only exist inside a MaterialApp. - home: Builder( - builder: (context) { - resolved = Theme.of(context).textTheme; - return const SizedBox.shrink(); - }, - ), - ), - ); - - final css = File('web/index.html').readAsStringSync(); - - // The app bar headline. - final headline = resolved.headlineMedium!; - expect(css, contains('font-size: ${headline.fontSize!.round()}px')); - expect(css, contains('font-weight: ${headline.fontWeight!.value}')); - - // The extended FAB's label. - final label = resolved.labelLarge!; - expect(css, contains('font-size: ${label.fontSize!.round()}px')); - expect(css, contains('font-weight: ${label.fontWeight!.value}')); - }); - - testWidgets('draws a card the size of the card that replaces it', ( - tester, - ) async { - // Measured, not compared. - await tester.pumpWidget( - MaterialApp( - theme: buildTheme(Brightness.light), - home: const Scaffold( - body: CustomScrollView(slivers: [GroupListSkeleton()]), - ), - ), - ); - await tester.pump(const Duration(milliseconds: 50)); - - final card = tester.getSize(find.byType(SkeletonGroupCard).first); - final css = File('web/index.html').readAsStringSync(); - - expect( - css, - contains('height: ${card.height.round()}px'), - reason: - 'web/index.html must draw a ${card.height.round()}dp card, which ' - 'is what a two-line ListTile inside Card.outlined comes to', - ); - }); - - testWidgets('gives its app bar the height the app gives one', ( - tester, - ) async { - // The other number with no published source. - await tester.pumpWidget( - MaterialApp( - theme: buildTheme(Brightness.light), - home: const Scaffold( - body: CustomScrollView( - slivers: [ - SliverAppBar.large(title: Text('Groups'), pinned: true), - SliverToBoxAdapter( - child: SizedBox(key: Key('below'), height: 2000), - ), - ], - ), - ), - ), - ); - await tester.pump(); - - // Read off where the content starts rather than off the bar's own box: - // SliverAppBar.large has no single widget whose size is the expanded - // height, and the number that matters is exactly how far down the page - // the first card begins. - final barHeight = tester - .getTopLeft(find.byKey(const Key('below'))) - .dy - .round(); - final css = File('web/index.html').readAsStringSync(); - - expect( - css, - contains('height: ${barHeight}px'), - reason: 'the skeleton bar must be as tall as an expanded large one', - ); - }); - - test('agrees with the app on every boot-hint key', () { - // The loader decides which shape to draw, and whether to fill it, by - // reading keys the Dart side writes. - final keys = RegExp(r"""localStorage\.getItem\((['"])(.+?)\1\)""") - .allMatches(File('web/index.html').readAsStringSync()) - .map((match) => match.group(2)!) - .toSet(); - - expect( - keys, - hasLength(2), - reason: 'the loader reads a session hint and a groups hint', - ); - - final dart = File('lib/data/web/boot_hint_web.dart').readAsStringSync(); - for (final key in keys) { - expect( - dart, - contains("'$key'"), - reason: 'boot_hint_web.dart must write $key, which index.html reads', - ); - } - }); - - test('puts its navigation rail where the app puts one', () { - // Two numbers in two languages describing the same edge. - final dart = RegExp( - r'_railBreakpoint\s*=\s*(\d+)', - ).firstMatch(File('lib/presentation/router.dart').readAsStringSync()); - final width = RegExp( - r'_railWidth\s*=\s*(\d+)', - ).firstMatch(File('lib/presentation/router.dart').readAsStringSync()); - - expect(dart, isNotNull, reason: 'no _railBreakpoint in router.dart'); - expect(width, isNotNull, reason: 'no _railWidth in router.dart'); - - final css = File('web/index.html').readAsStringSync(); - expect( - css, - contains('@media (min-width: ${dart!.group(1)}px)'), - reason: 'web/index.html must show its rail at _railBreakpoint', - ); - expect( - css, - contains('width: ${width!.group(1)}px'), - reason: 'the skeleton rail must be _railWidth wide', - ); - }); + group('the web splash', () { + test('uses the theme\'s own colours', _splashMatchesTheme); test('paints its launch background in the theme\'s surface', () { final surface = buildTheme(Brightness.light).colorScheme.surface; @@ -249,7 +106,7 @@ void main() { }); }); - // The same problem the web skeleton has, on the other platform: Android + // The same problem the web splash has, on the other platform: Android // paints the window before a line of Dart runs — and on 12 and up paints a // system splash screen over it — from colours it can only read out of // resources and generator configs. @@ -363,7 +220,7 @@ void main() { }); } -/// The colours the loading skeleton in `web/index.html` claims to be using. +/// The colours the splash in `web/index.html` claims to be using. Map _declaredColors(String css, int from) { final open = css.indexOf(':root {', from); expect(open, isNot(-1), reason: 'no :root block after offset $from'); @@ -383,7 +240,7 @@ String _hex(Color c) { return '#${channel(c.r)}${channel(c.g)}${channel(c.b)}'; } -void _skeletonMatchesTheme() { +void _splashMatchesTheme() { final css = File('web/index.html').readAsStringSync(); final darkBlock = css.indexOf('@media (prefers-color-scheme: dark)'); @@ -400,22 +257,14 @@ void _skeletonMatchesTheme() { final expected = { 'surface': scheme.surface, - 'placeholder': scheme.surfaceContainerHighest, - 'outline-variant': scheme.outlineVariant, - 'on-surface': scheme.onSurface, - 'secondary-container': scheme.secondaryContainer, - 'primary-container': scheme.primaryContainer, - 'on-primary-container': scheme.onPrimaryContainer, 'primary': scheme.primary, - 'on-surface-variant': scheme.onSurfaceVariant, - 'surface-container': scheme.surfaceContainer, }; expect( declared.keys.toSet(), expected.keys.toSet(), reason: - 'the ${entry.key} skeleton palette and this test disagree about ' + 'the ${entry.key} splash palette and this test disagree about ' 'which roles it uses', ); diff --git a/web/index.html b/web/index.html index 746ee56..3d41ae8 100644 --- a/web/index.html +++ b/web/index.html @@ -58,50 +58,22 @@ -