Skip to content

Commit b2a3cbf

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 4932db1 commit b2a3cbf

File tree

2 files changed

+52
-35
lines changed

2 files changed

+52
-35
lines changed

lib/widgets/message_list.dart

+30-28
Original file line numberDiff line numberDiff line change
@@ -461,34 +461,36 @@ 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+
return IgnorePointer(
465+
ignoring: unreadCount == 0,
466+
child: AnimatedOpacity(
467+
opacity: (unreadCount > 0) ? 1 : 0,
468+
duration: const Duration(milliseconds: 300),
469+
child: SizedBox(width: double.infinity,
470+
// Design referenced from:
471+
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?type=design&node-id=132-9684&mode=design&t=jJwHzloKJ0TMOG4M-0
472+
child: Padding(
473+
// vertical padding adjusted for tap target height (48px) of button
474+
padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 10 - ((48 - 38) / 2)),
475+
child: FilledButton.icon(
476+
style: FilledButton.styleFrom(
477+
backgroundColor: _UnreadMarker.color,
478+
minimumSize: const Size.fromHeight(38),
479+
textStyle:
480+
// Restate [FilledButton]'s default, which inherits from
481+
// [zulipTypography]…
482+
Theme.of(context).textTheme.labelLarge!
483+
// …then clobber some attributes to follow Figma:
484+
.merge(const TextStyle(
485+
fontSize: 18,
486+
height: (23 / 18))
487+
.merge(weightVariableTextStyle(context, wght: 400))),
488+
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
489+
),
490+
onPressed: () => _handlePress(context),
491+
icon: const Icon(Icons.playlist_add_check),
492+
label: Text(zulipLocalizations.markAllAsReadLabel))))),
493+
);
492494
}
493495
}
494496

test/widgets/message_list_test.dart

+22-7
Original file line numberDiff line numberDiff line change
@@ -610,13 +610,28 @@ 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+
// Locate the [MarkAsReadWidget] and check its state
614+
// to determine if it is visible.
615+
var markAsReadWidget = find.byType(MarkAsReadWidget);
616+
617+
// The [MarkAsReadWidget] is a [IgnorePointer] wrapped in a [AnimatedOpacity].
618+
// We need to check the state of both to determine if the widget is visible.
619+
// The [IgnorePointer] is visible when `ignoring` is false and the [AnimatedOpacity]
620+
// is visible when `opacity` is 1.0.
621+
final ignorePointerFinder = find.descendant(
622+
of: markAsReadWidget,
623+
matching: find.byType(IgnorePointer),
624+
);
625+
626+
final animatedOpacityFinder = find.descendant(
627+
of: markAsReadWidget,
628+
matching: find.byType(AnimatedOpacity),
629+
);
630+
631+
final IgnorePointer ignorePointer = tester.widget(ignorePointerFinder);
632+
final AnimatedOpacity animatedOpacity = tester.widget(animatedOpacityFinder);
633+
634+
return !ignorePointer.ignoring && animatedOpacity.opacity == 1.0;
620635
}
621636

622637
testWidgets('from read to unread', (WidgetTester tester) async {

0 commit comments

Comments
 (0)