Skip to content

Commit fe3a0fe

Browse files
committed
compose: Store controller on ComposeBox widget, not descendant
Now there's just one place in the code where the controller is stored, instead of in both _StreamComposeBoxState and _FixedDestinationComposeBoxState. Also, those two widgets are simplified to be stateless. One small functional change: if the compose box disappears and then reappears -- because the self-user lost and gained posting permission, or a DM recipient was deactivated then reactivated -- the compose box returns to its state before it disappeared, instead of being re-initialized. This should be rare, but seems fine and helpful when it happens; it's one fewer reason your compose progress might get lost.
1 parent 5c26056 commit fe3a0fe

File tree

2 files changed

+51
-57
lines changed

2 files changed

+51
-57
lines changed

lib/widgets/action_sheet.dart

+10-10
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,12 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
326326
@override void onPressed() async {
327327
final zulipLocalizations = ZulipLocalizations.of(pageContext);
328328

329-
// This will be null only if the compose box disappeared after the
330-
// message action sheet opened, and before "Quote and reply" was pressed.
331-
// This is rare: it happens when the self-user loses posting permission
332-
// or when a DM recipient becomes deactivated.
333329
var composeBoxController = findMessageListPage().composeBoxController;
334-
if (composeBoxController == null) return;
330+
// The compose box doesn't null out its controller; it's either always null
331+
// (e.g. in Combined Feed) or always non-null; it can't have been nulled out
332+
// after the action sheet opened.
333+
assert (composeBoxController != null);
334+
composeBoxController!;
335335
if (
336336
composeBoxController is StreamComposeBoxController
337337
&& composeBoxController.topic.textNormalized == kNoTopicTopic
@@ -355,12 +355,12 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
355355

356356
if (!pageContext.mounted) return;
357357

358-
// This will be null only if the compose box disappeared during the
359-
// quotation request. This is rare: it happens when the self-user loses
360-
// posting permission or when a DM recipient becomes deactivated.
361358
composeBoxController = findMessageListPage().composeBoxController;
362-
if (composeBoxController == null) return;
363-
composeBoxController.content
359+
// The compose box doesn't null out its controller; it's either always null
360+
// (e.g. in Combined Feed) or always non-null; it can't have been nulled out
361+
// during the raw-content request.
362+
assert(composeBoxController != null);
363+
composeBoxController!.content
364364
.registerQuoteAndReplyEnd(PerAccountStoreWidget.of(pageContext), tag,
365365
message: message,
366366
rawContent: rawContent,

lib/widgets/compose_box.dart

+41-47
Original file line numberDiff line numberDiff line change
@@ -1134,25 +1134,13 @@ class FixedDestinationComposeBoxController extends ComposeBoxController {}
11341134
///
11351135
/// This offers a text input for the topic to send to,
11361136
/// in addition to a text input for the message content.
1137-
class _StreamComposeBox extends StatefulWidget {
1138-
const _StreamComposeBox({super.key, required this.narrow});
1137+
class _StreamComposeBox extends StatelessWidget {
1138+
const _StreamComposeBox({required this.narrow, required this.controller});
11391139

11401140
/// The narrow on view in the message list.
11411141
final ChannelNarrow narrow;
11421142

1143-
@override
1144-
State<_StreamComposeBox> createState() => _StreamComposeBoxState();
1145-
}
1146-
1147-
class _StreamComposeBoxState extends State<_StreamComposeBox> {
1148-
StreamComposeBoxController get controller => _controller;
1149-
final StreamComposeBoxController _controller = StreamComposeBoxController();
1150-
1151-
@override
1152-
void dispose() {
1153-
_controller.dispose();
1154-
super.dispose();
1155-
}
1143+
final StreamComposeBoxController controller;
11561144

11571145
@override
11581146
Widget build(BuildContext context) {
@@ -1164,13 +1152,13 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> {
11641152
contentController: content,
11651153
contentFocusNode: contentFocusNode,
11661154
topicInput: _TopicInput(
1167-
streamId: widget.narrow.streamId,
1155+
streamId: narrow.streamId,
11681156
controller: topic,
11691157
focusNode: topicFocusNode,
11701158
contentFocusNode: contentFocusNode,
11711159
),
11721160
contentInput: _StreamContentInput(
1173-
narrow: widget.narrow,
1161+
narrow: narrow,
11741162
topicController: topic,
11751163
controller: content,
11761164
focusNode: contentFocusNode,
@@ -1179,7 +1167,7 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> {
11791167
topicController: topic,
11801168
contentController: content,
11811169
getDestination: () => StreamDestination(
1182-
widget.narrow.streamId, topic.textNormalized),
1170+
narrow.streamId, topic.textNormalized),
11831171
));
11841172
}
11851173
}
@@ -1205,24 +1193,12 @@ class _ErrorBanner extends StatelessWidget {
12051193
}
12061194
}
12071195

1208-
class _FixedDestinationComposeBox extends StatefulWidget {
1209-
const _FixedDestinationComposeBox({super.key, required this.narrow});
1196+
class _FixedDestinationComposeBox extends StatelessWidget {
1197+
const _FixedDestinationComposeBox({required this.narrow, required this.controller});
12101198

12111199
final SendableNarrow narrow;
12121200

1213-
@override
1214-
State<_FixedDestinationComposeBox> createState() => _FixedDestinationComposeBoxState();
1215-
}
1216-
1217-
class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox> {
1218-
FixedDestinationComposeBoxController get controller => _controller;
1219-
final FixedDestinationComposeBoxController _controller = FixedDestinationComposeBoxController();
1220-
1221-
@override
1222-
void dispose() {
1223-
_controller.dispose();
1224-
super.dispose();
1225-
}
1201+
final FixedDestinationComposeBoxController controller;
12261202

12271203
@override
12281204
Widget build(BuildContext context) {
@@ -1235,14 +1211,14 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
12351211
contentFocusNode: contentFocusNode,
12361212
topicInput: null,
12371213
contentInput: _FixedDestinationContentInput(
1238-
narrow: widget.narrow,
1214+
narrow: narrow,
12391215
controller: content,
12401216
focusNode: contentFocusNode,
12411217
),
12421218
sendButton: _SendButton(
12431219
topicController: null,
12441220
contentController: content,
1245-
getDestination: () => widget.narrow.destination,
1221+
getDestination: () => narrow.destination,
12461222
));
12471223
}
12481224
}
@@ -1273,20 +1249,34 @@ class ComposeBox extends StatefulWidget {
12731249

12741250
/// The interface for the state of a [ComposeBox].
12751251
abstract class ComposeBoxState extends State<ComposeBox> {
1276-
ComposeBoxController? get controller;
1252+
ComposeBoxController get controller;
12771253
}
12781254

12791255
class _ComposeBoxState extends State<ComposeBox> implements ComposeBoxState {
1280-
@override ComposeBoxController? get controller {
1281-
final forStream = _streamComposeBoxControllerKey.currentState?.controller;
1282-
final forFixedDestination = _fixedDestinationComposeBoxControllerKey.currentState?.controller;
1283-
assert(forStream == null || forFixedDestination == null);
1284-
// Both can be null (error-banner case).
1285-
return forStream ?? forFixedDestination;
1256+
@override ComposeBoxController get controller => _controller;
1257+
late final ComposeBoxController _controller;
1258+
1259+
@override
1260+
void initState() {
1261+
super.initState();
1262+
switch (widget.narrow) {
1263+
case ChannelNarrow():
1264+
_controller = StreamComposeBoxController();
1265+
case TopicNarrow():
1266+
case DmNarrow():
1267+
_controller = FixedDestinationComposeBoxController();
1268+
case CombinedFeedNarrow():
1269+
case MentionsNarrow():
1270+
case StarredMessagesNarrow():
1271+
assert(false);
1272+
}
12861273
}
1287-
final _streamComposeBoxControllerKey = GlobalKey<_StreamComposeBoxState>();
1288-
final _fixedDestinationComposeBoxControllerKey = GlobalKey<_FixedDestinationComposeBoxState>();
12891274

1275+
@override
1276+
void dispose() {
1277+
_controller.dispose();
1278+
super.dispose();
1279+
}
12901280

12911281
Widget? _errorBanner(BuildContext context) {
12921282
final store = PerAccountStoreWidget.of(context);
@@ -1325,12 +1315,16 @@ class _ComposeBoxState extends State<ComposeBox> implements ComposeBoxState {
13251315
final narrow = widget.narrow;
13261316
switch (narrow) {
13271317
case ChannelNarrow():
1328-
return _StreamComposeBox(key: _streamComposeBoxControllerKey, narrow: narrow);
1318+
assert(_controller is StreamComposeBoxController);
1319+
return _StreamComposeBox(controller: (_controller as StreamComposeBoxController),
1320+
narrow: narrow);
13291321
case TopicNarrow():
1330-
return _FixedDestinationComposeBox(key: _fixedDestinationComposeBoxControllerKey,
1322+
assert(_controller is FixedDestinationComposeBoxController);
1323+
return _FixedDestinationComposeBox(controller: (_controller as FixedDestinationComposeBoxController),
13311324
narrow: narrow);
13321325
case DmNarrow():
1333-
return _FixedDestinationComposeBox(key: _fixedDestinationComposeBoxControllerKey,
1326+
assert(_controller is FixedDestinationComposeBoxController);
1327+
return _FixedDestinationComposeBox(controller: (_controller as FixedDestinationComposeBoxController),
13341328
narrow: narrow);
13351329
case CombinedFeedNarrow():
13361330
case MentionsNarrow():

0 commit comments

Comments
 (0)