Skip to content

Commit 7cef3aa

Browse files
committed
msglist [nfc]: s/VisibilityEffect/UserTopicVisibilityEffect/
This is awkwardly verbose, but we're about to add another kind of visibility effect, and I think the code will end up clearer if we make a separate enum for it. Greg points out that an upcoming Dart feature "dot shorthands" should help with this verbosity: zulip#1561 (comment)
1 parent 1318605 commit 7cef3aa

File tree

3 files changed

+26
-25
lines changed

3 files changed

+26
-25
lines changed

lib/model/channel.dart

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ mixin ChannelStore {
6969

7070
/// Whether the given event will change the result of [isTopicVisibleInStream]
7171
/// for its stream and topic, compared to the current state.
72-
VisibilityEffect willChangeIfTopicVisibleInStream(UserTopicEvent event) {
72+
UserTopicVisibilityEffect willChangeIfTopicVisibleInStream(UserTopicEvent event) {
7373
final streamId = event.streamId;
7474
final topic = event.topicName;
75-
return VisibilityEffect._fromBeforeAfter(
75+
return UserTopicVisibilityEffect._fromBeforeAfter(
7676
_isTopicVisibleInStream(topicVisibilityPolicy(streamId, topic)),
7777
_isTopicVisibleInStream(event.visibilityPolicy));
7878
}
@@ -106,10 +106,10 @@ mixin ChannelStore {
106106

107107
/// Whether the given event will change the result of [isTopicVisible]
108108
/// for its stream and topic, compared to the current state.
109-
VisibilityEffect willChangeIfTopicVisible(UserTopicEvent event) {
109+
UserTopicVisibilityEffect willChangeIfTopicVisible(UserTopicEvent event) {
110110
final streamId = event.streamId;
111111
final topic = event.topicName;
112-
return VisibilityEffect._fromBeforeAfter(
112+
return UserTopicVisibilityEffect._fromBeforeAfter(
113113
_isTopicVisible(streamId, topicVisibilityPolicy(streamId, topic)),
114114
_isTopicVisible(streamId, event.visibilityPolicy));
115115
}
@@ -137,7 +137,7 @@ mixin ChannelStore {
137137
/// Whether and how a given [UserTopicEvent] will affect the results
138138
/// that [ChannelStore.isTopicVisible] or [ChannelStore.isTopicVisibleInStream]
139139
/// would give for some messages.
140-
enum VisibilityEffect {
140+
enum UserTopicVisibilityEffect {
141141
/// The event will have no effect on the visibility results.
142142
none,
143143

@@ -147,11 +147,11 @@ enum VisibilityEffect {
147147
/// The event will change some visibility results from false to true.
148148
unmuted;
149149

150-
factory VisibilityEffect._fromBeforeAfter(bool before, bool after) {
150+
factory UserTopicVisibilityEffect._fromBeforeAfter(bool before, bool after) {
151151
return switch ((before, after)) {
152-
(false, true) => VisibilityEffect.unmuted,
153-
(true, false) => VisibilityEffect.muted,
154-
_ => VisibilityEffect.none,
152+
(false, true) => UserTopicVisibilityEffect.unmuted,
153+
(true, false) => UserTopicVisibilityEffect.muted,
154+
_ => UserTopicVisibilityEffect.none,
155155
};
156156
}
157157
}

lib/model/message_list.dart

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -661,20 +661,20 @@ class MessageListView with ChangeNotifier, _MessageSequence {
661661

662662
/// Whether this event could affect the result that [_messageVisible]
663663
/// would ever have returned for any possible message in this message list.
664-
VisibilityEffect _canAffectVisibility(UserTopicEvent event) {
664+
UserTopicVisibilityEffect _userTopicEventCanAffectVisibility(UserTopicEvent event) {
665665
switch (narrow) {
666666
case CombinedFeedNarrow():
667667
return store.willChangeIfTopicVisible(event);
668668

669669
case ChannelNarrow(:final streamId):
670-
if (event.streamId != streamId) return VisibilityEffect.none;
670+
if (event.streamId != streamId) return UserTopicVisibilityEffect.none;
671671
return store.willChangeIfTopicVisibleInStream(event);
672672

673673
case TopicNarrow():
674674
case DmNarrow():
675675
case MentionsNarrow():
676676
case StarredMessagesNarrow():
677-
return VisibilityEffect.none;
677+
return UserTopicVisibilityEffect.none;
678678
}
679679
}
680680

@@ -950,11 +950,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
950950
}
951951

952952
void handleUserTopicEvent(UserTopicEvent event) {
953-
switch (_canAffectVisibility(event)) {
954-
case VisibilityEffect.none:
953+
switch (_userTopicEventCanAffectVisibility(event)) {
954+
case UserTopicVisibilityEffect.none:
955955
return;
956956

957-
case VisibilityEffect.muted:
957+
case UserTopicVisibilityEffect.muted:
958958
bool removed = _removeMessagesWhere((message) =>
959959
message is StreamMessage
960960
&& message.streamId == event.streamId
@@ -969,7 +969,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
969969
notifyListeners();
970970
}
971971

972-
case VisibilityEffect.unmuted:
972+
case UserTopicVisibilityEffect.unmuted:
973973
// TODO get the newly-unmuted messages from the message store
974974
// For now, we simplify the task by just refetching this message list
975975
// from scratch.

test/model/channel_test.dart

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,8 @@ void main() {
223223

224224
void checkChanges(PerAccountStore store,
225225
UserTopicVisibilityPolicy newPolicy,
226-
VisibilityEffect expectedInStream, VisibilityEffect expectedOverall) {
226+
UserTopicVisibilityEffect expectedInStream,
227+
UserTopicVisibilityEffect expectedOverall) {
227228
final event = mkEvent(newPolicy);
228229
check(store.willChangeIfTopicVisibleInStream(event)).equals(expectedInStream);
229230
check(store.willChangeIfTopicVisible (event)).equals(expectedOverall);
@@ -234,31 +235,31 @@ void main() {
234235
await store.addStream(stream1);
235236
await store.addSubscription(eg.subscription(stream1));
236237
checkChanges(store, UserTopicVisibilityPolicy.followed,
237-
VisibilityEffect.none, VisibilityEffect.none);
238+
UserTopicVisibilityEffect.none, UserTopicVisibilityEffect.none);
238239
});
239240

240241
test('stream not muted, policy none -> muted, means muted', () async {
241242
final store = eg.store();
242243
await store.addStream(stream1);
243244
await store.addSubscription(eg.subscription(stream1));
244245
checkChanges(store, UserTopicVisibilityPolicy.muted,
245-
VisibilityEffect.muted, VisibilityEffect.muted);
246+
UserTopicVisibilityEffect.muted, UserTopicVisibilityEffect.muted);
246247
});
247248

248249
test('stream muted, policy none -> followed, means none/unmuted', () async {
249250
final store = eg.store();
250251
await store.addStream(stream1);
251252
await store.addSubscription(eg.subscription(stream1, isMuted: true));
252253
checkChanges(store, UserTopicVisibilityPolicy.followed,
253-
VisibilityEffect.none, VisibilityEffect.unmuted);
254+
UserTopicVisibilityEffect.none, UserTopicVisibilityEffect.unmuted);
254255
});
255256

256257
test('stream muted, policy none -> muted, means muted/none', () async {
257258
final store = eg.store();
258259
await store.addStream(stream1);
259260
await store.addSubscription(eg.subscription(stream1, isMuted: true));
260261
checkChanges(store, UserTopicVisibilityPolicy.muted,
261-
VisibilityEffect.muted, VisibilityEffect.none);
262+
UserTopicVisibilityEffect.muted, UserTopicVisibilityEffect.none);
262263
});
263264

264265
final policies = [
@@ -293,10 +294,10 @@ void main() {
293294
final newVisibleInStream = store.isTopicVisibleInStream(stream1.streamId, eg.t('topic'));
294295
final newVisible = store.isTopicVisible(stream1.streamId, eg.t('topic'));
295296

296-
VisibilityEffect fromOldNew(bool oldVisible, bool newVisible) {
297-
if (newVisible == oldVisible) return VisibilityEffect.none;
298-
if (newVisible) return VisibilityEffect.unmuted;
299-
return VisibilityEffect.muted;
297+
UserTopicVisibilityEffect fromOldNew(bool oldVisible, bool newVisible) {
298+
if (newVisible == oldVisible) return UserTopicVisibilityEffect.none;
299+
if (newVisible) return UserTopicVisibilityEffect.unmuted;
300+
return UserTopicVisibilityEffect.muted;
300301
}
301302
check(willChangeInStream)
302303
.equals(fromOldNew(oldVisibleInStream, newVisibleInStream));

0 commit comments

Comments
 (0)