Skip to content

Commit

Permalink
msglist: Handle loading state in MarkAsReadWidget
Browse files Browse the repository at this point in the history 
MarkAsReadWidget needs to be disabled during loading state to prevent
multiple requests to the server.
  • Loading branch information
@Khader-1 @gnprice
Khader-1 authored and gnprice committed Jul 28, 2024
1 parent 016f22a commit 0d577a0
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 11 deletions.
36 changes: 25 additions & 11 deletions lib/widgets/message_list.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -458,30 +458,40 @@ class ScrollToBottomButton extends StatelessWidget {
}
}

class MarkAsReadWidget extends StatelessWidget {
class MarkAsReadWidget extends StatefulWidget {
const MarkAsReadWidget({super.key, required this.narrow});

final Narrow narrow;

@override
State<MarkAsReadWidget> createState() => _MarkAsReadWidgetState();
}

class _MarkAsReadWidgetState extends State<MarkAsReadWidget> {
bool _loading = false;

void _handlePress(BuildContext context) async {
if (!context.mounted) return;

final store = PerAccountStoreWidget.of(context);
final connection = store.connection;
final useLegacy = connection.zulipFeatureLevel! < 155;
setState(() => _loading = true);

try {
await markNarrowAsRead(context, narrow, useLegacy);
await markNarrowAsRead(context, widget.narrow, useLegacy);
} catch (e) {
if (!context.mounted) return;
final zulipLocalizations = ZulipLocalizations.of(context);
await showErrorDialog(context: context,
showErrorDialog(context: context,
title: zulipLocalizations.errorMarkAsReadFailedTitle,
message: e.toString()); // TODO(#741): extract user-facing message better
return;
} finally {
setState(() => _loading = false);
}
if (!context.mounted) return;
if (narrow is CombinedFeedNarrow && !useLegacy) {
if (widget.narrow is CombinedFeedNarrow && !useLegacy) {
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
}
}
Expand All @@ -490,13 +500,13 @@ class MarkAsReadWidget extends StatelessWidget {
Widget build(BuildContext context) {
final zulipLocalizations = ZulipLocalizations.of(context);
final store = PerAccountStoreWidget.of(context);
final unreadCount = store.unreads.countInNarrow(narrow);
final unreadCount = store.unreads.countInNarrow(widget.narrow);
final areMessagesRead = unreadCount == 0;

return IgnorePointer(
ignoring: areMessagesRead,
child: AnimatedOpacity(
opacity: areMessagesRead ? 0 : 1,
opacity: areMessagesRead ? 0 : _loading ? 0.5 : 1,
duration: Duration(milliseconds: areMessagesRead ? 2000 : 300),
curve: Curves.easeOut,
child: SizedBox(width: double.infinity,
Expand All @@ -507,8 +517,6 @@ class MarkAsReadWidget extends StatelessWidget {
padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 10 - ((48 - 38) / 2)),
child: FilledButton.icon(
style: FilledButton.styleFrom(
// TODO(#95) need dark-theme colors (foreground and background)
backgroundColor: _UnreadMarker.color,
minimumSize: const Size.fromHeight(38),
textStyle:
// Restate [FilledButton]'s default, which inherits from
Expand All @@ -522,11 +530,17 @@ class MarkAsReadWidget extends StatelessWidget {
height: (23 / 18))
.merge(weightVariableTextStyle(context, wght: 400))),
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
).copyWith(
// Give the buttons a constant color regardless of whether their
// state is disabled, pressed, etc. We handle those states
// separately, via MarkAsReadAnimation.
// TODO(#95) need dark-theme colors (foreground and background)
foregroundColor: WidgetStateColor.resolveWith((_) => Colors.white),
backgroundColor: WidgetStateColor.resolveWith((_) => _UnreadMarker.color),
),
onPressed: () => _handlePress(context),
onPressed: _loading ? null : () => _handlePress(context),
icon: const Icon(Icons.playlist_add_check),
label: Text(zulipLocalizations.markAllAsReadLabel))))),
);
label: Text(zulipLocalizations.markAllAsReadLabel))))));
}
}

Expand Down
59 changes: 59 additions & 0 deletions test/widgets/message_list_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,65 @@ void main() {
unreadMessageIds: [message.id]),
]);

group('MarkAsReadAnimation', () {
void checkAppearsLoading(WidgetTester tester, bool expected) {
final semantics = tester.firstWidget<Semantics>(find.descendant(
of: find.byType(MarkAsReadWidget),
matching: find.byType(Semantics)));
check(semantics.properties.enabled).equals(!expected);

final opacity = tester.widget<AnimatedOpacity>(find.descendant(
of: find.byType(MarkAsReadWidget),
matching: find.byType(AnimatedOpacity)));
check(opacity.opacity).equals(expected ? 0.5 : 1.0);
}

testWidgets('loading is changed correctly', (WidgetTester tester) async {
final narrow = TopicNarrow.ofMessage(message);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();

connection.prepare(
delay: const Duration(milliseconds: 2000),
json: UpdateMessageFlagsForNarrowResult(
processedCount: 11, updatedCount: 3,
firstProcessedId: null, lastProcessedId: null,
foundOldest: true, foundNewest: true).toJson());

checkAppearsLoading(tester, false);

await tester.tap(find.byType(MarkAsReadWidget));
await tester.pump();
checkAppearsLoading(tester, true);

await tester.pump(const Duration(milliseconds: 2000));
checkAppearsLoading(tester, false);
});

testWidgets('loading is changed correctly if request fails', (WidgetTester tester) async {
final narrow = TopicNarrow.ofMessage(message);
await setupMessageListPage(tester,
narrow: narrow, messages: [message], unreadMsgs: unreadMsgs);
check(isMarkAsReadButtonVisible(tester)).isTrue();

connection.prepare(httpStatus: 400, json: {
'code': 'BAD_REQUEST',
'msg': 'Invalid message(s)',
'result': 'error',
});

checkAppearsLoading(tester, false);

await tester.tap(find.byType(MarkAsReadWidget));
await tester.pump();
checkAppearsLoading(tester, true);

await tester.pump(const Duration(milliseconds: 2000));
checkAppearsLoading(tester, false);
});
});

testWidgets('smoke test on modern server', (WidgetTester tester) async {
final narrow = TopicNarrow.ofMessage(message);
await setupMessageListPage(tester,
Expand Down

0 comments on commit 0d577a0

Please sign in to comment.