Skip to content

msglist: add scroll-to-bottom button to MessageList #223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 75 additions & 3 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:math';

import 'package:flutter/material.dart';
import 'package:intl/intl.dart';

Expand Down Expand Up @@ -58,7 +60,6 @@ class _MessageListPageState extends State<MessageListPage> {

child: Expanded(
child: MessageList(narrow: widget.narrow))),

ComposeBox(controllerKey: _composeBoxKey, narrow: widget.narrow),
]))));
}
Expand Down Expand Up @@ -97,7 +98,6 @@ class MessageListAppBarTitle extends StatelessWidget {
}
}


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

Expand All @@ -109,6 +109,14 @@ class MessageList extends StatefulWidget {

class _MessageListState extends State<MessageList> {
MessageListView? model;
final ScrollController scrollController = ScrollController();
final ValueNotifier<bool> _scrollToBottomVisibleValue = ValueNotifier<bool>(false);

@override
void initState() {
super.initState();
scrollController.addListener(_scrollChanged);
}

@override
void didChangeDependencies() {
Expand All @@ -126,6 +134,8 @@ class _MessageListState extends State<MessageList> {
@override
void dispose() {
model?.dispose();
scrollController.dispose();
_scrollToBottomVisibleValue.dispose();
super.dispose();
}

Expand All @@ -142,6 +152,23 @@ class _MessageListState extends State<MessageList> {
});
}

void _adjustButtonVisibility(ScrollMetrics scrollMetrics) {
if (scrollMetrics.extentBefore == 0) {
_scrollToBottomVisibleValue.value = false;
} else {
_scrollToBottomVisibleValue.value = true;
}
}

void _scrollChanged() {
_adjustButtonVisibility(scrollController.position);
}

bool _metricsChanged(ScrollMetricsNotification scrollMetricsNotification) {
_adjustButtonVisibility(scrollMetricsNotification.metrics);
return true;
}

@override
Widget build(BuildContext context) {
assert(model != null);
Expand All @@ -161,7 +188,18 @@ class _MessageListState extends State<MessageList> {
child: Center(
child: ConstrainedBox(
constraints: const BoxConstraints(maxWidth: 760),
child: _buildListView(context))))));
child: NotificationListener<ScrollMetricsNotification>(
onNotification: _metricsChanged,
child: Stack(
children: <Widget>[
_buildListView(context),
Positioned(
bottom: 0,
right: 0,
child: ScrollToBottomButton(
scrollController: scrollController,
visibleValue: _scrollToBottomVisibleValue)),
])))))));
}

Widget _buildListView(context) {
Expand All @@ -179,6 +217,7 @@ class _MessageListState extends State<MessageList> {
_ => ScrollViewKeyboardDismissBehavior.manual,
},

controller: scrollController,
itemCount: length,
// Setting reverse: true means the scroll starts at the bottom.
// Flipping the indexes (in itemBuilder) means the start/bottom
Expand All @@ -194,6 +233,39 @@ class _MessageListState extends State<MessageList> {
}
}

class ScrollToBottomButton extends StatelessWidget {
const ScrollToBottomButton({super.key, required this.scrollController, required this.visibleValue});

final ValueNotifier<bool> visibleValue;
final ScrollController scrollController;

Future<void> _navigateToBottom() async {
final distance = scrollController.position.pixels;
final durationMsAtSpeedLimit = (1000 * distance / 8000).ceil();
final durationMs = max(300, durationMsAtSpeedLimit);
scrollController.animateTo(
0,
duration: Duration(milliseconds: durationMs),
curve: Curves.ease);
}

@override
Widget build(BuildContext context) {
return ValueListenableBuilder<bool>(
valueListenable: visibleValue,
builder: (BuildContext context, bool value, Widget? child) {
return (value && child != null) ? child : const SizedBox.shrink();
},
// TODO: fix hardcoded values for size and style here
child: IconButton(
tooltip: "Scroll to bottom",
icon: const Icon(Icons.expand_circle_down_rounded),
iconSize: 40,
color: const HSLColor.fromAHSL(0.5,240,0.96,0.68).toColor(),
onPressed: _navigateToBottom));
}
}

