Skip to content
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

feat: #2174 - doomscrolling instead of "download more" button #2770

Merged
merged 2 commits into from
Aug 11, 2022
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
12 changes: 6 additions & 6 deletions packages/smooth_app/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,13 @@ PODS:
- Realm/Headers (10.28.2)
- RealmSwift (10.28.2):
- Realm (= 10.28.2)
- Sentry (7.19.0):
- Sentry/Core (= 7.19.0)
- Sentry/Core (7.19.0)
- Sentry (7.22.0):
- Sentry/Core (= 7.22.0)
- Sentry/Core (7.22.0)
- sentry_flutter (0.0.1):
- Flutter
- FlutterMacOS
- Sentry (~> 7.19.0)
- Sentry (~> 7.22.0)
- share_plus (0.0.1):
- Flutter
- shared_preferences_ios (0.0.1):
Expand Down Expand Up @@ -255,8 +255,8 @@ SPEC CHECKSUMS:
Protobuf: 818c6a87e44193a77f56b87c6a1c106efda7e062
Realm: e617c54ad0f566b3d0f6a2abae7010de05f252ac
RealmSwift: 8a516a8759d80d52ebcec1ff1076f5f5159f98b2
Sentry: d6b16e66a0ad0c39a0b76d9a1194e68e78c37ce6
sentry_flutter: 609b096af497fa64af7c6c94e792d0780aa472a6
Sentry: 30b51086ca9aac23337880152e95538f7e177f7f
sentry_flutter: eaca55af5c951f4be6c4b1daddce1744c37d8476
share_plus: 056a1e8ac890df3e33cb503afffaf1e9b4fbae68
shared_preferences_ios: 548a61f8053b9b8a49ac19c1ffbc8b92c50d68ad
sqflite: 6d358c025f5b867b29ed92fc697fd34924e11904
Expand Down
123 changes: 61 additions & 62 deletions packages/smooth_app/lib/pages/product/common/product_query_page.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import 'dart:async';
import 'dart:math';

