@tduncan / Cortana / commits / 897174b

refactor: fix crash report deduplication, resolve dialog controller lifecycle issues, and update test suites for robustness

abdulmominsakib committed Aug 4, 2026 at 14:23 UTC 897174b0f6bbe433755e30bdbd255a8f7187d4e8
5 files changed +97 -67
lib/core/components/folder_management_dialogs.dart
+73 -54
@@ -7,65 +7,84 @@ import 'package:localmind/l10n/app_localizations.dart';
7
8 /// Shows a dialog asking the user to enter a new folder name. Returns the
9 /// trimmed new name, or `null` if cancelled. Rejects empty names.
10 +///
11 +/// The controller lives inside [_RenameFolderDialog]'s state so it is only
12 +/// disposed once the dialog element is actually unmounted — disposing it as
13 +/// soon as `showDialog` resolves is unsafe because the exit animation keeps
14 +/// the TextField alive (and rebuilding) for a short while.
15 Future<String?> showRenameFolderDialog(
16 BuildContext context, {
17 required String currentName,
13 -}) async {
14 - final l10n = AppLocalizations.of(context)!;
15 - final controller = TextEditingController(text: currentName);
16 - controller.selection = TextSelection(
17 - baseOffset: 0,
18 - extentOffset: currentName.length,
19 - );
20 - String? errorText;
21 -
22 - final result = await showDialog<String>(
18 +}) {
19 + return showDialog<String>(
20 context: context,
24 - builder: (ctx) {
25 - return StatefulBuilder(
26 - builder: (ctx, setState) {
27 - return AlertDialog(
28 - title: Text(l10n.rename_folder),
29 - content: TextField(
30 - controller: controller,
31 - autofocus: true,
32 - decoration: InputDecoration(
33 - hintText: l10n.folder_name_hint,
34 - errorText: errorText,
35 - ),
36 - onSubmitted: (_) {
37 - final value = controller.text.trim();
38 - if (value.isEmpty) {
39 - setState(() => errorText = l10n.folder_name_required);
40 - return;
41 - }
42 - Navigator.pop(ctx, value);
43 - },
44 - ),
45 - actions: [
46 - TextButton(
47 - onPressed: () => Navigator.pop(ctx),
48 - child: Text(l10n.cancel),
49 - ),
50 - TextButton(
51 - onPressed: () {
52 - final value = controller.text.trim();
53 - if (value.isEmpty) {
54 - setState(() => errorText = l10n.folder_name_required);
55 - return;
56 - }
57 - Navigator.pop(ctx, value);
58 - },
59 - child: Text(l10n.save),
60 - ),
61 - ],
62 - );
63 - },
64 - );
65 - },
21 + builder: (ctx) => _RenameFolderDialog(currentName: currentName),
22 );
67 - controller.dispose();
68 - return result;
23 +}
24 +
25 +class _RenameFolderDialog extends StatefulWidget {
26 + const _RenameFolderDialog({required this.currentName});
27 +
28 + final String currentName;
29 +
30 + @override
31 + State<_RenameFolderDialog> createState() => _RenameFolderDialogState();
32 +}
33 +
34 +class _RenameFolderDialogState extends State<_RenameFolderDialog> {
35 + late final TextEditingController _controller;
36 + String? _errorText;
37 +
38 + @override
39 + void initState() {
40 + super.initState();
41 + _controller = TextEditingController(text: widget.currentName);
42 + _controller.selection = TextSelection(
43 + baseOffset: 0,
44 + extentOffset: widget.currentName.length,
45 + );
46 + }
47 +
48 + @override
49 + void dispose() {
50 + _controller.dispose();
51 + super.dispose();
52 + }
53 +
54 + void _submit() {
55 + final value = _controller.text.trim();
56 + if (value.isEmpty) {
57 + setState(() {
58 + _errorText = AppLocalizations.of(context)!.folder_name_required;
59 + });
60 + return;
61 + }
62 + Navigator.pop(context, value);
63 + }
64 +
65 + @override
66 + Widget build(BuildContext context) {
67 + final l10n = AppLocalizations.of(context)!;
68 + return AlertDialog(
69 + title: Text(l10n.rename_folder),
70 + content: TextField(
71 + controller: _controller,
72 + autofocus: true,
73 + decoration: InputDecoration(
74 + hintText: l10n.folder_name_hint,
75 + errorText: _errorText,
76 + ),
77 + onSubmitted: (_) => _submit(),
78 + ),
79 + actions: [
80 + TextButton(
81 + onPressed: () => Navigator.pop(context),
82 + child: Text(l10n.cancel),
83 + ),
84 + TextButton(onPressed: _submit, child: Text(l10n.save)),
85 + ],
86 + );
87 + }
88 }
89
90 /// Shows a confirmation dialog asking the user whether they really want to
lib/core/services/crash_report_service.dart
+6 -1
@@ -243,12 +243,17 @@ class CrashReportService {
243 errorWidgetPayload: errorWidgetPayload,
244 );
245
246 + // The duplicate check must run before the report is recorded, otherwise
247 + // the freshly added report matches itself and no crash would ever surface
248 + // in the UI.
249 + final isDuplicate = _isDuplicate(report);
250 +
251 _recentCrashes.add(report);
252 if (_recentCrashes.length > 20) {
253 _recentCrashes.removeAt(0);
254 }
255
251 - if (!_isDuplicate(report)) {
256 + if (!isDuplicate) {
257 _currentCrash.value = report;
258 }
259 return report;
test/core/components/folder_filter_bar_test.dart
+2 -2
@@ -43,8 +43,8 @@ void main() {
43 await tester.longPress(find.text('Research'));
44 await tester.pump();
45 // The wrapper delivers globalPosition; we just confirm the callback fired
46 - // for the right folder.
47 - expect(captured?.id, 'Research');
46 + // for the right folder (callbacks receive the folder id, not the name).
47 + expect(captured?.id, 'research');
48 expect(capturedPos, isNotNull);
49 });
50
test/core/components/folder_management_dialogs_test.dart
+10 -7
@@ -14,17 +14,20 @@ Future<void> _pumpHarness(
14 locale: const Locale('en'),
15 home: Scaffold(
16 body: Builder(
17 - builder: (ctx) {
18 - // Call the test action once the initial frame has rendered.
19 - WidgetsBinding.instance.addPostFrameCallback((_) {
20 - body(ctx);
21 - });
22 - return const SizedBox.shrink();
23 - },
17 + builder: (ctx) => TextButton(
18 + // Trigger the dialog from a real user interaction instead of a
19 + // post-frame callback: pushing a route mid-frame marks focus
20 + // scopes dirty outside their build scope, which newer Flutter
21 + // versions reject with a "dirty widget in the wrong build scope"
22 + // assertion.
23 + onPressed: () => body(ctx),
24 + child: const Text('Open'),
25 + ),
26 ),
27 ),
28 ),
29 );
30 + await tester.tap(find.text('Open'));
31 // Allow async dialog futures to settle.
32 await tester.pump();
33 await tester.pump(const Duration(milliseconds: 300));
test/core/services/crash_report_service_test.dart
+6 -3
@@ -34,7 +34,9 @@ void main() {
34 error: Exception('Something went wrong'),
35 stack: StackTrace.current,
36 );
37 - expect(report.shortError, 'Something went wrong');
37 + // Dart's `Exception.toString()` includes the type prefix
38 + // ("Exception: ..."), and shortError surfaces the raw first line.
39 + expect(report.shortError, 'Exception: Something went wrong');
40 });
41
42 test('shortError truncates long first lines to 80 chars', () {
@@ -44,7 +46,8 @@ void main() {
46 stack: StackTrace.current,
47 );
48 expect(report.shortError.length, 80);
47 - expect(report.shortError, '${'A' * 77}...');
49 + // 77 chars kept from "Exception: AAA..." plus the ellipsis.
50 + expect(report.shortError, 'Exception: ${'A' * 66}...');
51 });
52
53 test('shortError sanitizes user-specific paths', () {
@@ -139,7 +142,7 @@ void main() {
142 expect(uri.path, '/abdulmominsakib/localmind/issues/new');
143 expect(uri.queryParameters['template'], 'crash_report.md');
144 expect(uri.queryParameters['labels'], 'crash,bug');
142 - expect(uri.queryParameters['title'], 'Crash: Boom');
145 + expect(uri.queryParameters['title'], 'Crash: Exception: Boom');
146 expect(uri.queryParameters['body'], isNotNull);
147 });
148