Merge pull request #234 from blazern/pull-to-refresh-news

Implement pull to refresh for News Feed

lib/ui/base/components/animated_list_simple_plante.dart

@@ -6,8 +6,14 @@ import 'package:flutter/material.dart';
 class AnimatedListSimplePlante extends StatefulWidget {
   final List<Widget> 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<AnimatedListSimplePlante> {
           }
         },
         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<AnimatedListSimplePlante> {
             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);

lib/ui/map/map_page/map_page.dart

@@ -436,6 +436,7 @@ class _MapPageState extends PageStatePlante<MapPage>
                     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<MapPage>
               padding: const EdgeInsets.only(right: 24, bottom: 24),
               child: e))
           .toList();
-      return AnimatedListSimplePlante(children: fabs);
+      return AnimatedListSimplePlante(shrinkWrap: true, children: fabs);
     });
   }
 

lib/ui/news/news_feed_page.dart

@@ -14,13 +14,15 @@ 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';
 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<NewsFeedPage> {
       UiListAddressesObtainer<Shop>(GetIt.I.get<AddressObtainer>());
 
   final _visibleShops = <Shop>{};
+  late final _loadingByPullToRefresh = UIValue(false, ref);
 
   _NewsFeedPageState() : super('PageStatePlante');
 
@@ -55,7 +58,7 @@ class _NewsFeedPageState extends PageStatePlante<NewsFeedPage> {
 
   void _initAsync() async {
     await nextFrame();
-    _model.maybeLoadNextNews();
+    await _model.maybeLoadNextNews();
   }
 
   @override
@@ -67,20 +70,28 @@ class _NewsFeedPageState extends PageStatePlante<NewsFeedPage> {
           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<NewsFeedPage> {
   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);

lib/ui/news/news_feed_page_model.dart

@@ -40,16 +40,42 @@ class NewsFeedPageModel {
   Shop? getShopWith(OsmUID uid) => _loadedShops[uid];
   Iterable<Shop> getAllLoadedShops() => _loadedShops.values;
 
-  void maybeLoadNextNews() async {
-    if (_allNewsLoaded) {
+  Future<void> reloadNews() async {
+    await _maybeLoadNextNews(clearOldNews: true);
+  }
+
+  Future<void> maybeLoadNextNews() async {
+    await _maybeLoadNextNews();
+  }
+
+  Future<void> _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<Result<None, GeneralError>> _loadNewsImpl() async {
+  // Returns loaded news
+  Future<Result<List<NewsPiece>, 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);
   }
 }

test/ui/news/news_feed_page_test.dart

@@ -91,7 +91,7 @@ void main() {
     GetIt.I.registerSingleton<UserParamsController>(userParamsController);
   });
 
-  Future<void> scrollDown(WidgetTester tester) async {
+  Future<void> 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<void> scrollDown(WidgetTester tester) async {
+    await scroll(tester, -3000);
+  }
+
+  Future<void> 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);
+  });
 }

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 = <NewsPiece>[];
   final _errorsForPages = <int, GeneralError>{};
+  final _obtainedPages = <int>[];
   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<int> obtainedPages_testing() => List.unmodifiable(_obtainedPages);
+
   @override
   Future<Result<List<NewsPiece>, GeneralError>> obtainNews(
       {required int page}) async {
+    _obtainedPages.add(page);
+
     final error = _errorsForPages[page];
     if (error != null) {
       return Err(error);