class MessageItem extends StatelessWidget {
const MessageItem({
super.key,
Expand Down
125 changes: 125 additions & 0 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import 'package:checks/checks.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:zulip/api/model/model.dart';
import 'package:zulip/api/route/messages.dart';
import 'package:zulip/model/narrow.dart';
import 'package:zulip/widgets/message_list.dart';
import 'package:zulip/widgets/sticky_header.dart';
import 'package:zulip/widgets/store.dart';

import '../api/fake_api.dart';
import '../example_data.dart' as eg;
import '../model/binding.dart';

Future<void> setupMessageListPage(WidgetTester tester, {
required Narrow narrow,
}) async {
addTearDown(TestZulipBinding.instance.reset);
addTearDown(tester.view.resetPhysicalSize);

tester.view.physicalSize = const Size(600, 800);

await TestZulipBinding.instance.globalStore.add(eg.selfAccount, eg.initialSnapshot());
final store = await TestZulipBinding.instance.globalStore.perAccount(eg.selfAccount.id);
final connection = store.connection as FakeApiConnection;

// prepare message list data
final List<StreamMessage> messages = List.generate(10, (index) {
return eg.streamMessage(id: index);
});
connection.prepare(json: GetMessagesResult(
anchor: messages[0].id,
foundNewest: true,
foundOldest: true,
foundAnchor: true,
historyLimited: false,
messages: messages,
).toJson());

await tester.pumpWidget(
MaterialApp(
home: GlobalStoreWidget(
child: PerAccountStoreWidget(
accountId: eg.selfAccount.id,
child: MessageListPage(narrow: narrow)))));

// global store, per-account store, and message list get loaded
await tester.pumpAndSettle();
}

void main() {
TestZulipBinding.ensureInitialized();

group('ScrollToBottomButton interactions', () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have one more test case — testing that the button actually works :-)

ScrollController? findMessageListScrollController(WidgetTester tester) {
final stickyHeaderListView = tester.widget<StickyHeaderListView>(find.byType(StickyHeaderListView));
return stickyHeaderListView.controller;
}

bool isButtonVisible(WidgetTester tester) {
return tester.any(find.descendant(
of: find.byType(ScrollToBottomButton),
matching: find.byTooltip("Scroll to bottom")));
}

testWidgets('scrolling changes visibility', (WidgetTester tester) async {
final stream = eg.stream();
await setupMessageListPage(tester, narrow: StreamNarrow(stream.streamId));

final scrollController = findMessageListScrollController(tester)!;

// Initial state should be not visible, as the message list renders with latest message in view
check(isButtonVisible(tester)).equals(false);

scrollController.jumpTo(600);
await tester.pump();
check(isButtonVisible(tester)).equals(true);

scrollController.jumpTo(0);
await tester.pump();
check(isButtonVisible(tester)).equals(false);
});

testWidgets('dimension updates changes visibility', (WidgetTester tester) async {
final stream = eg.stream();
await setupMessageListPage(tester, narrow: StreamNarrow(stream.streamId));

final scrollController = findMessageListScrollController(tester)!;

// Initial state should be not visible, as the message list renders with latest message in view
check(isButtonVisible(tester)).equals(false);

scrollController.jumpTo(600);
await tester.pump();
check(isButtonVisible(tester)).equals(true);

tester.view.physicalSize = const Size(2000, 40000);
await tester.pump();
// Dimension changes use NotificationListener<ScrollMetricsNotification
// which has a one frame lag. If that ever gets resolved this extra pump
// would ideally be removed
await tester.pump();
check(isButtonVisible(tester)).equals(false);
});

testWidgets('button functionality', (WidgetTester tester) async {
final stream = eg.stream();
await setupMessageListPage(tester, narrow: StreamNarrow(stream.streamId));

final scrollController = findMessageListScrollController(tester)!;

// Initial state should be not visible, as the message list renders with latest message in view
check(isButtonVisible(tester)).equals(false);

scrollController.jumpTo(600);
await tester.pump();
check(isButtonVisible(tester)).equals(true);

await tester.tap(find.byType(ScrollToBottomButton));
await tester.pumpAndSettle();
check(isButtonVisible(tester)).equals(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check instead that the scroll controller's actual position is at the bottom — that way this test wouldn't accidentally pass if we introduced some bug where the button didn't actually work, but did cause the button to go away 🙂

check(scrollController.position.pixels).equals(0);
});
});
}