From a9a4e97330e269900003a1ded2dab9ae94454903 Mon Sep 17 00:00:00 2001 From: blazern Date: Thu, 21 Apr 2022 13:37:28 +0300 Subject: [PATCH] Implement pull to refresh for News Feed https://github.com/plante-app-team/plante/issues/228 --- .../animated_list_simple_plante.dart | 13 +++- lib/ui/map/map_page/map_page.dart | 3 +- lib/ui/news/news_feed_page.dart | 44 ++++++++----- lib/ui/news/news_feed_page_model.dart | 42 +++++++++--- test/ui/news/news_feed_page_test.dart | 64 +++++++++++++++++-- test/z_fakes/fake_news_feed_manager.dart | 8 ++- 6 files changed, 139 insertions(+), 35 deletions(-) diff --git a/lib/ui/base/components/animated_list_simple_plante.dart b/lib/ui/base/components/animated_list_simple_plante.dart index 6d481771..5ded1ffe 100644 --- a/lib/ui/base/components/animated_list_simple_plante.dart +++ b/lib/ui/base/components/animated_list_simple_plante.dart @@ -6,8 +6,14 @@ import 'package:flutter/material.dart'; class AnimatedListSimplePlante extends StatefulWidget { final List children; final EdgeInsets padding; + final bool shrinkWrap; + final ScrollPhysics? physics; const AnimatedListSimplePlante( - {Key? key, required this.children, this.padding = EdgeInsets.zero}) + {Key? key, + required this.children, + this.physics, + this.shrinkWrap = false, + this.padding = EdgeInsets.zero}) : super(key: key); @override @@ -36,7 +42,7 @@ class _AnimatedListSimplePlanteState extends State { } }, remove: (pos, count) { - for (var index = pos; index < pos + count; ++index) { + for (var index = pos + count - 1; pos <= index; --index) { listState.removeItem( index, (context, animation) => @@ -72,7 +78,8 @@ class _AnimatedListSimplePlanteState extends State { behavior: _ScrollBehavior(), child: AnimatedList( key: _listKey, - shrinkWrap: true, + physics: widget.physics, + shrinkWrap: widget.shrinkWrap, initialItemCount: widget.children.length, itemBuilder: (context, index, animation) { return _wrapChild(widget.children[index], animation); diff --git a/lib/ui/map/map_page/map_page.dart b/lib/ui/map/map_page/map_page.dart index efb71e85..77ceb497 100644 --- a/lib/ui/map/map_page/map_page.dart +++ b/lib/ui/map/map_page/map_page.dart @@ -436,6 +436,7 @@ class _MapPageState extends PageStatePlante MapBottomHint(_bottomHint.watch(ref))), Consumer( builder: (context, ref, _) => AnimatedListSimplePlante( + shrinkWrap: true, children: _mode.watch(ref).buildBottomActions())), ])), Align( @@ -508,7 +509,7 @@ class _MapPageState extends PageStatePlante padding: const EdgeInsets.only(right: 24, bottom: 24), child: e)) .toList(); - return AnimatedListSimplePlante(children: fabs); + return AnimatedListSimplePlante(shrinkWrap: true, children: fabs); }); } diff --git a/lib/ui/news/news_feed_page.dart b/lib/ui/news/news_feed_page.dart index 558b9468..0f001f56 100644 --- a/lib/ui/news/news_feed_page.dart +++ b/lib/ui/news/news_feed_page.dart @@ -14,6 +14,7 @@ import 'package:plante/outside/map/ui_list_addresses_obtainer.dart'; import 'package:plante/products/products_obtainer.dart'; import 'package:plante/ui/base/colors_plante.dart'; import 'package:plante/ui/base/components/address_widget.dart'; +import 'package:plante/ui/base/components/animated_list_simple_plante.dart'; import 'package:plante/ui/base/components/button_filled_plante.dart'; import 'package:plante/ui/base/components/circular_progress_indicator_plante.dart'; import 'package:plante/ui/base/components/licence_label.dart'; @@ -21,6 +22,7 @@ import 'package:plante/ui/base/components/visibility_detector_plante.dart'; import 'package:plante/ui/base/page_state_plante.dart'; import 'package:plante/ui/base/text_styles.dart'; import 'package:plante/ui/base/ui_utils.dart'; +import 'package:plante/ui/base/ui_value.dart'; import 'package:plante/ui/news/news_feed_page_model.dart'; import 'package:plante/ui/product/product_header_widget.dart'; import 'package:plante/ui/product/product_page_wrapper.dart'; @@ -44,6 +46,7 @@ class _NewsFeedPageState extends PageStatePlante { UiListAddressesObtainer(GetIt.I.get()); final _visibleShops = {}; + late final _loadingByPullToRefresh = UIValue(false, ref); _NewsFeedPageState() : super('PageStatePlante'); @@ -55,7 +58,7 @@ class _NewsFeedPageState extends PageStatePlante { void _initAsync() async { await nextFrame(); - _model.maybeLoadNextNews(); + await _model.maybeLoadNextNews(); } @override @@ -67,20 +70,28 @@ class _NewsFeedPageState extends PageStatePlante { consumer((ref) { final newsWidgets = _newsPiecesWidgets(_model.newsPieces.watch(ref)); - return ListView( - key: const Key('news_pieces_list'), - children: [ - Padding( - padding: - const EdgeInsets.symmetric(vertical: 16, horizontal: 21), - child: Text(context.strings.news_feed_page_new_events_title, - style: TextStyles.newsTitle), - ), - ...newsWidgets, - if (_model.newsPieces.watch(ref).isNotEmpty) - _loadingOrErrorOrNothing(), - ], - ); + return RefreshIndicator( + onRefresh: () async { + _loadingByPullToRefresh.setValue(true); + await _model.reloadNews(); + _loadingByPullToRefresh.setValue(false); + }, + child: AnimatedListSimplePlante( + key: const Key('news_pieces_list'), + physics: const AlwaysScrollableScrollPhysics(), + children: [ + Padding( + padding: const EdgeInsets.symmetric( + vertical: 16, horizontal: 21), + child: Text( + context.strings.news_feed_page_new_events_title, + style: TextStyles.newsTitle), + ), + ...newsWidgets, + if (_model.newsPieces.watch(ref).isNotEmpty) + _loadingOrErrorOrNothing(), + ], + )); }), if (_model.newsPieces.watch(ref).isEmpty) _loadingOrErrorOrNothing(), ]))); @@ -89,6 +100,9 @@ class _NewsFeedPageState extends PageStatePlante { Widget _loadingOrErrorOrNothing() { return consumer((ref) { if (_model.loading.watch(ref)) { + if (_loadingByPullToRefresh.watch(ref)) { + return const SizedBox(); + } return const _LoadingWidget(); } final error = _model.lastError.watch(ref); diff --git a/lib/ui/news/news_feed_page_model.dart b/lib/ui/news/news_feed_page_model.dart index 2e791682..5f00d747 100644 --- a/lib/ui/news/news_feed_page_model.dart +++ b/lib/ui/news/news_feed_page_model.dart @@ -40,16 +40,42 @@ class NewsFeedPageModel { Shop? getShopWith(OsmUID uid) => _loadedShops[uid]; Iterable getAllLoadedShops() => _loadedShops.values; - void maybeLoadNextNews() async { - if (_allNewsLoaded) { + Future reloadNews() async { + await _maybeLoadNextNews(clearOldNews: true); + } + + Future maybeLoadNextNews() async { + await _maybeLoadNextNews(); + } + + Future _maybeLoadNextNews({bool clearOldNews = false}) async { + if (loading.cachedVal) { return; } + if (_allNewsLoaded && !clearOldNews) { + return; + } + _loading.setValue(true); _lastError.setValue(null); try { + if (clearOldNews) { + _allNewsLoaded = false; + _lastLoadedNewsPage = -1; + } final result = await _loadNewsImpl(); if (result.isOk) { + if (clearOldNews) { + _newsPieces.setValue(const []); + } _lastLoadedNewsPage += 1; + final newNews = result.unwrap(); + if (newNews.isEmpty) { + _allNewsLoaded = true; + } + final allNews = _newsPieces.cachedVal.toList(); + allNews.addAll(newNews); + _newsPieces.setValue(allNews); } else { _lastError.setValue(result.unwrapErr()); } @@ -58,7 +84,8 @@ class NewsFeedPageModel { } } - Future> _loadNewsImpl() async { + // Returns loaded news + Future, GeneralError>> _loadNewsImpl() async { // Get news final newsRes = await _newsFeedManager.obtainNews(page: _lastLoadedNewsPage + 1); @@ -67,8 +94,7 @@ class NewsFeedPageModel { } final newNews = newsRes.unwrap(); if (newNews.isEmpty) { - _allNewsLoaded = true; - return Ok(None()); + return Ok(const []); } // Extract barcodes and shops UIDs @@ -99,10 +125,6 @@ class NewsFeedPageModel { } _loadedShops.addAll(shopsRes.unwrap()); - // Finish - final allNews = _newsPieces.cachedVal.toList(); - allNews.addAll(newNews); - _newsPieces.setValue(allNews); - return Ok(None()); + return Ok(newNews); } } diff --git a/test/ui/news/news_feed_page_test.dart b/test/ui/news/news_feed_page_test.dart index 60c2e5ef..89a560ee 100644 --- a/test/ui/news/news_feed_page_test.dart +++ b/test/ui/news/news_feed_page_test.dart @@ -91,7 +91,7 @@ void main() { GetIt.I.registerSingleton(userParamsController); }); - Future scrollDown(WidgetTester tester) async { + Future scroll(WidgetTester tester, double yDiff) async { // NOTE: we pause products retrieval before the scroll down // and resume it after. // This is needed because we expected new batches of products to be @@ -101,13 +101,21 @@ void main() { productsObtainer.pauseProductsRetrieval(); for (var i = 0; i < 50; ++i) { await tester.drag( - find.byKey(const Key('news_pieces_list')), const Offset(0, -3000)); + find.byKey(const Key('news_pieces_list')), Offset(0, yDiff)); await tester.pump(); } productsObtainer.resumeProductsRetrieval(); await tester.pumpAndSettle(); } + Future scrollDown(WidgetTester tester) async { + await scroll(tester, -3000); + } + + Future scrollUp(WidgetTester tester) async { + await scroll(tester, 3000); + } + Product createProduct(String name) { lastBarcode += 1; final product = ProductLangSlice((e) => e @@ -276,9 +284,11 @@ void main() { // And an error is also present expect( find.text(context.strings.global_something_went_wrong), findsOneWidget); - // And the first product of the second page is not present + // And the first+last products of the second page are not present expect(find.text('Product ${newsFeedManager.pageSizeTesting + 1}'), findsNothing); + expect(find.text('Product ${newsFeedManager.pageSizeTesting * 2}'), + findsNothing); // Remove the error, retry newsFeedManager.setErrorForPage_testing(1, null); @@ -287,10 +297,11 @@ void main() { // No error expect( find.text(context.strings.global_something_went_wrong), findsNothing); - // Both the last and the first product from pages 0 and 1 are present + // Both last products from pages 0 and 1 are present expect(find.text('Product ${newsFeedManager.pageSizeTesting}'), findsOneWidget); - expect(find.text('Product ${newsFeedManager.pageSizeTesting + 1}'), + await scrollDown(tester); + expect(find.text('Product ${newsFeedManager.pageSizeTesting * 2}'), findsOneWidget); }); @@ -350,4 +361,47 @@ void main() { expect( find.text(context.strings.global_something_went_wrong), findsNothing); }); + + testWidgets('pull to refresh behaviour', (WidgetTester tester) async { + // Prepare 2 pages of products' news + for (var index = 1; index <= newsFeedManager.pageSizeTesting * 2; ++index) { + addProductToShop(createProduct('Product $index'), + createShop('Shop $index', OsmAddress((e) => e.road = 'Lenina'))); + } + + await tester.superPump(const NewsFeedPage()); + + // Both first of page 0 and last product of page 1 are present + expect(find.text('Product 1'), findsOneWidget); + await scrollDown(tester); + await scrollDown(tester); + expect(find.text('Product ${newsFeedManager.pageSizeTesting * 2}'), + findsOneWidget); + + // Prepare absolutely new news + newsFeedManager.deleteAllNews_testing(); + addProductToShop(createProduct('New product 1'), + createShop('New shop 1', OsmAddress((e) => e.road = 'Lenina'))); + addProductToShop(createProduct('New product 2'), + createShop('New shop 2', OsmAddress((e) => e.road = 'Lenina'))); + + // Pull to refresh + await scrollUp(tester); + await scrollUp(tester); + + // New news are visible + expect(find.text('New product 1'), findsOneWidget); + expect(find.text('New product 2'), findsOneWidget); + + // Old news are not present, even from the second page + expect(find.text('Product 1'), findsNothing); + expect(find.text('Product ${newsFeedManager.pageSizeTesting * 2}'), + findsNothing); + + // They were not present on the top, they are not present on the bottom + await scrollDown(tester); + expect(find.text('Product 1'), findsNothing); + expect(find.text('Product ${newsFeedManager.pageSizeTesting * 2}'), + findsNothing); + }); } diff --git a/test/z_fakes/fake_news_feed_manager.dart b/test/z_fakes/fake_news_feed_manager.dart index 07c2676b..f4f9e2d6 100644 --- a/test/z_fakes/fake_news_feed_manager.dart +++ b/test/z_fakes/fake_news_feed_manager.dart @@ -6,9 +6,10 @@ import 'package:plante/outside/backend/news/news_piece.dart'; class FakeNewsFeedManager implements NewsFeedManager { final _news = []; final _errorsForPages = {}; + final _obtainedPages = []; final int pageSizeTesting; - FakeNewsFeedManager({this.pageSizeTesting = 5}); + FakeNewsFeedManager({this.pageSizeTesting = 10}); // ignore: non_constant_identifier_names void setErrorForPage_testing(int page, GeneralError? error) { @@ -29,9 +30,14 @@ class FakeNewsFeedManager implements NewsFeedManager { _news.clear(); } + // ignore: non_constant_identifier_names + List obtainedPages_testing() => List.unmodifiable(_obtainedPages); + @override Future, GeneralError>> obtainNews( {required int page}) async { + _obtainedPages.add(page); + final error = _errorsForPages[page]; if (error != null) { return Err(error);