Skip to content

Commit 2683df0

Browse files
committed
compose: Fix bug where compose progress can be lost on new event queue
And factor out a helper for the part that reads `widget.narrow` and sets a new controller accordingly; we'll use this helper for the edit-message UI, to reset the compose box at the end of an edit-message session. Fixes: #1470
1 parent 6852eaa commit 2683df0

File tree

2 files changed

+61
-8
lines changed

2 files changed

+61
-8
lines changed

lib/widgets/compose_box.dart

+18-6
Original file line numberDiff line numberDiff line change
@@ -1523,14 +1523,26 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
15231523

15241524
@override
15251525
void onNewStore() {
1526+
final newStore = PerAccountStoreWidget.of(context);
1527+
1528+
final controller = _controller;
1529+
if (controller == null) {
1530+
_setNewController(newStore);
1531+
return;
1532+
}
1533+
1534+
switch (controller) {
1535+
case StreamComposeBoxController():
1536+
controller.topic.store = newStore;
1537+
case FixedDestinationComposeBoxController():
1538+
// no reference to the store that needs updating
1539+
}
1540+
}
1541+
1542+
void _setNewController(PerAccountStore store) {
15261543
switch (widget.narrow) {
15271544
case ChannelNarrow():
1528-
final store = PerAccountStoreWidget.of(context);
1529-
if (_controller == null) {
1530-
_controller = StreamComposeBoxController(store: store);
1531-
} else {
1532-
(controller as StreamComposeBoxController).topic.store = store;
1533-
}
1545+
_controller = StreamComposeBoxController(store: store);
15341546
case TopicNarrow():
15351547
case DmNarrow():
15361548
_controller = FixedDestinationComposeBoxController();

test/widgets/compose_box_test.dart

+43-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import '../api/fake_api.dart';
2828
import '../example_data.dart' as eg;
2929
import '../flutter_checks.dart';
3030
import '../model/binding.dart';
31+
import '../model/store_checks.dart';
3132
import '../model/test_store.dart';
3233
import '../model/typing_status_test.dart';
3334
import '../stdlib_checks.dart';
@@ -58,14 +59,14 @@ void main() {
5859
zulipFeatureLevel ??= eg.futureZulipFeatureLevel;
5960
final selfAccount = eg.account(user: selfUser, zulipFeatureLevel: zulipFeatureLevel);
6061
await testBinding.globalStore.add(selfAccount, eg.initialSnapshot(
62+
realmUsers: [selfUser, ...otherUsers],
63+
streams: streams,
6164
zulipFeatureLevel: zulipFeatureLevel,
6265
realmMandatoryTopics: mandatoryTopics,
6366
));
6467

6568
store = await testBinding.globalStore.perAccount(selfAccount.id);
6669

67-
await store.addUsers([selfUser, ...otherUsers]);
68-
await store.addStreams(streams);
6970
connection = store.connection as FakeApiConnection;
7071

7172
await tester.pumpWidget(TestZulipApp(accountId: selfAccount.id,
@@ -111,6 +112,11 @@ void main() {
111112
await tester.enterText(contentInputFinder, content);
112113
}
113114

115+
void checkContentInputValue(WidgetTester tester, String expected) {
116+
check(tester.widget<TextField>(contentInputFinder))
117+
.controller.isNotNull().value.text.equals(expected);
118+
}
119+
114120
Future<void> tapSendButton(WidgetTester tester) async {
115121
connection.prepare(json: SendMessageResult(id: 123).toJson());
116122
await tester.tap(find.byIcon(ZulipIcons.send));
@@ -1239,4 +1245,39 @@ void main() {
12391245
maxHeight: verticalPadding + 170 * 1.5, maxVisibleLines: 6);
12401246
});
12411247
});
1248+
1249+
group('ComposeBoxState new-event-queue transition', () {
1250+
testWidgets('content input not cleared when store changes', (tester) async {
1251+
// Regression test for: https://github.com/zulip/zulip-flutter/issues/1470
1252+
1253+
TypingNotifier.debugEnable = false;
1254+
addTearDown(TypingNotifier.debugReset);
1255+
1256+
final channel = eg.stream();
1257+
await prepareComposeBox(tester,
1258+
narrow: eg.topicNarrow(channel.streamId, 'topic'), streams: [channel]);
1259+
1260+
await enterContent(tester, 'some content');
1261+
checkContentInputValue(tester, 'some content');
1262+
1263+
store.updateMachine!
1264+
..debugPauseLoop()
1265+
..poll()
1266+
..debugPrepareLoopError(
1267+
eg.apiExceptionBadEventQueueId(queueId: store.queueId))
1268+
..debugAdvanceLoop();
1269+
await tester.pump();
1270+
1271+
final newStore = testBinding.globalStore.perAccountSync(store.accountId)!;
1272+
check(newStore)
1273+
// a new store has replaced the old one
1274+
..not((it) => it.identicalTo(store))
1275+
// new store has the same boring data, in order to present a compose box
1276+
// that allows composing, instead of a no-posting-permission banner
1277+
..accountId.equals(store.accountId)
1278+
..streams.containsKey(channel.streamId);
1279+
1280+
checkContentInputValue(tester, 'some content');
1281+
});
1282+
});
12421283
}

0 commit comments

Comments
 (0)