Skip to content

Commit 09c1421

Browse files
committed
compose [nfc]: Make state "has-a" instead of "is-a" ComposeBoxController
We've been using ComposeBoxController as an interface, implemented by _StreamComposeBoxState and _FixedDestinationComposeBoxState. This commit puts the relevant implementation in ComposeBoxController itself -- really in subclasses StreamComposeBoxController and FixedDestinationComposeBoxController -- and makes those _FooState classes instantiate it and store the instance in a field. Next, we'll move the field up from those _FooState classes onto ComposeBox's state to make the controller straightforwardly available there (i.e. available without going through GlobalKey logic). That will be helpful for zulip#720 when the controller grows a send-in-progress flag; ComposeBox will be a conveniently central place to build a progress indicator for that state.
1 parent 8561937 commit 09c1421

File tree

5 files changed

+120
-83
lines changed

5 files changed

+120
-83
lines changed

lib/widgets/action_sheet.dart

+4-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import '../model/narrow.dart';
1414
import 'actions.dart';
1515
import 'clipboard.dart';
1616
import 'color.dart';
17+
import 'compose_box.dart';
1718
import 'dialog.dart';
1819
import 'icons.dart';
1920
import 'inset_shadow.dart';
@@ -331,13 +332,12 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
331332
// or when a DM recipient becomes deactivated.
332333
var composeBoxController = findMessageListPage().composeBoxController;
333334
if (composeBoxController == null) return;
334-
final topicController = composeBoxController.topicController;
335335
if (
336-
topicController != null
337-
&& topicController.textNormalized == kNoTopicTopic
336+
composeBoxController is StreamComposeBoxController
337+
&& composeBoxController.topicController.textNormalized == kNoTopicTopic
338338
&& message is StreamMessage
339339
) {
340-
topicController.value = TextEditingValue(text: message.topic);
340+
composeBoxController.topicController.value = TextEditingValue(text: message.topic);
341341
}
342342

343343
// This inserts a "[Quoting…]" placeholder into the content input,

lib/widgets/compose_box.dart

+89-52
Original file line numberDiff line numberDiff line change
@@ -1099,12 +1099,37 @@ class _ComposeBoxLayout extends StatelessWidget {
10991099
}
11001100
}
11011101