import 'package:auto_size_text/auto_size_text.dart';
import 'package:flutter/material.dart';
Expand All @@ -8,6 +7,7 @@ import 'package:iso_countries/iso_countries.dart';
import 'package:matomo_tracker/matomo_tracker.dart';
import 'package:openfoodfacts/utils/CountryHelper.dart';
import 'package:provider/provider.dart';
import 'package:smooth_app/cards/product_cards/smooth_product_card_template.dart';
import 'package:smooth_app/data_models/product_list_supplier.dart';
import 'package:smooth_app/data_models/product_query_model.dart';
import 'package:smooth_app/database/local_database.dart';
Expand Down Expand Up @@ -87,39 +87,46 @@ class _ProductQueryPageState extends State<ProductQueryPage>
final Size screenSize = MediaQuery.of(context).size;
final ThemeData themeData = Theme.of(context);
switch (_model.loadingStatus) {
case LoadingStatus.ERROR:
return _getErrorWidget(
screenSize, themeData, '${_model.loadingError}');
case LoadingStatus.LOADING:
return _getEmptyScreen(
screenSize,
themeData,
const CircularProgressIndicator(),
);
case LoadingStatus.LOADED:
if (_model.isNotEmpty()) {
// TODO(monsieurtanuki): add country, language?
AnalyticsHelper.trackSearch(
search: widget.name,
searchCount: _model.displayBarcodes.length,
);
return _getNotEmptyScreen(
if (!_model.isNotEmpty()) {
return _getEmptyScreen(
screenSize,
themeData,
appLocalizations,
const CircularProgressIndicator(),
);
}
// TODO(monsieurtanuki): should be tracked as well, shouldn't it?
return _getEmptyScreen(
screenSize,
themeData,
_getEmptyText(
break;
case LoadingStatus.LOADED:
if (!_model.isNotEmpty()) {
// TODO(monsieurtanuki): should be tracked as well, shouldn't it?
return _getEmptyScreen(
screenSize,
themeData,
appLocalizations.no_product_found,
),
actions: _getAppBarButtons(),
_getEmptyText(
themeData,
appLocalizations.no_product_found,
),
actions: _getAppBarButtons(),
);
}
// TODO(monsieurtanuki): add country, language?
AnalyticsHelper.trackSearch(
search: widget.name,
searchCount: _model.displayBarcodes.length,
);
case LoadingStatus.ERROR:
return _getErrorWidget(
screenSize, themeData, '${_model.loadingError}');
break;
}
// Now used in two cases.
// 1. we have data downloaded and we display it (normal mode)
// 2. we are downloading extra data, and display what we already knew
return _getNotEmptyScreen(
screenSize,
themeData,
appLocalizations,
);
},
);
}
Expand Down Expand Up @@ -212,41 +219,13 @@ class _ProductQueryPageState extends State<ProductQueryPage>
return _getTopMessagesCard();
}
index--;
// TODO(monsieurtanuki): maybe call it earlier, like for first unknown page index - 5?
if (index >= _model.displayBarcodes.length) {
// final button
final int already = _model.displayBarcodes.length;
final int totalSize =
_model.supplier.partialProductList.totalSize;
final int next = max(
0,
min(
_model.supplier.productQuery.pageSize,
totalSize - already,
),
);
final Widget child;
if (next == 0) {
child = EMPTY_WIDGET;
} else {
child = SmoothLargeButtonWithIcon(
text: appLocalizations.product_search_button_download_more(
next,
already,
totalSize,
),
icon: Icons.download_rounded,
onPressed: () async =>
_loadAndRefreshDisplay(_model.loadNextPage()),
);
}
return Padding(
padding: const EdgeInsetsDirectional.only(
bottom: 90.0,
start: VERY_LARGE_SPACE,
end: VERY_LARGE_SPACE,
),
child: child,
);
_downloadNextPage();
}
if (index >= _model.displayBarcodes.length) {
// TODO(monsieurtanuki): maybe display something specific for data being downloaded (the next page) and unknown data (beyond next page)
return const SmoothProductCardTemplate();
}
Comment on lines +226 to 229
Copy link
Member

Choose a reason for hiding this comment

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

One thing, the template has no common loading animation it is just a plan thing, if it takes over a certain amount of time and the user can scroll as long as he want's it could create the illusion that something is broken, therefor I'd suggest to at max show only one template at a time.

Copy link
Member

@teolemon teolemon Aug 11, 2022

Choose a reason for hiding this comment

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

it's usually solved using a shimmering effect that suggests loading

Copy link
Member

Choose a reason for hiding this comment

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

Yeah but I'd still prefer a single one and as we know monsieurtanuki isn't a friend of fancy animations so thats something we can always add later

Copy link
Member

Choose a reason for hiding this comment

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

ok

return Padding(
padding: const EdgeInsets.symmetric(
Expand All @@ -258,8 +237,8 @@ class _ProductQueryPageState extends State<ProductQueryPage>
),
);
},
// 2 additional widgets, on top and on bottom
itemCount: _model.displayBarcodes.length + 2,
// 1 additional widget, on top of ALL expected products
itemCount: _model.supplier.partialProductList.totalSize + 1,
),
),
);
Expand Down Expand Up @@ -469,6 +448,26 @@ class _ProductQueryPageState extends State<ProductQueryPage>
}
return success;
}

/// Flags if the next page is currently being downloaded.
bool _loadingNext = false;

/// Downloads the next page, asynchronously.
Future<void> _downloadNextPage() async {
if (_loadingNext) {
return;
}
_loadingNext = true;
try {
final bool result = await _model.loadNextPage();
if (result) {
setState(() {});
}
} catch (e) {
//
}
_loadingNext = false;
}
}

// TODO(monsieurtanki): put it in a specific Widget class
Expand Down