Skip to content

Commit 1e45ca2

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 b5e37d6 commit 1e45ca2

File tree

5 files changed

+118
-84
lines changed

5 files changed

+118
-84
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-53
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+
}
11061114
}
11071115

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+
}
1129+
}
1130+
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,50 +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();
1140-
_topicFocusNode.dispose();
1153+
_controller.dispose();
11411154
super.dispose();
11421155
}
11431156

11441157
@override
11451158
Widget build(BuildContext context) {
1159+
final StreamComposeBoxController(
1160+
:topicController, :contentController, :topicFocusNode, :contentFocusNode,
1161+
) = controller;
1162+
11461163
return _ComposeBoxLayout(
1147-
contentController: _contentController,
1148-
contentFocusNode: _contentFocusNode,
1164+
contentController: contentController,
1165+
contentFocusNode: contentFocusNode,
11491166
topicInput: _TopicInput(
11501167
streamId: widget.narrow.streamId,
1151-
controller: _topicController,
1168+
controller: topicController,
11521169
focusNode: topicFocusNode,
1153-
contentFocusNode: _contentFocusNode,
1170+
contentFocusNode: contentFocusNode,
11541171
),
11551172
contentInput: _StreamContentInput(
11561173
narrow: widget.narrow,
1157-
topicController: _topicController,
1158-
controller: _contentController,
1159-
focusNode: _contentFocusNode,
1174+
topicController: topicController,
1175+
controller: contentController,
1176+
focusNode: contentFocusNode,
11601177
),
11611178
sendButton: _SendButton(
1162-
topicController: _topicController,
1163-
contentController: _contentController,
1179+
topicController: topicController,
1180+
contentController: contentController,
11641181
getDestination: () => StreamDestination(
1165-
widget.narrow.streamId, _topicController.textNormalized),
1182+
widget.narrow.streamId, topicController.textNormalized),
11661183
));
11671184
}
11681185
}
@@ -1197,46 +1214,43 @@ class _FixedDestinationComposeBox extends StatefulWidget {
11971214
State<_FixedDestinationComposeBox> createState() => _FixedDestinationComposeBoxState();
11981215
}
11991216

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

12091221
@override
12101222
void dispose() {
1211-
_contentController.dispose();
1212-
_contentFocusNode.dispose();
1223+
_controller.dispose();
12131224
super.dispose();
12141225
}
12151226

12161227
@override
12171228
Widget build(BuildContext context) {
1229+
final FixedDestinationComposeBoxController(
1230+
:contentController, :contentFocusNode,
1231+
) = controller;
1232+
12181233
return _ComposeBoxLayout(
1219-
contentController: _contentController,
1220-
contentFocusNode: _contentFocusNode,
1234+
contentController: contentController,
1235+
contentFocusNode: contentFocusNode,
12211236
topicInput: null,
12221237
contentInput: _FixedDestinationContentInput(
12231238
narrow: widget.narrow,
1224-
controller: _contentController,
1225-
focusNode: _contentFocusNode,
1239+
controller: contentController,
1240+
focusNode: contentFocusNode,
12261241
),
12271242
sendButton: _SendButton(
12281243
topicController: null,
1229-
contentController: _contentController,
1244+
contentController: contentController,
12301245
getDestination: () => widget.narrow.destination,
12311246
));
12321247
}
12331248
}
12341249

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

1239-
final GlobalKey<ComposeBoxController>? controllerKey;
12401254
final Narrow narrow;
12411255

12421256
static bool hasComposeBox(Narrow narrow) {
@@ -1253,10 +1267,30 @@ class ComposeBox extends StatelessWidget {
12531267
}
12541268
}
12551269

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+
12561290
Widget? _errorBanner(BuildContext context) {
12571291
final store = PerAccountStoreWidget.of(context);
12581292
final selfUser = store.users[store.selfUserId]!;
1259-
switch (narrow) {
1293+
switch (widget.narrow) {
12601294
case ChannelNarrow(:final streamId):
12611295
case TopicNarrow(:final streamId):
12621296
final channel = store.streams[streamId];
@@ -1287,14 +1321,16 @@ class ComposeBox extends StatelessWidget {
12871321
return _ComposeBoxContainer(child: errorBanner);
12881322
}
12891323

1290-
final narrow = this.narrow;
1324+
final narrow = widget.narrow;
12911325
switch (narrow) {
12921326
case ChannelNarrow():
1293-
return _StreamComposeBox(key: controllerKey, narrow: narrow);
1327+
return _StreamComposeBox(key: _streamComposeBoxControllerKey, narrow: narrow);
12941328
case TopicNarrow():
1295-
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
1329+
return _FixedDestinationComposeBox(key: _fixedDestinationComposeBoxControllerKey,
1330+
narrow: narrow);
12961331
case DmNarrow():
1297-
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
1332+
return _FixedDestinationComposeBox(key: _fixedDestinationComposeBoxControllerKey,
1333+
narrow: narrow);
12981334
case CombinedFeedNarrow():
12991335
case MentionsNarrow():
13001336
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)