From e048b6b6bdb1fe0d0cdf481ad633f2144fe0481b Mon Sep 17 00:00:00 2001 From: Edison Jwa Date: Sun, 7 Jun 2026 23:05:17 +0900 Subject: [PATCH] fix(chat): propagate empty draft on target swap and dispose Oracle re-review on PR #30 flagged that _ChatDetailViewState only called onDraftChanged when _textCtl.text was non-empty. The empty case is load-bearing: if the user restored a saved draft, deleted the text, then switched target (or closed the panel), the parent's draft map kept the stale entry and resurrected it on the next swap. Fix: call onDraftChanged unconditionally in both didUpdateWidget (target change) and dispose (tear-down), so the parent map learns when a draft is now empty. Adds a regression test exercising the restore-clear-swap sequence. --- .../lib/widgets/chat_views.dart | 16 +++-- .../test/widgets/chat_panel_test.dart | 65 +++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/apps/chanora_flutter/lib/widgets/chat_views.dart b/apps/chanora_flutter/lib/widgets/chat_views.dart index 175c62e..b4332d2 100644 --- a/apps/chanora_flutter/lib/widgets/chat_views.dart +++ b/apps/chanora_flutter/lib/widgets/chat_views.dart @@ -1144,9 +1144,12 @@ class _ChatDetailViewState extends State { void didUpdateWidget(covariant ChatDetailView oldWidget) { super.didUpdateWidget(oldWidget); if (oldWidget.target != widget.target) { - if (oldWidget.onDraftChanged != null && _textCtl.text.isNotEmpty) { - oldWidget.onDraftChanged!(_textCtl.text); - } + // Propagate the OUTGOING draft unconditionally, including empty + // text. The empty case is load-bearing: if the user typed text, + // saved it, restored it, then deleted everything, the parent + // map must learn the draft is now empty — otherwise the stale + // entry resurrects on the next target swap. + oldWidget.onDraftChanged?.call(_textCtl.text); _textCtl.text = widget.restoredDraft ?? ''; _lastRenderedTarget = null; } @@ -1154,9 +1157,10 @@ class _ChatDetailViewState extends State { @override void dispose() { - if (widget.onDraftChanged != null && _textCtl.text.isNotEmpty) { - widget.onDraftChanged!(_textCtl.text); - } + // Same unconditional flush on tear-down. The `isNotEmpty` guard + // here would silently drop "user cleared the field then closed + // the panel" into the same stale-entry bug class as didUpdateWidget. + widget.onDraftChanged?.call(_textCtl.text); _textCtl.dispose(); _scrollCtl.dispose(); super.dispose(); diff --git a/apps/chanora_flutter/test/widgets/chat_panel_test.dart b/apps/chanora_flutter/test/widgets/chat_panel_test.dart index 4a5ad13..2e9406f 100644 --- a/apps/chanora_flutter/test/widgets/chat_panel_test.dart +++ b/apps/chanora_flutter/test/widgets/chat_panel_test.dart @@ -133,4 +133,69 @@ void main() { 'server draft', ); }); + + testWidgets( + 'chat detail propagates empty draft when the user clears it before switching target', + (tester) async { + // Regression: previously, _ChatDetailViewState only emitted + // onDraftChanged when the text was non-empty. If the user + // restored a saved draft, deleted it, then switched target, + // the stale entry stayed in the parent's draft map and + // resurrected on the next target swap. + String? savedDraft = 'sentinel-unset'; + final messages = []; + + Widget detail({ + required rust.BridgeMessageTarget target, + required String? restoredDraft, + }) { + return MaterialApp( + localizationsDelegates: AppL10n.localizationsDelegates, + supportedLocales: AppL10n.supportedLocales, + home: Scaffold( + body: ChatDetailView( + messages: messages, + snapshot: snapshot(), + target: target, + clientName: '', + currentChannelId: BigInt.from(10), + channelName: 'Lobby', + restoredDraft: restoredDraft, + onDraftChanged: (text) => savedDraft = text, + ), + ), + ); + } + + await tester.pumpWidget( + detail( + target: const rust.BridgeMessageTarget.channel(), + restoredDraft: 'previously saved channel draft', + ), + ); + + expect( + tester.widget(find.byType(TextField)).controller!.text, + 'previously saved channel draft', + ); + + // User clears the field, then switches target. + await tester.enterText(find.byType(TextField), ''); + await tester.pumpWidget( + detail( + target: const rust.BridgeMessageTarget.server(), + restoredDraft: null, + ), + ); + + // The empty string MUST reach the parent so the stale entry + // is overwritten in the draft map. With the previous guarded + // implementation, savedDraft would still hold the sentinel. + expect( + savedDraft, + '', + reason: 'empty draft must overwrite stale entry on target swap', + ); + }, + ); }