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.
This commit is contained in:
Edison Jwa
2026-06-07 23:30:00 +09:00
parent dad633e381
commit e048b6b6bd
2 changed files with 75 additions and 6 deletions
@@ -1144,9 +1144,12 @@ class _ChatDetailViewState extends State<ChatDetailView> {
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<ChatDetailView> {
@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();
@@ -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 = <ChatEntry>[];
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<TextField>(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',
);
},
);
}