diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index b224d74769..9ff17385ec 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -26,7 +26,8 @@ void showMessageActionSheet({required BuildContext context, required Message mes // of the action sheet (we avoid calling composeBoxControllerOf in a build // method; see its doc). But currently it will be constant through the life of // any message list, so that's fine. - final isComposeBoxOffered = MessageListPage.composeBoxControllerOf(context) != null; + final messageListPage = MessageListPage.ancestorOf(context); + final isComposeBoxOffered = messageListPage.composeBoxController != null; final hasThumbsUpReactionVote = message.reactions ?.aggregated.any((reactionWithVotes) => @@ -239,7 +240,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { // message action sheet opened, and before "Quote and reply" was pressed. // Currently a compose box can't ever disappear, so this is impossible. ComposeBoxController composeBoxController = - MessageListPage.composeBoxControllerOf(messageListContext)!; + MessageListPage.ancestorOf(messageListContext).composeBoxController!; final topicController = composeBoxController.topicController; if ( topicController != null @@ -264,7 +265,8 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { // This will be null only if the compose box disappeared during the // quotation request. Currently a compose box can't ever disappear, // so this is impossible. - composeBoxController = MessageListPage.composeBoxControllerOf(messageListContext)!; + composeBoxController = + MessageListPage.ancestorOf(messageListContext).composeBoxController!; composeBoxController.contentController .registerQuoteAndReplyEnd(PerAccountStoreWidget.of(messageListContext), tag, message: message, diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 329dabf58d..5b620c7592 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -25,6 +25,18 @@ import 'edit_state_marker.dart'; import 'text.dart'; import 'theme.dart'; +/// The interface for the state of a [MessageListPage]. +/// +/// To obtain one of these, see [MessageListPage.ancestorOf]. +abstract class MessageListPageState { + /// The narrow for this page's message list. + Narrow get narrow; + + /// The controller for this [MessageListPage]'s compose box, + /// if this [MessageListPage] offers a compose box. + ComposeBoxController? get composeBoxController; +} + class MessageListPage extends StatefulWidget { const MessageListPage({super.key, required this.narrow}); @@ -34,14 +46,16 @@ class MessageListPage extends StatefulWidget { page: MessageListPage(narrow: narrow)); } - /// A [ComposeBoxController], if this [MessageListPage] offers a compose box. + /// The [MessageListPageState] above this context in the tree. /// /// Uses the inefficient [BuildContext.findAncestorStateOfType]; /// don't call this in a build method. - static ComposeBoxController? composeBoxControllerOf(BuildContext context) { - final messageListPageState = context.findAncestorStateOfType<_MessageListPageState>(); - assert(messageListPageState != null, 'No MessageListPage ancestor'); - return messageListPageState!._composeBoxKey.currentState; + // If we do find ourselves wanting this in a build method, it won't be hard + // to enable that: we'd just need to add an [InheritedWidget] here. + static MessageListPageState ancestorOf(BuildContext context) { + final state = context.findAncestorStateOfType<_MessageListPageState>(); + assert(state != null, 'No MessageListPage ancestor'); + return state!; } final Narrow narrow; @@ -53,7 +67,13 @@ class MessageListPage extends StatefulWidget { // TODO(design) this seems ad-hoc; is there a better color? const _kUnsubscribedStreamRecipientHeaderColor = Color(0xfff5f5f5); -class _MessageListPageState extends State { +class _MessageListPageState extends State implements MessageListPageState { + @override + Narrow get narrow => widget.narrow; + + @override + ComposeBoxController? get composeBoxController => _composeBoxKey.currentState; + final GlobalKey _composeBoxKey = GlobalKey(); @override diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 41e19b8f3c..bf14b874c0 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -88,6 +88,47 @@ void main() { return scrollView.controller; } + group('MessageListPage', () { + testWidgets('ancestorOf finds page state from message', (tester) async { + await setupMessageListPage(tester, + messages: [eg.streamMessage(content: "

a message

")]); + final expectedState = tester.state(find.byType(MessageListPage)); + check(MessageListPage.ancestorOf(tester.element(find.text("a message")))) + .identicalTo(expectedState as MessageListPageState); + }); + + testWidgets('ancestorOf throws when not a descendant of MessageListPage', (tester) async { + await setupMessageListPage(tester, + messages: [eg.streamMessage(content: "

a message

")]); + final element = tester.element(find.byType(PerAccountStoreWidget)); + check(() => MessageListPage.ancestorOf(element)) + .throws(); + }); + + testWidgets('MessageListPageState.narrow', (tester) async { + final stream = eg.stream(); + await setupMessageListPage(tester, narrow: StreamNarrow(stream.streamId), + messages: [eg.streamMessage(stream: stream, content: "

a message

")]); + final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); + check(state.narrow).equals(StreamNarrow(stream.streamId)); + }); + + testWidgets('composeBoxController finds compose box', (tester) async { + final stream = eg.stream(); + await setupMessageListPage(tester, narrow: StreamNarrow(stream.streamId), + messages: [eg.streamMessage(stream: stream, content: "

a message

")]); + final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); + check(state.composeBoxController).isNotNull(); + }); + + testWidgets('composeBoxController null when no compose box', (tester) async { + await setupMessageListPage(tester, narrow: const CombinedFeedNarrow(), + messages: [eg.streamMessage(content: "

a message

")]); + final state = MessageListPage.ancestorOf(tester.element(find.text("a message"))); + check(state.composeBoxController).isNull(); + }); + }); + group('presents message content appropriately', () { // regression test for https://github.com/zulip/zulip-flutter/issues/736 testWidgets('content in "Combined feed" not asked to consume insets (including bottom)', (tester) async {