Skip to content

msglist: Provide narrow to MessageListPage descendants #811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
32 changes: 26 additions & 6 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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});

Expand All @@ -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;
Expand All @@ -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<MessageListPage> {
class _MessageListPageState extends State<MessageListPage> implements MessageListPageState {
@override
Narrow get narrow => widget.narrow;

@override
ComposeBoxController? get composeBoxController => _composeBoxKey.currentState;

final GlobalKey<ComposeBoxController> _composeBoxKey = GlobalKey();

@override
Expand Down
41 changes: 41 additions & 0 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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: "<p>a message</p>")]);
final expectedState = tester.state<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: "<p>a message</p>")]);
final element = tester.element(find.byType(PerAccountStoreWidget));
check(() => MessageListPage.ancestorOf(element))
.throws<void>();
});

testWidgets('MessageListPageState.narrow', (tester) async {
final stream = eg.stream();
await setupMessageListPage(tester, narrow: StreamNarrow(stream.streamId),
messages: [eg.streamMessage(stream: stream, content: "<p>a message</p>")]);
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: "<p>a message</p>")]);
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: "<p>a message</p>")]);
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 {
Expand Down