From e0badb07845cee952973cf94838bfbf2ddc2ceb5 Mon Sep 17 00:00:00 2001 From: Isaac Banner Date: Mon, 20 Jul 2026 17:31:07 -0700 Subject: [PATCH 1/3] fix(dance-editor): clear tag input after committing a tag (#402) Flutter's Autocomplete filled the field with the selected option's label before onSelected ran, so the typed text lingered after a tag pill was added, making back-to-back tag entry awkward. _AddAutocomplete now owns its TextEditingController and FocusNode and, only after a tag is successfully committed, clears the field and re-requests focus. Empty/duplicate/no-op submits never reach onSelected, so they leave state untouched. The shared NamePicker also gives the Authors field the same consistent clear-on-commit behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/screens/dance_editor/name_picker.dart | 42 ++++++++-- app/test/dance_editor_screen_test.dart | 78 +++++++++++++++++++ 2 files changed, 113 insertions(+), 7 deletions(-) diff --git a/app/lib/src/screens/dance_editor/name_picker.dart b/app/lib/src/screens/dance_editor/name_picker.dart index dfed7b40..c746d391 100644 --- a/app/lib/src/screens/dance_editor/name_picker.dart +++ b/app/lib/src/screens/dance_editor/name_picker.dart @@ -71,7 +71,7 @@ class NamePicker extends StatelessWidget { } } -class _AddAutocomplete extends StatelessWidget { +class _AddAutocomplete extends StatefulWidget { const _AddAutocomplete({ required this.fieldKey, required this.selectedIds, @@ -86,34 +86,62 @@ class _AddAutocomplete extends StatelessWidget { final ValueChanged onAdd; final Future Function(String name) onCreate; + @override + State<_AddAutocomplete> createState() => _AddAutocompleteState(); +} + +class _AddAutocompleteState extends State<_AddAutocomplete> { + // Owned so we can clear the field and keep focus after a tag is committed + // (issue #402: the typed text lingered because Flutter's Autocomplete fills + // the field with the selected option's label before onSelected runs). + final TextEditingController _controller = TextEditingController(); + final FocusNode _focusNode = FocusNode(); + + @override + void dispose() { + _controller.dispose(); + _focusNode.dispose(); + super.dispose(); + } + @override Widget build(BuildContext context) { + final fieldKey = widget.fieldKey; return Autocomplete<_PickerChoice>( key: ValueKey('$fieldKey-autocomplete'), + textEditingController: _controller, + focusNode: _focusNode, displayStringForOption: (choice) => choice.label, optionsBuilder: (value) { final q = value.text.trim(); if (q.isEmpty) return const Iterable<_PickerChoice>.empty(); final lower = q.toLowerCase(); - final matches = options + final matches = widget.options .where( (o) => - !selectedIds.contains(o.id) && + !widget.selectedIds.contains(o.id) && o.name.toLowerCase().contains(lower), ) .map((o) => _PickerChoice.existing(o.id, o.name)) .toList(); - final exact = options.any((o) => o.name.toLowerCase() == lower); + final exact = widget.options.any((o) => o.name.toLowerCase() == lower); if (!exact) matches.add(_PickerChoice.create(q)); return matches; }, onSelected: (choice) async { if (choice.isCreate) { - final id = await onCreate(choice.name); - onAdd(id); + // Only clear after the create + add succeeds; a thrown onCreate + // short-circuits before we touch the field. + final id = await widget.onCreate(choice.name); + widget.onAdd(id); } else { - onAdd(choice.id!); + widget.onAdd(choice.id!); } + // Reset the field and keep focus so the next tag can be typed straight + // away. Clearing lives only here, so empty/duplicate/no-op submits + // (which never reach onSelected) leave state untouched. + _controller.clear(); + _focusNode.requestFocus(); }, fieldViewBuilder: (context, controller, focusNode, onSubmit) { return TextField( diff --git a/app/test/dance_editor_screen_test.dart b/app/test/dance_editor_screen_test.dart index a10d35a9..a50daeb6 100644 --- a/app/test/dance_editor_screen_test.dart +++ b/app/test/dance_editor_screen_test.dart @@ -610,6 +610,84 @@ void main() { expect(find.text('Choreographer details'), findsNothing); }); + testWidgets( + 'committing a tag clears the input, keeps focus, and supports back-to-back adds', + (tester) async { + final repos = openTestRepositories(); + await repos.tags.upsert(Tag(id: 't1', name: 'flowy')); + await repos.tags.upsert(Tag(id: 't2', name: 'smooth')); + await _pumpEditor(tester, repos); + + await _expandMoreDetails(tester); + + TextField tagField() => + tester.widget(find.byKey(const ValueKey('tag-input'))); + + // First tag: type, pick the existing option. + await tester.enterText(find.byKey(const ValueKey('tag-input')), 'flowy'); + await tester.pumpAndSettle(); + await tester.tap(find.byKey(const ValueKey('tag-option-t1'))); + await tester.pumpAndSettle(); + + expect(find.widgetWithText(Chip, 'flowy'), findsOneWidget); + // The typed text is cleared and focus stays in the field so the next tag + // can be typed immediately (issue #402). + expect(tagField().controller!.text, isEmpty); + expect(tagField().focusNode!.hasFocus, isTrue); + + // Second tag added straight away from the now-empty field. + await tester.enterText(find.byKey(const ValueKey('tag-input')), 'smooth'); + await tester.pumpAndSettle(); + await tester.tap(find.byKey(const ValueKey('tag-option-t2'))); + await tester.pumpAndSettle(); + + expect(find.widgetWithText(Chip, 'flowy'), findsOneWidget); + expect(find.widgetWithText(Chip, 'smooth'), findsOneWidget); + expect(tagField().controller!.text, isEmpty); + }, + ); + + testWidgets('creating a new tag inline clears the input', (tester) async { + final repos = openTestRepositories(); + await _pumpEditor(tester, repos); + + await _expandMoreDetails(tester); + + await tester.enterText(find.byKey(const ValueKey('tag-input')), 'sparkly'); + await tester.pumpAndSettle(); + await tester.tap(find.byKey(const ValueKey('tag-option-create:sparkly'))); + await tester.pumpAndSettle(); + + expect(find.widgetWithText(Chip, 'sparkly'), findsOneWidget); + final field = tester.widget( + find.byKey(const ValueKey('tag-input')), + ); + expect(field.controller!.text, isEmpty); + }); + + testWidgets('an empty tag entry does not add a chip or corrupt state', ( + tester, + ) async { + final repos = openTestRepositories(); + await repos.tags.upsert(Tag(id: 't1', name: 'flowy')); + await _pumpEditor(tester, repos); + + await _expandMoreDetails(tester); + + // Whitespace-only input never opens options, so nothing can be committed. + await tester.enterText(find.byKey(const ValueKey('tag-input')), ' '); + await tester.pumpAndSettle(); + expect(find.byKey(const ValueKey('tag-option-t1')), findsNothing); + expect(find.byType(Chip), findsNothing); + + // The field is still usable: a real tag can still be added afterwards. + await tester.enterText(find.byKey(const ValueKey('tag-input')), 'flowy'); + await tester.pumpAndSettle(); + await tester.tap(find.byKey(const ValueKey('tag-option-t1'))); + await tester.pumpAndSettle(); + expect(find.widgetWithText(Chip, 'flowy'), findsOneWidget); + }); + testWidgets('surfaces non-blocking phrase warnings', (tester) async { final repos = openTestRepositories(); await repos.dances.create( From f0101fea98c3c4ce1cd2e822f761e8c3d2df728d Mon Sep 17 00:00:00 2001 From: Isaac Banner Date: Mon, 20 Jul 2026 17:44:22 -0700 Subject: [PATCH 2/3] fix(dance-editor): guard tag-input clear against disposal; tighten test Address Copilot review on #421: - Add `if (!mounted) return;` after awaiting onCreate so a disposed controller/focus node is never touched when the editor closes mid-create. - Assert the specific tag chip is absent (not any Chip) in the empty-entry test so unrelated chips (e.g. tune chips) can't mask a regression. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- app/lib/src/screens/dance_editor/name_picker.dart | 3 +++ app/test/dance_editor_screen_test.dart | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/lib/src/screens/dance_editor/name_picker.dart b/app/lib/src/screens/dance_editor/name_picker.dart index c746d391..58701c3a 100644 --- a/app/lib/src/screens/dance_editor/name_picker.dart +++ b/app/lib/src/screens/dance_editor/name_picker.dart @@ -133,6 +133,9 @@ class _AddAutocompleteState extends State<_AddAutocomplete> { // Only clear after the create + add succeeds; a thrown onCreate // short-circuits before we touch the field. final id = await widget.onCreate(choice.name); + // Guard against the widget being disposed during the await (e.g. the + // editor route closed while the tag was being created). + if (!mounted) return; widget.onAdd(id); } else { widget.onAdd(choice.id!); diff --git a/app/test/dance_editor_screen_test.dart b/app/test/dance_editor_screen_test.dart index a50daeb6..ca365795 100644 --- a/app/test/dance_editor_screen_test.dart +++ b/app/test/dance_editor_screen_test.dart @@ -678,7 +678,7 @@ void main() { await tester.enterText(find.byKey(const ValueKey('tag-input')), ' '); await tester.pumpAndSettle(); expect(find.byKey(const ValueKey('tag-option-t1')), findsNothing); - expect(find.byType(Chip), findsNothing); + expect(find.byKey(const ValueKey('tag-chip-t1')), findsNothing); // The field is still usable: a real tag can still be added afterwards. await tester.enterText(find.byKey(const ValueKey('tag-input')), 'flowy'); From 0ad9f7f30e28d4c555f7109b4e5bf9de5dbd5441 Mon Sep 17 00:00:00 2001 From: Isaac Banner Date: Mon, 20 Jul 2026 17:59:51 -0700 Subject: [PATCH 3/3] test(dance-editor): pin picker-disposed-mid-create guard and Authors clear-on-commit Follow-up to review on #421: - Add a NamePicker unit test that disposes the picker while onCreate is still in flight, then resolves it, asserting no disposed-controller exception and that the !mounted guard short-circuits before onAdd/clear. - Add an Authors-field test pinning that the shared picker also clears the input and keeps focus on commit, so the shared-widget guarantee can't silently regress for one field but not the other. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- app/test/dance_editor_screen_test.dart | 79 ++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/app/test/dance_editor_screen_test.dart b/app/test/dance_editor_screen_test.dart index ca365795..2bad2e75 100644 --- a/app/test/dance_editor_screen_test.dart +++ b/app/test/dance_editor_screen_test.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:compendium_core/compendium_core.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -9,6 +11,7 @@ import 'package:compendium_app/src/data/repositories_scope.dart'; import 'package:compendium_app/src/editor/editor_draft_codec.dart'; import 'package:compendium_app/src/editor/editor_snapshot.dart'; import 'package:compendium_app/src/screens/dance_editor_screen.dart'; +import 'package:compendium_app/src/screens/dance_editor/name_picker.dart'; import 'package:compendium_app/src/screens/dance_list_screen.dart'; import 'package:compendium_app/src/theme/app_theme.dart'; import 'package:compendium_app/src/widgets/figure_list_editor.dart'; @@ -688,6 +691,82 @@ void main() { expect(find.widgetWithText(Chip, 'flowy'), findsOneWidget); }); + testWidgets( + 'disposing the picker mid-create does not touch the disposed controller', + (tester) async { + // Reproduces the async gap on the create path (#402 follow-up): onCreate + // is a real Future, so if the editor is dismissed before it completes the + // owned controller/focus node must not be used. Guarded by !mounted. + final createCompleter = Completer(); + var addCount = 0; + + Widget harness(bool showPicker) => MaterialApp( + home: Scaffold( + body: showPicker + ? NamePicker( + fieldKey: 'tag', + selectedIds: const [], + namesById: const {}, + options: const [], + onAdd: (_) => addCount++, + onRemove: (_) {}, + onCreate: (_) => createCompleter.future, + ) + : const SizedBox.shrink(), + ), + ); + + await tester.pumpWidget(harness(true)); + await tester.enterText( + find.byKey(const ValueKey('tag-input')), + 'sparkly', + ); + await tester.pumpAndSettle(); + // Commit via the create option; onSelected now awaits onCreate. + await tester.tap(find.byKey(const ValueKey('tag-option-create:sparkly'))); + await tester.pump(); + + // Tear the picker down while the create is still in flight, then let the + // future resolve. Without the !mounted guard this would throw + // "A TextEditingController was used after being disposed." + await tester.pumpWidget(harness(false)); + createCompleter.complete('t-new'); + await tester.pumpAndSettle(); + + expect(tester.takeException(), isNull); + // The guard short-circuits before onAdd/clear once disposed. + expect(addCount, 0); + }, + ); + + testWidgets( + 'committing an author clears the shared picker input and keeps focus', + (tester) async { + final repos = openTestRepositories(); + await repos.choreographers.upsert( + Choreographer(id: 'c1', name: 'Gene Hubert'), + ); + await _pumpEditor(tester, repos, danceId: null); + + await tester.enterText( + find.byKey(const ValueKey('author-input')), + 'Gene', + ); + await tester.pumpAndSettle(); + await tester.tap(find.byKey(const ValueKey('author-option-c1'))); + await tester.pumpAndSettle(); + + expect(find.widgetWithText(InputChip, 'Gene Hubert'), findsOneWidget); + // The shared NamePicker clears + keeps focus for authors too, so the + // guarantee can't silently regress for one field but not the other. + final field = tester.widget( + find.byKey(const ValueKey('author-input')), + ); + expect(field.controller!.text, isEmpty); + expect(field.focusNode!.hasFocus, isTrue); + }, + ); + testWidgets('surfaces non-blocking phrase warnings', (tester) async { final repos = openTestRepositories(); await repos.dances.create(