Skip to content

Commit f351eae

Browse files
chrisbobbegnprice
authored andcommitted
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: #1561 (comment)
1 parent 6214c61 commit f351eae

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
@@ -697,20 +697,20 @@ class MessageListView with ChangeNotifier, _MessageSequence {
697697

698698
/// Whether this event could affect the result that [_messageVisible]
699699
/// would ever have returned for any possible message in this message list.
700-
VisibilityEffect _canAffectVisibility(UserTopicEvent event) {
700+
UserTopicVisibilityEffect _userTopicEventCanAffectVisibility(UserTopicEvent event) {
701701
switch (narrow) {
702702
case CombinedFeedNarrow():
703703
return store.willChangeIfTopicVisible(event);
704704

705705
case ChannelNarrow(:final streamId):
706-
if (event.streamId != streamId) return VisibilityEffect.none;
706+
if (event.streamId != streamId) return UserTopicVisibilityEffect.none;
707707
return store.willChangeIfTopicVisibleInStream(event);
708708

709709
case TopicNarrow():
710710
case DmNarrow():
711711
case MentionsNarrow():
712712
case StarredMessagesNarrow():
713-
return VisibilityEffect.none;
713+
return UserTopicVisibilityEffect.none;
714714
}
715715
}
716716

@@ -986,11 +986,11 @@ class MessageListView with ChangeNotifier, _MessageSequence {
986986
}
987987

988988
void handleUserTopicEvent(UserTopicEvent event) {
989-
switch (_canAffectVisibility(event)) {
990-
case VisibilityEffect.none:
989+
switch (_userTopicEventCanAffectVisibility(event)) {
990+
case UserTopicVisibilityEffect.none:
991991
return;
992992

993-
case VisibilityEffect.muted:
993+
case UserTopicVisibilityEffect.muted:
994994
bool removed = _removeMessagesWhere((message) =>
995995
message is StreamMessage
996996
&& message.streamId == event.streamId
@@ -1005,7 +1005,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
10051005
notifyListeners();
10061006
}
10071007

1008-
case VisibilityEffect.unmuted:
1008+
case UserTopicVisibilityEffect.unmuted:
10091009
// TODO get the newly-unmuted messages from the message store
10101010
// For now, we simplify the task by just refetching this message list
10111011
// 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)