1102-
abstract class ComposeBoxController<T extends StatefulWidget> extends State<T> {
1103-
ComposeTopicController? get topicController;
1104-
ComposeContentController get contentController;
1105-
FocusNode get contentFocusNode;
1102+
sealed class ComposeBoxController {
1103+
ComposeContentController get contentController => _contentController;
1104+
final _contentController = ComposeContentController();
1105+
1106+
FocusNode get contentFocusNode => _contentFocusNode;
1107+
final _contentFocusNode = FocusNode();
1108+
1109+
@mustCallSuper
1110+
void dispose() {
1111+
_contentController.dispose();
1112+
_contentFocusNode.dispose();
1113+
}
1114+
}
1115+
1116+
class StreamComposeBoxController extends ComposeBoxController {
1117+
ComposeTopicController get topicController => _topicController;
1118+
final _topicController = ComposeTopicController();
1119+
1120+
FocusNode get topicFocusNode => _topicFocusNode;
1121+
final _topicFocusNode = FocusNode();
1122+
1123+
@override
1124+
void dispose() {
1125+
_topicController.dispose();
1126+
_topicFocusNode.dispose();
1127+
super.dispose();
1128+
}
11061129
}
11071130

1131+
class FixedDestinationComposeBoxController extends ComposeBoxController {}
1132+
11081133
/// A compose box for use in a channel narrow.
11091134
///
11101135
/// This offers a text input for the topic to send to,
@@ -1119,49 +1144,42 @@ class _StreamComposeBox extends StatefulWidget {
11191144
State<_StreamComposeBox> createState() => _StreamComposeBoxState();
11201145
}
11211146

1122-
class _StreamComposeBoxState extends State<_StreamComposeBox> implements ComposeBoxController<_StreamComposeBox> {
1123-
@override ComposeTopicController get topicController => _topicController;
1124-
final _topicController = ComposeTopicController();
1125-
1126-
@override ComposeContentController get contentController => _contentController;
1127-
final _contentController = ComposeContentController();
1128-
1129-
@override FocusNode get contentFocusNode => _contentFocusNode;
1130-
final _contentFocusNode = FocusNode();
1131-
1132-
FocusNode get topicFocusNode => _topicFocusNode;
1133-
final _topicFocusNode = FocusNode();
1147+
class _StreamComposeBoxState extends State<_StreamComposeBox> {
1148+
StreamComposeBoxController get controller => _controller;
1149+
final _controller = StreamComposeBoxController();
11341150

11351151
@override
11361152
void dispose() {
1137-
_topicController.dispose();
1138-
_contentController.dispose();
1139-
_contentFocusNode.dispose();
1153+
_controller.dispose();
11401154
super.dispose();
11411155
}
11421156

11431157
@override
11441158
Widget build(BuildContext context) {
1159+
final StreamComposeBoxController(
1160+
:topicController, :contentController, :topicFocusNode, :contentFocusNode,
1161+
) = controller;
1162+
11451163
return _ComposeBoxLayout(
1146-
contentController: _contentController,
1147-
contentFocusNode: _contentFocusNode,
1164+
contentController: contentController,
1165+
contentFocusNode: contentFocusNode,
11481166
topicInput: _TopicInput(
11491167
streamId: widget.narrow.streamId,
1150-
controller: _topicController,
1168+
controller: topicController,
11511169
focusNode: topicFocusNode,
1152-
contentFocusNode: _contentFocusNode,
1170+
contentFocusNode: contentFocusNode,
11531171
),
11541172
contentInput: _StreamContentInput(
11551173
narrow: widget.narrow,
1156-
topicController: _topicController,
1157-
controller: _contentController,
1158-
focusNode: _contentFocusNode,
1174+
topicController: topicController,
1175+
controller: contentController,
1176+
focusNode: contentFocusNode,
11591177
),
11601178
sendButton: _SendButton(
1161-
topicController: _topicController,
1162-
contentController: _contentController,
1179+
topicController: topicController,
1180+
contentController: contentController,
11631181
getDestination: () => StreamDestination(
1164-
widget.narrow.streamId, _topicController.textNormalized),
1182+
widget.narrow.streamId, topicController.textNormalized),
11651183
));
11661184
}
11671185
}
@@ -1196,46 +1214,43 @@ class _FixedDestinationComposeBox extends StatefulWidget {
11961214
State<_FixedDestinationComposeBox> createState() => _FixedDestinationComposeBoxState();
11971215
}
11981216

1199-
class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox> implements ComposeBoxController<_FixedDestinationComposeBox> {
1200-
@override ComposeTopicController? get topicController => null;
1201-
1202-
@override ComposeContentController get contentController => _contentController;
1203-
final _contentController = ComposeContentController();
1204-
1205-
@override FocusNode get contentFocusNode => _contentFocusNode;
1206-
final _contentFocusNode = FocusNode();
1217+
class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox> {
1218+
FixedDestinationComposeBoxController get controller => _controller;
1219+
final _controller = FixedDestinationComposeBoxController();
12071220

12081221
@override
12091222
void dispose() {
1210-
_contentController.dispose();
1211-
_contentFocusNode.dispose();
1223+
_controller.dispose();
12121224
super.dispose();
12131225
}
12141226

12151227
@override
12161228
Widget build(BuildContext context) {
1229+
final FixedDestinationComposeBoxController(
1230+
:contentController, :contentFocusNode,
1231+
) = controller;
1232+
12171233
return _ComposeBoxLayout(
1218-
contentController: _contentController,
1219-
contentFocusNode: _contentFocusNode,
1234+
contentController: contentController,
1235+
contentFocusNode: contentFocusNode,
12201236
topicInput: null,
12211237
contentInput: _FixedDestinationContentInput(
12221238
narrow: widget.narrow,
1223-
controller: _contentController,
1224-
focusNode: _contentFocusNode,
1239+
controller: contentController,
1240+
focusNode: contentFocusNode,
12251241
),
12261242
sendButton: _SendButton(
12271243
topicController: null,
1228-
contentController: _contentController,
1244+
contentController: contentController,
12291245
getDestination: () => widget.narrow.destination,
12301246
));
12311247
}
12321248
}
12331249

1234-
class ComposeBox extends StatelessWidget {
1235-
ComposeBox({super.key, this.controllerKey, required this.narrow})
1250+
class ComposeBox extends StatefulWidget {
1251+
ComposeBox({super.key, required this.narrow})
12361252
: assert(ComposeBox.hasComposeBox(narrow));
12371253

1238-
final GlobalKey<ComposeBoxController>? controllerKey;
12391254
final Narrow narrow;
12401255

12411256
static bool hasComposeBox(Narrow narrow) {
@@ -1252,10 +1267,30 @@ class ComposeBox extends StatelessWidget {
12521267
}
12531268
}
12541269

1270+
@override
1271+
State<ComposeBox> createState() => _ComposeBoxState();
1272+
}
1273+
1274+
/// The interface for the state of a [ComposeBox].
1275+
abstract class ComposeBoxState extends State<ComposeBox> {
1276+
ComposeBoxController? get controller;
1277+
}
1278+
1279+
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;
1286+
}
1287+
final _streamComposeBoxControllerKey = GlobalKey<_StreamComposeBoxState>();
1288+
final _fixedDestinationComposeBoxControllerKey = GlobalKey<_FixedDestinationComposeBoxState>();
1289+
12551290
Widget? _errorBanner(BuildContext context) {
12561291
final store = PerAccountStoreWidget.of(context);
12571292
final selfUser = store.users[store.selfUserId]!;
1258-
switch (narrow) {
1293+
switch (widget.narrow) {
12591294
case ChannelNarrow(:final streamId):
12601295
case TopicNarrow(:final streamId):
12611296
final channel = store.streams[streamId];
@@ -1286,14 +1321,16 @@ class ComposeBox extends StatelessWidget {
12861321
return _ComposeBoxContainer(child: errorBanner);
12871322
}
12881323

1289-
final narrow = this.narrow;
1324+
final narrow = widget.narrow;
12901325
switch (narrow) {
12911326
case ChannelNarrow():
1292-
return _StreamComposeBox(key: controllerKey, narrow: narrow);
1327+
return _StreamComposeBox(key: _streamComposeBoxControllerKey, narrow: narrow);
12931328
case TopicNarrow():
1294-
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
1329+
return _FixedDestinationComposeBox(key: _fixedDestinationComposeBoxControllerKey,
1330+
narrow: narrow);
12951331
case DmNarrow():
1296-
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
1332+
return _FixedDestinationComposeBox(key: _fixedDestinationComposeBoxControllerKey,
1333+
narrow: narrow);
12971334
case CombinedFeedNarrow():
12981335
case MentionsNarrow():
12991336
case StarredMessagesNarrow():

lib/widgets/message_list.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -213,9 +213,9 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
213213
late Narrow narrow;
214214

215215
@override
216-
ComposeBoxController? get composeBoxController => _composeBoxKey.currentState;
216+
ComposeBoxController? get composeBoxController => _composeBoxKey.currentState?.controller;
217217

218-
final GlobalKey<ComposeBoxController> _composeBoxKey = GlobalKey();
218+
final GlobalKey<ComposeBoxState> _composeBoxKey = GlobalKey();
219219

220220
@override
221221
void initState() {
@@ -302,7 +302,7 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
302302
child: Expanded(
303303
child: MessageList(narrow: narrow, onNarrowChanged: _narrowChanged))),
304304
if (ComposeBox.hasComposeBox(narrow))
305-
ComposeBox(controllerKey: _composeBoxKey, narrow: narrow)
305+
ComposeBox(key: _composeBoxKey, narrow: narrow)
306306
]))));
307307
}
308308
}

