Skip to content

Commit c0964d7

Browse files
committed
msglist: Leave blank space for "mark as read" button so using it doesn't cause messages to shift
This commit addresses the issue of the message list shifting downwards when the "mark as read" button is pressed. The previous implementation would return an empty widget when the button was pressed and cause the message list to shift down. Now it takes a widget which will display the button based on the opacity level and will disable the region the button holds once the button is pressed, thus won't cause the message list to shift down. Fixes: #562
1 parent 9044a9a commit c0964d7

File tree

2 files changed

+62
-35
lines changed

2 files changed

+62
-35
lines changed

lib/widgets/message_list.dart

+33-28
Original file line numberDiff line numberDiff line change
@@ -461,34 +461,39 @@ class MarkAsReadWidget extends StatelessWidget {
461461
final zulipLocalizations = ZulipLocalizations.of(context);
462462
final store = PerAccountStoreWidget.of(context);
463463
final unreadCount = store.unreads.countInNarrow(narrow);
464-
return AnimatedCrossFade(
465-
duration: const Duration(milliseconds: 300),
466-
crossFadeState: (unreadCount > 0) ? CrossFadeState.showSecond : CrossFadeState.showFirst,
467-
firstChild: const SizedBox.shrink(),
468-
secondChild: SizedBox(width: double.infinity,
469-
// Design referenced from:
470-
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=132-9684&mode=design&t=jJwHzloKJ0TMOG4M-0
471-
child: Padding(
472-
// vertical padding adjusted for tap target height (48px) of button
473-
padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 10 - ((48 - 38) / 2)),
474-
child: FilledButton.icon(
475-
style: FilledButton.styleFrom(
476-
backgroundColor: _UnreadMarker.color,
477-
minimumSize: const Size.fromHeight(38),
478-
textStyle:
479-
// Restate [FilledButton]'s default, which inherits from
480-
// [zulipTypography]…
481-
Theme.of(context).textTheme.labelLarge!
482-
// …then clobber some attributes to follow Figma:
483-
.merge(const TextStyle(
484-
fontSize: 18,
485-
height: (23 / 18))
486-
.merge(weightVariableTextStyle(context, wght: 400))),
487-
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
488-
),
489-
onPressed: () => _handlePress(context),
490-
icon: const Icon(Icons.playlist_add_check),
491-
label: Text(zulipLocalizations.markAllAsReadLabel)))));
464+
final areMessagesRead = unreadCount == 0;
465+
466+
return IgnorePointer(
467+
ignoring: areMessagesRead,
468+
child: AnimatedOpacity(
469+
opacity: areMessagesRead ? 0 : 1,
470+
duration: Duration(milliseconds: areMessagesRead ? 2000 : 300),
471+
curve: Curves.easeOut,
472+
child: SizedBox(width: double.infinity,
473+
// Design referenced from:
474+
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=132-9684&mode=design&t=jJwHzloKJ0TMOG4M-0
475+
child: Padding(
476+
// vertical padding adjusted for tap target height (48px) of button
477+
padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 10 - ((48 - 38) / 2)),
478+
child: FilledButton.icon(
479+
style: FilledButton.styleFrom(
480+
backgroundColor: _UnreadMarker.color,
481+
minimumSize: const Size.fromHeight(38),
482+
textStyle:
483+
// Restate [FilledButton]'s default, which inherits from
484+
// [zulipTypography]…
485+
Theme.of(context).textTheme.labelLarge!
486+
// …then clobber some attributes to follow Figma:
487+
.merge(const TextStyle(
488+
fontSize: 18,
489+
height: (23 / 18))
490+
.merge(weightVariableTextStyle(context, wght: 400))),
491+
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
492+
),
493+
onPressed: () => _handlePress(context),
494+
icon: const Icon(Icons.playlist_add_check),
495+
label: Text(zulipLocalizations.markAllAsReadLabel))))),
496+
);
492497
}
493498
}
494499

test/widgets/message_list_test.dart

+29-7
Original file line numberDiff line numberDiff line change
@@ -610,13 +610,10 @@ void main() {
610610

611611
group('MarkAsReadWidget', () {
612612
bool isMarkAsReadButtonVisible(WidgetTester tester) {
613-
// Zero height elements on the edge of a scrolling viewport
614-
// are treated as invisible for hit-testing, see
615-
// [SliverMultiBoxAdaptorElement.debugVisitOnstageChildren].
616-
// Set `skipOffstage: false` here to safely target the
617-
// [MarkAsReadWidget] even when it is inactive.
618-
return tester.getSize(
619-
find.byType(MarkAsReadWidget, skipOffstage: false)).height > 0;
613+
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
614+
final finder = find.text(
615+
zulipLocalizations.markAllAsReadLabel).hitTestable();
616+
return finder.evaluate().isNotEmpty;
620617
}
621618

622619
testWidgets('from read to unread', (WidgetTester tester) async {
@@ -648,6 +645,31 @@ void main() {
648645
check(isMarkAsReadButtonVisible(tester)).isFalse();
649646
});
650647

648+
testWidgets("messages don't shift position", (WidgetTester tester) async {
649+
final message = eg.streamMessage(flags: []);
650+
final unreadMsgs = eg.unreadMsgs(streams:[
651+
UnreadStreamSnapshot(topic: message.subject, streamId: message.streamId,
652+
unreadMessageIds: [message.id])
653+
]);
654+
await setupMessageListPage(tester,
655+
messages: [message], unreadMsgs: unreadMsgs);
656+
check(isMarkAsReadButtonVisible(tester)).isTrue();
657+
check(tester.widgetList(find.byType(MessageItem))).length.equals(1);
658+
final before = tester.getTopLeft(find.byType(MessageItem)).dy;
659+
660+
store.handleEvent(UpdateMessageFlagsAddEvent(
661+
id: 1,
662+
flag: MessageFlag.read,
663+
messages: [message.id],
664+
all: false,
665+
));
666+
await tester.pumpAndSettle();
667+
check(isMarkAsReadButtonVisible(tester)).isFalse();
668+
check(tester.widgetList(find.byType(MessageItem))).length.equals(1);
669+
final after = tester.getTopLeft(find.byType(MessageItem)).dy;
670+
check(after).equals(before);
671+
});
672+
651673
group('onPressed behavior', () {
652674
final message = eg.streamMessage(flags: []);
653675
final unreadMsgs = eg.unreadMsgs(streams: [

0 commit comments

Comments
 (0)