mirror of
https://github.com/rzuasti/oott.git
synced 2026-07-08 19:21:54 +02:00
Scroll to top of list when changing pages
The notifications and devices lists kept their scroll offset when paging, so a new page would open partway down. Attach a ScrollController to each CustomScrollView and animate back to the top whenever the page changes via the pagination bar. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
aa5a29271b
commit
aa5f65de2b
@@ -22,6 +22,7 @@
|
|||||||
|
|
||||||
## Frontend
|
## Frontend
|
||||||
|
|
||||||
|
- [ ] The notifications list should not refresh coldly every time. It should add/remove notifications with an animation as if a stack
|
||||||
- [x] Make gruvbox the default theme
|
- [x] Make gruvbox the default theme
|
||||||
- [x] Can we add front-end tests?
|
- [x] Can we add front-end tests?
|
||||||
- [x] Break down oott_api.dart in modules
|
- [x] Break down oott_api.dart in modules
|
||||||
|
|||||||
@@ -41,6 +41,7 @@ class _DeviceListState extends State<DeviceList> with RouteAware {
|
|||||||
int _currentPage = 0;
|
int _currentPage = 0;
|
||||||
bool _hasNextPage = false;
|
bool _hasNextPage = false;
|
||||||
CancelToken? _fetchToken;
|
CancelToken? _fetchToken;
|
||||||
|
final ScrollController _scrollController = ScrollController();
|
||||||
|
|
||||||
@override
|
@override
|
||||||
void initState() {
|
void initState() {
|
||||||
@@ -69,9 +70,23 @@ class _DeviceListState extends State<DeviceList> with RouteAware {
|
|||||||
_ownerDebounce?.cancel();
|
_ownerDebounce?.cancel();
|
||||||
_fetchToken?.cancel();
|
_fetchToken?.cancel();
|
||||||
_ownerController.dispose();
|
_ownerController.dispose();
|
||||||
|
_scrollController.dispose();
|
||||||
super.dispose();
|
super.dispose();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Fetches [page] and scrolls back to the top of the list, so changing pages
|
||||||
|
/// always starts the new page from its first row.
|
||||||
|
void _goToPage(int page) {
|
||||||
|
if (_scrollController.hasClients) {
|
||||||
|
_scrollController.animateTo(
|
||||||
|
0,
|
||||||
|
duration: const Duration(milliseconds: 300),
|
||||||
|
curve: Curves.easeOut,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
_fetchPage(page);
|
||||||
|
}
|
||||||
|
|
||||||
void _onOwnerChanged() {
|
void _onOwnerChanged() {
|
||||||
_ownerDebounce?.cancel();
|
_ownerDebounce?.cancel();
|
||||||
_ownerDebounce = Timer(
|
_ownerDebounce = Timer(
|
||||||
@@ -286,6 +301,7 @@ class _DeviceListState extends State<DeviceList> with RouteAware {
|
|||||||
return RefreshIndicator(
|
return RefreshIndicator(
|
||||||
onRefresh: () => _fetchPage(_currentPage),
|
onRefresh: () => _fetchPage(_currentPage),
|
||||||
child: CustomScrollView(
|
child: CustomScrollView(
|
||||||
|
controller: _scrollController,
|
||||||
physics: const AlwaysScrollableScrollPhysics(),
|
physics: const AlwaysScrollableScrollPhysics(),
|
||||||
slivers: [
|
slivers: [
|
||||||
if (isWide)
|
if (isWide)
|
||||||
@@ -323,7 +339,7 @@ class _DeviceListState extends State<DeviceList> with RouteAware {
|
|||||||
currentPage: _currentPage,
|
currentPage: _currentPage,
|
||||||
hasNextPage: _hasNextPage,
|
hasNextPage: _hasNextPage,
|
||||||
isLoading: _isLoading,
|
isLoading: _isLoading,
|
||||||
onPageChanged: _fetchPage,
|
onPageChanged: _goToPage,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
],
|
],
|
||||||
|
|||||||
@@ -52,6 +52,7 @@ class _NotificationsListState extends State<NotificationsList>
|
|||||||
bool _hasNextPage = false;
|
bool _hasNextPage = false;
|
||||||
String? _error;
|
String? _error;
|
||||||
CancelToken? _fetchToken;
|
CancelToken? _fetchToken;
|
||||||
|
final ScrollController _scrollController = ScrollController();
|
||||||
|
|
||||||
@override
|
@override
|
||||||
void initState() {
|
void initState() {
|
||||||
@@ -107,9 +108,23 @@ class _NotificationsListState extends State<NotificationsList>
|
|||||||
routeObserver.unsubscribe(this);
|
routeObserver.unsubscribe(this);
|
||||||
_notificationTimer?.cancel();
|
_notificationTimer?.cancel();
|
||||||
_fetchToken?.cancel();
|
_fetchToken?.cancel();
|
||||||
|
_scrollController.dispose();
|
||||||
super.dispose();
|
super.dispose();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Fetches [page] and scrolls back to the top of the list, so changing pages
|
||||||
|
/// always starts the new page from its first item.
|
||||||
|
void _goToPage(int page) {
|
||||||
|
if (_scrollController.hasClients) {
|
||||||
|
_scrollController.animateTo(
|
||||||
|
0,
|
||||||
|
duration: const Duration(milliseconds: 300),
|
||||||
|
curve: Curves.easeOut,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
_fetchPage(page);
|
||||||
|
}
|
||||||
|
|
||||||
Future<void> _fetchPage(int page) async {
|
Future<void> _fetchPage(int page) async {
|
||||||
_fetchToken?.cancel();
|
_fetchToken?.cancel();
|
||||||
final token = CancelToken();
|
final token = CancelToken();
|
||||||
@@ -207,6 +222,7 @@ class _NotificationsListState extends State<NotificationsList>
|
|||||||
_buildNotificationsHeader(context),
|
_buildNotificationsHeader(context),
|
||||||
Expanded(
|
Expanded(
|
||||||
child: CustomScrollView(
|
child: CustomScrollView(
|
||||||
|
controller: _scrollController,
|
||||||
slivers: [
|
slivers: [
|
||||||
..._buildNotificationSlivers(context),
|
..._buildNotificationSlivers(context),
|
||||||
...widget.trailingSlivers,
|
...widget.trailingSlivers,
|
||||||
@@ -296,7 +312,7 @@ class _NotificationsListState extends State<NotificationsList>
|
|||||||
currentPage: _currentPage,
|
currentPage: _currentPage,
|
||||||
hasNextPage: _hasNextPage,
|
hasNextPage: _hasNextPage,
|
||||||
isLoading: _isLoading,
|
isLoading: _isLoading,
|
||||||
onPageChanged: _fetchPage,
|
onPageChanged: _goToPage,
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
];
|
];
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
import 'package:flutter/material.dart';
|
||||||
import 'package:flutter_test/flutter_test.dart';
|
import 'package:flutter_test/flutter_test.dart';
|
||||||
import 'package:frontend/devices/device_list.dart';
|
import 'package:frontend/devices/device_list.dart';
|
||||||
import 'package:frontend/devices/device_list_rows.dart';
|
import 'package:frontend/devices/device_list_rows.dart';
|
||||||
@@ -31,8 +32,9 @@ void main() {
|
|||||||
await tearDownTree(tester);
|
await tearDownTree(tester);
|
||||||
});
|
});
|
||||||
|
|
||||||
testWidgets('shows the empty message when there are no devices',
|
testWidgets('shows the empty message when there are no devices', (
|
||||||
(tester) async {
|
tester,
|
||||||
|
) async {
|
||||||
adapter.onGet('/devices', (server) => server.reply(200, <dynamic>[]));
|
adapter.onGet('/devices', (server) => server.reply(200, <dynamic>[]));
|
||||||
|
|
||||||
await pumpScreen(tester, const DeviceList());
|
await pumpScreen(tester, const DeviceList());
|
||||||
@@ -57,4 +59,45 @@ void main() {
|
|||||||
|
|
||||||
await tearDownTree(tester);
|
await tearDownTree(tester);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
testWidgets('changing pages scrolls back to the top of the list', (
|
||||||
|
tester,
|
||||||
|
) async {
|
||||||
|
// 11 devices: a full page of 10 plus one, so the pagination bar appears.
|
||||||
|
adapter.onGet(
|
||||||
|
'/devices',
|
||||||
|
(server) => server.reply(
|
||||||
|
200,
|
||||||
|
List.generate(
|
||||||
|
11,
|
||||||
|
(i) => deviceJson(
|
||||||
|
macAddress: '00:00:00:00:00:${i.toString().padLeft(2, '0')}',
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
// A short viewport so the rows overflow the screen and the list can scroll.
|
||||||
|
await pumpScreen(tester, const DeviceList(), size: const Size(900, 500));
|
||||||
|
await pumpUntilFound(tester, find.byType(DeviceRowWide));
|
||||||
|
|
||||||
|
final scrollable = find.descendant(
|
||||||
|
of: find.byType(CustomScrollView),
|
||||||
|
matching: find.byType(Scrollable),
|
||||||
|
);
|
||||||
|
ScrollPosition position() =>
|
||||||
|
tester.state<ScrollableState>(scrollable).position;
|
||||||
|
|
||||||
|
// Scroll down to reveal the pagination bar.
|
||||||
|
await tester.drag(scrollable, const Offset(0, -2000));
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
expect(position().pixels, greaterThan(0));
|
||||||
|
|
||||||
|
// Advance a page: the list should jump back to the top.
|
||||||
|
await tester.tap(find.byTooltip('Next page'));
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
expect(position().pixels, 0);
|
||||||
|
|
||||||
|
await tearDownTree(tester);
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -54,4 +54,55 @@ void main() {
|
|||||||
await tester.pumpWidget(const SizedBox());
|
await tester.pumpWidget(const SizedBox());
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
|
testWidgets('changing pages scrolls back to the top of the list', (
|
||||||
|
tester,
|
||||||
|
) async {
|
||||||
|
tester.view.physicalSize = const Size(400, 300);
|
||||||
|
tester.view.devicePixelRatio = 1.0;
|
||||||
|
addTearDown(tester.view.resetPhysicalSize);
|
||||||
|
|
||||||
|
List<Map<String, dynamic>> page(int firstId) => List.generate(
|
||||||
|
6,
|
||||||
|
(i) => notificationJson(id: firstId + i, title: 'Item ${firstId + i}'),
|
||||||
|
);
|
||||||
|
adapter.onGet(
|
||||||
|
'/notifications',
|
||||||
|
(server) => server.reply(200, page(1)),
|
||||||
|
queryParameters: {'is_new': true, 'page_offset': 0, 'page_limit': 6},
|
||||||
|
);
|
||||||
|
adapter.onGet(
|
||||||
|
'/notifications',
|
||||||
|
(server) => server.reply(200, page(7)),
|
||||||
|
queryParameters: {'is_new': true, 'page_offset': 5, 'page_limit': 6},
|
||||||
|
);
|
||||||
|
|
||||||
|
await tester.pumpWidget(
|
||||||
|
MaterialApp(
|
||||||
|
theme: gruvboxDarkTheme,
|
||||||
|
home: const Scaffold(body: NotificationsList()),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
final scrollable = find.descendant(
|
||||||
|
of: find.byType(CustomScrollView),
|
||||||
|
matching: find.byType(Scrollable),
|
||||||
|
);
|
||||||
|
ScrollPosition position() =>
|
||||||
|
tester.state<ScrollableState>(scrollable).position;
|
||||||
|
|
||||||
|
// Scroll down to reveal the pagination bar.
|
||||||
|
await tester.drag(scrollable, const Offset(0, -2000));
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
expect(position().pixels, greaterThan(0));
|
||||||
|
|
||||||
|
// Advance a page: the list should jump back to the top.
|
||||||
|
await tester.tap(find.byTooltip('Next page'));
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
expect(find.textContaining('Item 7'), findsWidgets);
|
||||||
|
expect(position().pixels, 0);
|
||||||
|
|
||||||
|
await tester.pumpWidget(const SizedBox());
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user