fix(#28): guard settings onChanged persisters against silent failures (panes)
Settings onChanged/onTap callbacks fire-and-forgot AppSettingsService Future<void> setters, so a persist failure rejected unhandled and was swallowed. Add a shared persistSetting(context, action) helper that awaits the setter in a try/catch and shows a dismissible error SnackBar, capturing the messenger before the await so it survives a picker-dialog pop. Wrap the 27 onChanged/onTap setter sites in app_settings_view + message_settings_view; setAppDebugLogEnabled and setNotificationsEnabled (which await + show their own confirmation) get an inline try/catch instead. Covers the two onChanged panes; the settings_screen action flows (GPX export, location dialogs, version-info mounted guard) follow in a second pass, so #28 stays open. Error strings are English for now (separate settings l10n sweep). Adds a persistSetting widget test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>pull/60/head
Strycher
1 month ago
parent
5227507810
commit
d2cca1da82
@ -0,0 +1,36 @@
|
||||
import 'package:flutter/material.dart';
|
||||
|
||||
/// Runs a settings-persist [action] and surfaces a dismissible error SnackBar if
|
||||
/// it throws, instead of dropping the returned Future and swallowing the error
|
||||
/// (no silent failures — #28).
|
||||
///
|
||||
/// Call sites intentionally don't await the returned Future: this function never
|
||||
/// rethrows (it catches internally), so dropping it is safe. The
|
||||
/// [ScaffoldMessenger] is captured before the await, so the error still shows
|
||||
/// even when the triggering widget (e.g. a picker dialog) is popped immediately
|
||||
/// after.
|
||||
///
|
||||
/// [message] is English-only for now; localizing it is part of the separate
|
||||
/// settings l10n sweep noted on #28.
|
||||
Future<void> persistSetting(
|
||||
BuildContext context,
|
||||
Future<void> Function() action, {
|
||||
String message = 'Could not save that setting. Please try again.',
|
||||
}) async {
|
||||
final messenger = ScaffoldMessenger.of(context);
|
||||
try {
|
||||
await action();
|
||||
} catch (_) {
|
||||
messenger.showSnackBar(
|
||||
SnackBar(
|
||||
content: GestureDetector(
|
||||
onTap: () => messenger.hideCurrentSnackBar(),
|
||||
child: Text(message),
|
||||
),
|
||||
behavior: SnackBarBehavior.floating,
|
||||
duration: const Duration(seconds: 4),
|
||||
dismissDirection: DismissDirection.down,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,47 @@
|
||||
import 'package:flutter/material.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:meshcore_open/helpers/settings_persist.dart';
|
||||
|
||||
void main() {
|
||||
Widget harness(Future<void> Function() action) {
|
||||
return MaterialApp(
|
||||
home: Scaffold(
|
||||
body: Builder(
|
||||
builder: (context) => ElevatedButton(
|
||||
onPressed: () =>
|
||||
persistSetting(context, action, message: 'Save failed'),
|
||||
child: const Text('go'),
|
||||
),
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
testWidgets('shows an error SnackBar when the persist action throws', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(
|
||||
harness(() async {
|
||||
throw Exception('boom');
|
||||
}),
|
||||
);
|
||||
|
||||
await tester.tap(find.text('go'));
|
||||
await tester.pump(); // run persistSetting (catch + schedule SnackBar)
|
||||
await tester.pump(); // build the SnackBar
|
||||
|
||||
expect(find.text('Save failed'), findsOneWidget);
|
||||
});
|
||||
|
||||
testWidgets('shows no SnackBar when the persist action succeeds', (
|
||||
tester,
|
||||
) async {
|
||||
await tester.pumpWidget(harness(() async {}));
|
||||
|
||||
await tester.tap(find.text('go'));
|
||||
await tester.pump();
|
||||
await tester.pump();
|
||||
|
||||
expect(find.byType(SnackBar), findsNothing);
|
||||
});
|
||||
}
|
||||
Loading…
Reference in new issue