Skip to content
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

Rename "All messages" to "Combined feed" in the codebase. #688

Merged
merged 2 commits into from
May 28, 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
2 changes: 1 addition & 1 deletion integration_test/unreadmarker_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void main() {
child: PerAccountStoreWidget(
accountId: eg.selfAccount.id,
placeholder: const LoadingPlaceholderPage(),
child: const MessageListPage(narrow: AllMessagesNarrow())))));
child: const MessageListPage(narrow: CombinedFeedNarrow())))));
await tester.pumpAndSettle();
return messages;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/model/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
/// See also [_allMessagesVisible].
bool _messageVisible(Message message) {
switch (narrow) {
case AllMessagesNarrow():
case CombinedFeedNarrow():
return switch (message) {
StreamMessage() =>
store.isTopicVisible(message.streamId, message.subject),
Expand All @@ -355,7 +355,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
/// This is useful for an optimization.
bool get _allMessagesVisible {
switch (narrow) {
case AllMessagesNarrow():
case CombinedFeedNarrow():
case StreamNarrow():
return false;

Expand Down
15 changes: 7 additions & 8 deletions lib/model/narrow.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,12 @@ sealed class SendableNarrow extends Narrow {
MessageDestination get destination;
}

/// The narrow called "All messages" in the UI.
/// The narrow called "Combined feed" in the UI.
///
/// This does not literally mean all messages, or even all messages
/// that the user has access to: in particular it excludes muted streams
/// and topics.
Comment on lines -44 to -46
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether I should remove this comment now or not?

Copy link
Collaborator

@chrisbobbe chrisbobbe May 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I think we can replace it with something like:

/// All messages the user has access to, excluding unsubscribed streams
/// and muted streams and topics. See [PerAccountStore.isTopicVisible].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good done!

class AllMessagesNarrow extends Narrow {
const AllMessagesNarrow();
/// All messages the user has access to, excluding unsubscribed streams
/// and muted streams and topics. See [PerAccountStore.isTopicVisible].
class CombinedFeedNarrow extends Narrow {
const CombinedFeedNarrow();

@override
bool containsMessage(Message message) {
Expand All @@ -57,13 +56,13 @@ class AllMessagesNarrow extends Narrow {

@override
bool operator ==(Object other) {
if (other is! AllMessagesNarrow) return false;
if (other is! CombinedFeedNarrow) return false;
// Conceptually there's only one value of this type.
return true;
}

@override
int get hashCode => 'AllMessagesNarrow'.hashCode;
int get hashCode => 'CombinedFeedNarrow'.hashCode;
}

class StreamNarrow extends Narrow {
Expand Down
6 changes: 3 additions & 3 deletions lib/model/unreads.dart
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class Unreads extends ChangeNotifier {
final int selfUserId;

// TODO(#370): maintain this count incrementally, rather than recomputing from scratch
int countInAllMessagesNarrow() {
int countInCombinedFeedNarrow() {
int c = 0;
for (final messageIds in dms.values) {
c = c + messageIds.length;
Expand Down Expand Up @@ -195,8 +195,8 @@ class Unreads extends ChangeNotifier {

int countInNarrow(Narrow narrow) {
switch (narrow) {
case AllMessagesNarrow():
return countInAllMessagesNarrow();
case CombinedFeedNarrow():
return countInCombinedFeedNarrow();
case StreamNarrow():
return countInStreamNarrow(narrow.streamId);
case TopicNarrow():
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ class HomePage extends StatelessWidget {
ElevatedButton(
onPressed: () => Navigator.push(context,
MessageListPage.buildRoute(context: context,
narrow: const AllMessagesNarrow())),
narrow: const CombinedFeedNarrow())),
child: Text(zulipLocalizations.combinedFeedPageTitle)),
const SizedBox(height: 16),
ElevatedButton(
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ class ComposeBox extends StatelessWidget {
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
} else if (narrow is DmNarrow) {
return _FixedDestinationComposeBox(key: controllerKey, narrow: narrow);
} else if (narrow is AllMessagesNarrow) {
} else if (narrow is CombinedFeedNarrow) {
return const SizedBox.shrink();
} else {
throw Exception("impossible narrow"); // TODO(dart-3): show this statically
Expand Down
14 changes: 7 additions & 7 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class _MessageListPageState extends State<MessageListPage> {
final Color? appBarBackgroundColor;
bool removeAppBarBottomBorder = false;
switch(widget.narrow) {
case AllMessagesNarrow():
case CombinedFeedNarrow():
appBarBackgroundColor = null; // i.e., inherit

case StreamNarrow(:final streamId):
Expand Down Expand Up @@ -106,7 +106,7 @@ class _MessageListPageState extends State<MessageListPage> {
// if those details get complicated, refactor to avoid copying.
// TODO(#311) If we have a bottom nav, it will pad the bottom
// inset, and this should always be true.
removeBottom: widget.narrow is! AllMessagesNarrow,
removeBottom: widget.narrow is! CombinedFeedNarrow,

child: Expanded(
child: MessageList(narrow: widget.narrow))),
Expand Down Expand Up @@ -142,7 +142,7 @@ class MessageListAppBarTitle extends StatelessWidget {
final zulipLocalizations = ZulipLocalizations.of(context);

switch (narrow) {
case AllMessagesNarrow():
case CombinedFeedNarrow():
return Text(zulipLocalizations.combinedFeedPageTitle);

case StreamNarrow(:var streamId):
Expand Down Expand Up @@ -327,7 +327,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
// The keys are of type [ValueKey] with a value of [Message.id]
// and here we use a O(log n) binary search method. This could
// be improved but for now it only triggers for materialized
// widgets. As a simple test, flinging through All Messages in
// widgets. As a simple test, flinging through Combined feed in
// CZO on a Pixel 5, this only runs about 10 times per rebuild
// and the timing for each call is <100 microseconds.
//
Expand Down Expand Up @@ -447,7 +447,7 @@ class MarkAsReadWidget extends StatelessWidget {
return;
}
if (!context.mounted) return;
if (narrow is AllMessagesNarrow && !useLegacy) {
if (narrow is CombinedFeedNarrow && !useLegacy) {
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
}
}
Expand Down Expand Up @@ -507,7 +507,7 @@ class RecipientHeader extends StatelessWidget {
final message = this.message;
return switch (message) {
StreamMessage() => StreamMessageRecipientHeader(message: message,
showStream: narrow is AllMessagesNarrow),
showStream: narrow is CombinedFeedNarrow),
DmMessage() => DmRecipientHeader(message: message),
};
}
Expand Down Expand Up @@ -1081,7 +1081,7 @@ Future<void> _legacyMarkNarrowAsRead(BuildContext context, Narrow narrow) async
final store = PerAccountStoreWidget.of(context);
final connection = store.connection;
switch (narrow) {
case AllMessagesNarrow():
case CombinedFeedNarrow():
await markAllAsRead(connection);
case StreamNarrow(:final streamId):
await markStreamAsRead(connection, streamId: streamId);
Expand Down
10 changes: 5 additions & 5 deletions test/api/route/messages_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ void main() {
check(jsonEncode(narrow)).equals(expected);
}

checkNarrow(const AllMessagesNarrow().apiEncode(), jsonEncode([]));
checkNarrow(const CombinedFeedNarrow().apiEncode(), jsonEncode([]));
checkNarrow(const StreamNarrow(12).apiEncode(), jsonEncode([
{'operator': 'stream', 'operand': 12},
]));
Expand Down Expand Up @@ -246,7 +246,7 @@ void main() {
return FakeApiConnection.with_((connection) async {
connection.prepare(json: fakeResult.toJson());
await checkGetMessages(connection,
narrow: const AllMessagesNarrow().apiEncode(),
narrow: const CombinedFeedNarrow().apiEncode(),
anchor: AnchorCode.newest, numBefore: 10, numAfter: 20,
expected: {
'narrow': jsonEncode([]),
Expand Down Expand Up @@ -278,7 +278,7 @@ void main() {
return FakeApiConnection.with_((connection) async {
connection.prepare(json: fakeResult.toJson());
await checkGetMessages(connection,
narrow: const AllMessagesNarrow().apiEncode(),
narrow: const CombinedFeedNarrow().apiEncode(),
anchor: const NumericAnchor(42),
numBefore: 10, numAfter: 20,
expected: {
Expand Down Expand Up @@ -582,7 +582,7 @@ void main() {
await checkUpdateMessageFlagsForNarrow(connection,
anchor: AnchorCode.oldest,
numBefore: 0, numAfter: 20,
narrow: const AllMessagesNarrow().apiEncode(),
narrow: const CombinedFeedNarrow().apiEncode(),
op: UpdateMessageFlagsOp.add, flag: MessageFlag.read,
expected: {
'anchor': 'oldest',
Expand Down Expand Up @@ -622,7 +622,7 @@ void main() {
await checkUpdateMessageFlagsForNarrow(connection,
anchor: const NumericAnchor(42),
numBefore: 0, numAfter: 20,
narrow: const AllMessagesNarrow().apiEncode(),
narrow: const CombinedFeedNarrow().apiEncode(),
op: UpdateMessageFlagsOp.add, flag: MessageFlag.read,
expected: {
'anchor': '42',
Expand Down
10 changes: 5 additions & 5 deletions test/model/autocomplete_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ void main() {
});

test('MentionAutocompleteView misc', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
final store = eg.store();
await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]);
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
Expand All @@ -183,7 +183,7 @@ void main() {

test('MentionAutocompleteView not starve timers', () {
fakeAsync((binding) async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
final store = eg.store();
await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]);
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
Expand Down Expand Up @@ -218,7 +218,7 @@ void main() {
});

test('MentionAutocompleteView yield between batches of 1000', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
final store = eg.store();
for (int i = 0; i < 2500; i++) {
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
Expand All @@ -241,7 +241,7 @@ void main() {
});

test('MentionAutocompleteView new query during computation replaces old', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
final store = eg.store();
for (int i = 0; i < 1500; i++) {
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
Expand Down Expand Up @@ -275,7 +275,7 @@ void main() {
});

test('MentionAutocompleteView mutating store.users while in progress causes retry', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
final store = eg.store();
for (int i = 0; i < 1500; i++) {
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
Expand Down
6 changes: 3 additions & 3 deletions test/model/compose_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,11 @@ hello
});

group('narrowLink', () {
test('AllMessagesNarrow', () {
test('CombinedFeedNarrow', () {
final store = eg.store();
check(narrowLink(store, const AllMessagesNarrow()))
check(narrowLink(store, const CombinedFeedNarrow()))
.equals(store.realmUrl.resolve('#narrow'));
check(narrowLink(store, const AllMessagesNarrow(), nearMessageId: 1))
check(narrowLink(store, const CombinedFeedNarrow(), nearMessageId: 1))
.equals(store.realmUrl.resolve('#narrow/near/1'));
});

Expand Down
18 changes: 9 additions & 9 deletions test/model/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ void main() async {
void checkNotifiedOnce() => checkNotified(count: 1);

/// Initialize [model] and the rest of the test state.
Future<void> prepare({Narrow narrow = const AllMessagesNarrow()}) async {
Future<void> prepare({Narrow narrow = const CombinedFeedNarrow()}) async {
final stream = eg.stream();
subscription = eg.subscription(stream);
store = eg.store();
Expand Down Expand Up @@ -87,7 +87,7 @@ void main() async {
}

test('fetchInitial', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
await prepare(narrow: narrow);
connection.prepare(json: newestResult(
foundOldest: false,
Expand Down Expand Up @@ -140,7 +140,7 @@ void main() async {
});

test('fetchOlder', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
await prepare(narrow: narrow);
await prepareMessages(foundOldest: false,
messages: List.generate(100, (i) => eg.streamMessage(id: 1000 + i)));
Expand Down Expand Up @@ -168,7 +168,7 @@ void main() async {
});

test('fetchOlder nop when already fetching', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
await prepare(narrow: narrow);
await prepareMessages(foundOldest: false,
messages: List.generate(100, (i) => eg.streamMessage(id: 1000 + i)));
Expand Down Expand Up @@ -198,7 +198,7 @@ void main() async {
});

test('fetchOlder nop when already haveOldest true', () async {
await prepare(narrow: const AllMessagesNarrow());
await prepare(narrow: const CombinedFeedNarrow());
await prepareMessages(foundOldest: true, messages:
List.generate(30, (i) => eg.streamMessage()));
check(model)
Expand All @@ -216,7 +216,7 @@ void main() async {
});

test('fetchOlder handles servers not understanding includeAnchor', () async {
const narrow = AllMessagesNarrow();
const narrow = CombinedFeedNarrow();
await prepare(narrow: narrow);
await prepareMessages(foundOldest: false,
messages: List.generate(100, (i) => eg.streamMessage(id: 1000 + i)));
Expand Down Expand Up @@ -555,10 +555,10 @@ void main() async {
});

group('stream/topic muting', () {
test('in AllMessagesNarrow', () async {
test('in CombinedFeedNarrow', () async {
final stream1 = eg.stream(streamId: 1, name: 'stream 1');
final stream2 = eg.stream(streamId: 2, name: 'stream 2');
await prepare(narrow: const AllMessagesNarrow());
await prepare(narrow: const CombinedFeedNarrow());
await store.addStreams([stream1, stream2]);
await store.addSubscription(eg.subscription(stream1));
await store.addUserTopic(stream1, 'B', UserTopicVisibilityPolicy.muted);
Expand Down Expand Up @@ -923,7 +923,7 @@ void checkInvariants(MessageListView model) {

if (message is! StreamMessage) continue;
switch (model.narrow) {
case AllMessagesNarrow():
case CombinedFeedNarrow():
check(model.store.isTopicVisible(message.streamId, message.subject))
.isTrue();
case StreamNarrow():
Expand Down
4 changes: 2 additions & 2 deletions test/model/unreads_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ void main() {
});

group('count helpers', () {
test('countInAllMessagesNarrow', () async {
test('countInCombinedFeedNarrow', () async {
final stream1 = eg.stream(streamId: 1, name: 'stream 1');
final stream2 = eg.stream(streamId: 2, name: 'stream 2');
final stream3 = eg.stream(streamId: 3, name: 'stream 3');
Expand All @@ -171,7 +171,7 @@ void main() {
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []),
eg.dmMessage(from: eg.thirdUser, to: [eg.selfUser], flags: []),
]);
check(model.countInAllMessagesNarrow()).equals(5);
check(model.countInCombinedFeedNarrow()).equals(5);
});

test('countInStream/Narrow', () async {
Expand Down
4 changes: 2 additions & 2 deletions test/widgets/action_sheet_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,9 @@ void main() {
));
});

testWidgets('not offered in AllMessagesNarrow (composing to reply is not yet supported)', (WidgetTester tester) async {
testWidgets('not offered in CombinedFeedNarrow (composing to reply is not yet supported)', (WidgetTester tester) async {
final message = eg.streamMessage();
await setupToMessageActionSheet(tester, message: message, narrow: const AllMessagesNarrow());
await setupToMessageActionSheet(tester, message: message, narrow: const CombinedFeedNarrow());
check(findQuoteAndReplyButton(tester)).isNull();
});
});
Expand Down
Loading
Loading