From 037a1b9248cd2d720906702a96d61a631b76cf9c Mon Sep 17 00:00:00 2001 From: davidwei Date: Tue, 16 Jun 2026 20:35:21 +1200 Subject: [PATCH 1/2] fix: replace TagController.dispose() misuse with closeOverlay() method dispose() was being called on every pointer-down to close the tag overlay, which would cause a double-dispose exception if TagController ever gains a field that does not tolerate calling dispose() twice (StreamSubscription, AnimationController, etc.). Adds closeOverlay() as the correct API for runtime close operations. dispose() now calls closeOverlay() internally and asserts it is only called once. Updates the Listener.onPointerDown handler and the inline close in handleTextChanged() to use closeOverlay(). Adds TagController unit tests covering the new semantics. Closes #38 Co-Authored-By: Claude Sonnet 4.6 --- .../controllers/tag_controller.dart | 15 ++-- lib/screens/components/note_edit.dart | 2 +- test/controllers/tag_controller_test.dart | 68 +++++++++++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) create mode 100644 test/controllers/tag_controller_test.dart diff --git a/lib/screens/components/controllers/tag_controller.dart b/lib/screens/components/controllers/tag_controller.dart index 936d21e6..d0dcb9fe 100644 --- a/lib/screens/components/controllers/tag_controller.dart +++ b/lib/screens/components/controllers/tag_controller.dart @@ -10,6 +10,7 @@ class TagController { final NoteEditController noteEditController; OverlayEntry? _tagListOverlay; Timer? _tagListTimer; + bool _disposed = false; TagController({required this.noteTagService, required this.noteEditController}); @@ -23,11 +24,7 @@ class TagController { showTagList(noteModel, text, cursorPosition, context); }); } else { - _tagListTimer?.cancel(); - if (_tagListOverlay != null) { - _tagListOverlay?.remove(); - _tagListOverlay = null; - } + closeOverlay(); } } @@ -66,9 +63,15 @@ class TagController { return TextSelection.fromPosition(TextPosition(offset: newCursorPosition)); } - void dispose() { + void closeOverlay() { _tagListTimer?.cancel(); _tagListOverlay?.remove(); _tagListOverlay = null; } + + void dispose() { + assert(!_disposed, 'TagController.dispose() called more than once'); + _disposed = true; + closeOverlay(); + } } diff --git a/lib/screens/components/note_edit.dart b/lib/screens/components/note_edit.dart index a0abb77b..7cb106bb 100644 --- a/lib/screens/components/note_edit.dart +++ b/lib/screens/components/note_edit.dart @@ -217,7 +217,7 @@ class NoteEditState extends State { }, child: Listener( onPointerDown: (event) { - tagController.dispose(); // Close tag overlay if open + tagController.closeOverlay(); }, child: AnimatedBuilder( animation: noteModel.focusNode, diff --git a/test/controllers/tag_controller_test.dart b/test/controllers/tag_controller_test.dart new file mode 100644 index 00000000..40cec662 --- /dev/null +++ b/test/controllers/tag_controller_test.dart @@ -0,0 +1,68 @@ +import 'package:dio/dio.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:happy_notes/apis/file_uploader_api.dart'; +import 'package:happy_notes/apis/note_tag_api.dart'; +import 'package:happy_notes/screens/components/controllers/html_to_markdown_converter.dart'; +import 'package:happy_notes/screens/components/controllers/note_edit_controller.dart'; +import 'package:happy_notes/screens/components/controllers/tag_controller.dart'; +import 'package:happy_notes/services/clipboard_service.dart'; +import 'package:happy_notes/services/image_service.dart'; +import 'package:happy_notes/services/note_tag_service.dart'; + +class _FakeNoteTagApi extends NoteTagApi { + @override + Future> getMyTagCloud() async => + Response(requestOptions: RequestOptions(), data: {'successful': true, 'data': []}); +} + +class _FakeImageService extends ImageService { + _FakeImageService() : super(fileUploaderApi: FileUploaderApi()); +} + +TagController _makeTagController(NoteEditController noteEditController) { + return TagController( + noteTagService: NoteTagService(noteTagApi: _FakeNoteTagApi()), + noteEditController: noteEditController, + ); +} + +NoteEditController _makeNoteEditController() { + return NoteEditController( + imageService: _FakeImageService(), + clipboardService: ClipboardService(), + htmlToMarkdownConverter: HtmlToMarkdownConverter(), + ); +} + +void main() { + group('TagController.closeOverlay', () { + late NoteEditController noteEditController; + late TagController tagController; + + setUp(() { + noteEditController = _makeNoteEditController(); + tagController = _makeTagController(noteEditController); + }); + + tearDown(() { + tagController.dispose(); + noteEditController.dispose(); + }); + + test('closeOverlay is a no-op when no overlay is open', () { + expect(() => tagController.closeOverlay(), returnsNormally); + }); + + test('closeOverlay can be called multiple times without throwing', () { + tagController.closeOverlay(); + tagController.closeOverlay(); + tagController.closeOverlay(); + }); + + test('dispose throws AssertionError if called a second time', () { + final ctrl = _makeTagController(noteEditController); + ctrl.dispose(); + expect(() => ctrl.dispose(), throwsA(isA())); + }); + }); +} From 7267f0849d4ea3fd7ec8d1d120eccf6769fe83b0 Mon Sep 17 00:00:00 2001 From: davidwei Date: Tue, 16 Jun 2026 20:43:46 +1200 Subject: [PATCH 2/2] fix: clarify dispose() assert is debug-mode only Co-Authored-By: Claude Sonnet 4.6 --- lib/screens/components/controllers/tag_controller.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/screens/components/controllers/tag_controller.dart b/lib/screens/components/controllers/tag_controller.dart index d0dcb9fe..73fef078 100644 --- a/lib/screens/components/controllers/tag_controller.dart +++ b/lib/screens/components/controllers/tag_controller.dart @@ -69,6 +69,7 @@ class TagController { _tagListOverlay = null; } + // assert fires in debug builds only; use closeOverlay() for runtime close operations. void dispose() { assert(!_disposed, 'TagController.dispose() called more than once'); _disposed = true;