test/widgets/action_sheet_test.dart

+4-4
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ void main() {
230230

231231
group('QuoteAndReplyButton', () {
232232
ComposeBoxController? findComposeBoxController(WidgetTester tester) {
233-
return tester.widgetList<ComposeBox>(find.byType(ComposeBox))
234-
.singleOrNull?.controllerKey?.currentState;
233+
return tester.stateList<ComposeBoxState>(find.byType(ComposeBox))
234+
.singleOrNull?.controller;
235235
}
236236

237237
Widget? findQuoteAndReplyButton(WidgetTester tester) {
@@ -283,14 +283,14 @@ void main() {
283283
final message = eg.streamMessage();
284284
await setupToMessageActionSheet(tester, message: message, narrow: ChannelNarrow(message.streamId));
285285

286-
final composeBoxController = findComposeBoxController(tester)!;
286+
final composeBoxController = findComposeBoxController(tester) as StreamComposeBoxController;
287287
final contentController = composeBoxController.contentController;
288288

289289
// Ensure channel-topics are loaded before testing quote & reply behavior
290290
connection.prepare(body:
291291
jsonEncode(GetStreamTopicsResult(topics: [eg.getStreamTopicsEntry()]).toJson()));
292292
final topicController = composeBoxController.topicController;
293-
topicController?.value = const TextEditingValue(text: kNoTopicTopic);
293+
topicController.value = const TextEditingValue(text: kNoTopicTopic);
294294

295295
final valueBefore = contentController.value;
296296
prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world');

0 commit comments

Comments
 (0)