From b11732690ad47a5b7b940d82df725191ac7e0957 Mon Sep 17 00:00:00 2001 From: Edouard Marquez Date: Sat, 17 Jun 2023 22:22:06 +0200 Subject: [PATCH 1/4] Fully working lifecycle for the Robotoff question --- .../lib/pages/product/new_product_page.dart | 21 +- .../product/product_questions_widget.dart | 212 ++++++++++++++---- 2 files changed, 182 insertions(+), 51 deletions(-) diff --git a/packages/smooth_app/lib/pages/product/new_product_page.dart b/packages/smooth_app/lib/pages/product/new_product_page.dart index 275fc135415..be5d3051265 100644 --- a/packages/smooth_app/lib/pages/product/new_product_page.dart +++ b/packages/smooth_app/lib/pages/product/new_product_page.dart @@ -26,6 +26,7 @@ import 'package:smooth_app/pages/inherited_data_manager.dart'; import 'package:smooth_app/pages/product/common/product_list_page.dart'; import 'package:smooth_app/pages/product/common/product_refresher.dart'; import 'package:smooth_app/pages/product/edit_product_page.dart'; +import 'package:smooth_app/pages/product/product_questions_widget.dart'; import 'package:smooth_app/pages/product/summary_card.dart'; import 'package:smooth_app/pages/product_list_user_dialog_helper.dart'; import 'package:smooth_app/query/product_query.dart'; @@ -57,6 +58,7 @@ class _ProductPageState extends State with TraceableClientMixin { late final Product _initialProduct; late final LocalDatabase _localDatabase; late ProductPreferences _productPreferences; + bool _keepRobotoffQuestionsAlive = true; bool scrollingUp = true; @@ -202,11 +204,14 @@ class _ProductPageState extends State with TraceableClientMixin { widget.heroTag?.isNotEmpty == true, child: Hero( tag: widget.heroTag ?? '', - child: SummaryCard( - _product, - _productPreferences, - isFullVersion: true, - showUnansweredQuestions: true, + child: KeepQuestionWidgetAlive( + keepWidgetAlive: _keepRobotoffQuestionsAlive, + child: SummaryCard( + _product, + _productPreferences, + isFullVersion: true, + showUnansweredQuestions: true, + ), ), ), ), @@ -343,10 +348,13 @@ class _ProductPageState extends State with TraceableClientMixin { Icons.edit, appLocalizations.edit_product_label, () async { + setState(() => _keepRobotoffQuestionsAlive = false); + AnalyticsHelper.trackEvent( AnalyticsEvent.openProductEditPage, barcode: _barcode, ); + await Navigator.push( context, MaterialPageRoute( @@ -354,6 +362,9 @@ class _ProductPageState extends State with TraceableClientMixin { EditProductPage(_product), ), ); + + // Force Robotoff questions to be reloaded + setState(() => _keepRobotoffQuestionsAlive = true); }, ), _buildActionBarItem( diff --git a/packages/smooth_app/lib/pages/product/product_questions_widget.dart b/packages/smooth_app/lib/pages/product/product_questions_widget.dart index 5cdf8b53356..b4b5aad15eb 100644 --- a/packages/smooth_app/lib/pages/product/product_questions_widget.dart +++ b/packages/smooth_app/lib/pages/product/product_questions_widget.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/app_localizations.dart'; import 'package:openfoodfacts/openfoodfacts.dart'; import 'package:provider/provider.dart'; +import 'package:shimmer/shimmer.dart'; import 'package:smooth_app/database/local_database.dart'; import 'package:smooth_app/generic_lib/design_constants.dart'; import 'package:smooth_app/helpers/robotoff_insight_helper.dart'; @@ -17,29 +18,72 @@ class ProductQuestionsWidget extends StatefulWidget { State createState() => _ProductQuestionsWidgetState(); } -class _ProductQuestionsWidgetState extends State { +/// This Widget has three views possible: +/// - When loading: a [Shimmer] effect +/// - With questions: a Button to open the dedicated screen +/// - Without questions: the default [EMPTY_WIDGET] +class _ProductQuestionsWidgetState extends State + with AutomaticKeepAliveClientMixin { + /// This Widget has three states possible: + /// - Loading + /// - With questions: questions available AND never answered + /// - Without questions: when there is no question OR a generic error happened + _ProductQuestionsState _state = const _ProductQuestionsLoading(); + bool _annotationVoted = false; + bool _keepWidgetAlive = true; + + @override + void initState() { + super.initState(); + + if (mounted) { + _reloadQuestions(); + } + } + + @override + void didChangeDependencies() { + super.didChangeDependencies(); + + final bool shouldKeepWidgetAlive = + KeepQuestionWidgetAlive.shouldKeepAlive(context); + + // Force the Widget to reload questions only when transitioning + // from not kept alive (false) to keep alive (true) + if (_keepWidgetAlive != shouldKeepWidgetAlive && shouldKeepWidgetAlive) { + _reloadQuestions(); + } + + _keepWidgetAlive = shouldKeepWidgetAlive; + } @override Widget build(BuildContext context) { - final AppLocalizations appLocalizations = AppLocalizations.of(context); - final bool isDarkMode = Theme.of(context).brightness == Brightness.dark; - return FutureBuilder?>( - future: _loadProductQuestions(), - builder: ( - BuildContext context, - AsyncSnapshot?> snapshot, - ) { - final List questions = - snapshot.data ?? []; - - if (questions.isNotEmpty && !_annotationVoted) { + // Mandatory to call with an [AutomaticKeepAliveClientMixin] + super.build(context); + + return AnimatedCrossFade( + crossFadeState: _state is _ProductQuestionsWithoutQuestions + ? CrossFadeState.showFirst + : CrossFadeState.showSecond, + duration: const Duration(seconds: 5), + firstChild: EMPTY_WIDGET, + secondChild: Builder(builder: (BuildContext context) { + final Widget child = _buildContent(context); + + // We need to differentiate with / without a Shimmer, because + // [Shimmer] doesn't support [Ink] + if (_state is _ProductQuestionsWithQuestions) { return InkWell( borderRadius: ANGULAR_BORDER_RADIUS, onTap: () => openQuestionPage( context, product: widget.product, - questions: questions.toList(), + questions: + (_state as _ProductQuestionsWithQuestions).questions.toList( + growable: false, + ), updateProductUponAnswers: _updateProductUponAnswers, ), child: Ink( @@ -50,45 +94,75 @@ class _ProductQuestionsWidgetState extends State { padding: const EdgeInsets.all( SMALL_SPACE, ), - child: SizedBox( - width: double.infinity, - child: Column( - children: [ - // TODO(jasmeet): Use Material icon or SVG (after consulting UX). - Text( - '🏅 ${appLocalizations.tap_to_answer}', - style: Theme.of(context) - .primaryTextTheme - .bodyLarge! - .copyWith( - color: isDarkMode ? Colors.black : WHITE_COLOR, - ), - ), - Padding( - padding: const EdgeInsetsDirectional.only( - top: SMALL_SPACE, - ), - child: Text( - appLocalizations.contribute_to_get_rewards, - style: Theme.of(context) - .primaryTextTheme - .bodyMedium! - .copyWith( - color: isDarkMode ? Colors.black : WHITE_COLOR, - ), - ), - ), - ], - ), + child: child, + ), + ); + } else { + return Shimmer.fromColors( + baseColor: GREY_COLOR, + highlightColor: WHITE_COLOR, + child: Container( + decoration: BoxDecoration( + color: Theme.of(context).colorScheme.primary, + borderRadius: ANGULAR_BORDER_RADIUS, + ), + padding: const EdgeInsets.all( + SMALL_SPACE, ), + child: child, ), ); } - return EMPTY_WIDGET; - }, + }), + ); + } + + Widget _buildContent(BuildContext context) { + final AppLocalizations appLocalizations = AppLocalizations.of(context); + final bool isDarkMode = Theme.of(context).brightness == Brightness.dark; + + return SizedBox( + width: double.infinity, + child: Column( + children: [ + // TODO(jasmeet): Use Material icon or SVG (after consulting UX). + Text( + '🏅 ${appLocalizations.tap_to_answer}', + style: Theme.of(context).primaryTextTheme.bodyLarge!.copyWith( + color: isDarkMode ? Colors.black : WHITE_COLOR, + ), + ), + Padding( + padding: const EdgeInsetsDirectional.only( + top: SMALL_SPACE, + ), + child: Text( + appLocalizations.contribute_to_get_rewards, + style: Theme.of(context).primaryTextTheme.bodyMedium!.copyWith( + color: isDarkMode ? Colors.black : WHITE_COLOR, + ), + ), + ), + ], + ), ); } + Future _reloadQuestions() async { + setState(() => _state = const _ProductQuestionsLoading()); + final List? list = await _loadProductQuestions(); + + if (!mounted) { + return; + } + + if (list?.isNotEmpty == true && !_annotationVoted) { + setState(() => _state = _ProductQuestionsWithQuestions(list!)); + } else { + setState(() => _state = const _ProductQuestionsWithoutQuestions()); + } + } + Future?> _loadProductQuestions() async { final LocalDatabase localDatabase = context.read(); final List questions = @@ -122,4 +196,50 @@ class _ProductQuestionsWidgetState extends State { _annotationVoted = await robotoffInsightHelper.areQuestionsAlreadyVoted(questions); } + + @override + bool get wantKeepAlive => _keepWidgetAlive; +} + +// Widget State +sealed class _ProductQuestionsState { + const _ProductQuestionsState(); +} + +class _ProductQuestionsLoading extends _ProductQuestionsState { + const _ProductQuestionsLoading(); +} + +class _ProductQuestionsWithQuestions extends _ProductQuestionsState { + const _ProductQuestionsWithQuestions(this.questions); + + final List questions; +} + +class _ProductQuestionsWithoutQuestions extends _ProductQuestionsState { + const _ProductQuestionsWithoutQuestions(); +} + +/// Indicates whether we should force a [ProductQuestionsWidget] Widget +/// to keep its state or not +class KeepQuestionWidgetAlive extends InheritedWidget { + const KeepQuestionWidgetAlive({ + super.key, + required this.keepWidgetAlive, + required Widget child, + }) : super(child: child); + + final bool keepWidgetAlive; + + static bool shouldKeepAlive(BuildContext context) { + final KeepQuestionWidgetAlive? result = + context.dependOnInheritedWidgetOfExactType(); + + return result?.keepWidgetAlive ?? false; + } + + @override + bool updateShouldNotify(KeepQuestionWidgetAlive oldWidget) { + return oldWidget.keepWidgetAlive != keepWidgetAlive; + } } From f801c083ef35bebf5771543b74793aed0e81d626 Mon Sep 17 00:00:00 2001 From: Edouard Marquez Date: Sat, 17 Jun 2023 22:32:45 +0200 Subject: [PATCH 2/4] And obviously, let's improve a11n --- packages/smooth_app/lib/l10n/app_en.arb | 8 ++ .../product/product_questions_widget.dart | 74 +++++++++++-------- 2 files changed, 50 insertions(+), 32 deletions(-) diff --git a/packages/smooth_app/lib/l10n/app_en.arb b/packages/smooth_app/lib/l10n/app_en.arb index afad748a059..e87084eb7a4 100644 --- a/packages/smooth_app/lib/l10n/app_en.arb +++ b/packages/smooth_app/lib/l10n/app_en.arb @@ -350,6 +350,14 @@ "@tap_to_answer": { "description": "Button label shown on a product, clicking the button opens a card with unanswered product questions, users can answer these to contribute to Open food facts and gain rewards." }, + "tap_to_answer_hint": "Tap here to answer questions about this product", + "@tap_to_answer_hint": { + "description": "Hint for accessibility readers to answer Robotoff questions." + }, + "robotoff_questions_loading_hint": "Please wait while questions about this product are loaded", + "@robotoff_questions_loading_hint": { + "description": "Hint for accessibility readers while Robotoff questions are loaded" + }, "saving_answer": "Saving your answer", "@saving_answer": { "description": "Dialog shown to users after they have answered a question, while the answer is being saved in the BE." diff --git a/packages/smooth_app/lib/pages/product/product_questions_widget.dart b/packages/smooth_app/lib/pages/product/product_questions_widget.dart index b4b5aad15eb..de17023331a 100644 --- a/packages/smooth_app/lib/pages/product/product_questions_widget.dart +++ b/packages/smooth_app/lib/pages/product/product_questions_widget.dart @@ -70,46 +70,56 @@ class _ProductQuestionsWidgetState extends State duration: const Duration(seconds: 5), firstChild: EMPTY_WIDGET, secondChild: Builder(builder: (BuildContext context) { - final Widget child = _buildContent(context); + final AppLocalizations appLocalizations = AppLocalizations.of(context); + final Widget child = _buildContent(context, appLocalizations); // We need to differentiate with / without a Shimmer, because // [Shimmer] doesn't support [Ink] if (_state is _ProductQuestionsWithQuestions) { - return InkWell( - borderRadius: ANGULAR_BORDER_RADIUS, - onTap: () => openQuestionPage( - context, - product: widget.product, - questions: - (_state as _ProductQuestionsWithQuestions).questions.toList( - growable: false, - ), - updateProductUponAnswers: _updateProductUponAnswers, - ), - child: Ink( - decoration: BoxDecoration( - color: Theme.of(context).colorScheme.primary, - borderRadius: ANGULAR_BORDER_RADIUS, + return Semantics( + value: appLocalizations.tap_to_answer_hint, + button: true, + excludeSemantics: true, + child: InkWell( + borderRadius: ANGULAR_BORDER_RADIUS, + onTap: () => openQuestionPage( + context, + product: widget.product, + questions: + (_state as _ProductQuestionsWithQuestions).questions.toList( + growable: false, + ), + updateProductUponAnswers: _updateProductUponAnswers, ), - padding: const EdgeInsets.all( - SMALL_SPACE, + child: Ink( + decoration: BoxDecoration( + color: Theme.of(context).colorScheme.primary, + borderRadius: ANGULAR_BORDER_RADIUS, + ), + padding: const EdgeInsets.all( + SMALL_SPACE, + ), + child: child, ), - child: child, ), ); } else { - return Shimmer.fromColors( - baseColor: GREY_COLOR, - highlightColor: WHITE_COLOR, - child: Container( - decoration: BoxDecoration( - color: Theme.of(context).colorScheme.primary, - borderRadius: ANGULAR_BORDER_RADIUS, - ), - padding: const EdgeInsets.all( - SMALL_SPACE, + return Semantics( + value: appLocalizations.robotoff_questions_loading_hint, + excludeSemantics: true, + child: Shimmer.fromColors( + baseColor: GREY_COLOR, + highlightColor: WHITE_COLOR, + child: Container( + decoration: BoxDecoration( + color: Theme.of(context).colorScheme.primary, + borderRadius: ANGULAR_BORDER_RADIUS, + ), + padding: const EdgeInsets.all( + SMALL_SPACE, + ), + child: child, ), - child: child, ), ); } @@ -117,8 +127,8 @@ class _ProductQuestionsWidgetState extends State ); } - Widget _buildContent(BuildContext context) { - final AppLocalizations appLocalizations = AppLocalizations.of(context); + Widget _buildContent( + BuildContext context, AppLocalizations appLocalizations) { final bool isDarkMode = Theme.of(context).brightness == Brightness.dark; return SizedBox( From 7c4d3ae034edc8584a815436361c14cfa7de90d9 Mon Sep 17 00:00:00 2001 From: Edouard Marquez Date: Sat, 17 Jun 2023 22:53:27 +0200 Subject: [PATCH 3/4] Improve a bit the animation for the question page --- .../lib/pages/hunger_games/question_page.dart | 74 +++++++++++++------ 1 file changed, 51 insertions(+), 23 deletions(-) diff --git a/packages/smooth_app/lib/pages/hunger_games/question_page.dart b/packages/smooth_app/lib/pages/hunger_games/question_page.dart index 41eca4fa3fc..d32d4d65d7d 100755 --- a/packages/smooth_app/lib/pages/hunger_games/question_page.dart +++ b/packages/smooth_app/lib/pages/hunger_games/question_page.dart @@ -22,17 +22,58 @@ Future openQuestionPage( List? questions, Function()? updateProductUponAnswers, }) => - showDialog( + showGeneralDialog( context: context, barrierColor: Colors.black.withOpacity(0.7), - builder: (_) => BackdropFilter( - filter: ImageFilter.blur(sigmaX: 1.0, sigmaY: 1.0), - child: _QuestionPage( - product: product, - questions: questions, - updateProductUponAnswers: updateProductUponAnswers, - ), - ), + pageBuilder: (_, __, ___) => EMPTY_WIDGET, + transitionBuilder: ( + BuildContext context, + Animation a1, + Animation a2, + Widget child, + ) { + return SafeArea( + child: Stack( + children: [ + Positioned.fill( + child: GestureDetector( + excludeFromSemantics: true, + onTap: () => Navigator.of(context).maybePop(), + ), + ), + Align( + alignment: Alignment.center, + child: Transform.scale( + scale: a1.value, + child: Opacity( + opacity: a1.value, + child: SafeArea( + child: BackdropFilter( + filter: ImageFilter.blur(sigmaX: 1.0, sigmaY: 1.0), + child: _QuestionPage( + product: product, + questions: questions, + updateProductUponAnswers: updateProductUponAnswers, + ), + ), + ), + ), + ), + ), + Positioned.directional( + textDirection: Directionality.of(context), + top: 0.0, + start: SMALL_SPACE, + child: Opacity( + opacity: a1.value, + child: const _CloseButton(), + ), + ), + ], + ), + ); + }, + transitionDuration: const Duration(milliseconds: 300), ); class _QuestionPage extends StatefulWidget { @@ -112,20 +153,7 @@ class _QuestionPageState extends State<_QuestionPage> } return true; }, - child: Stack( - children: [ - Align( - alignment: Alignment.center, - child: _buildAnimationSwitcher(), - ), - Positioned.directional( - textDirection: Directionality.of(context), - top: 0.0, - start: SMALL_SPACE, - child: const _CloseButton(), - ), - ], - ), + child: Center(child: _buildAnimationSwitcher()), ); AnimatedSwitcher _buildAnimationSwitcher() => AnimatedSwitcher( From e4be8a95e7903341686b761ad69756de606aa6bb Mon Sep 17 00:00:00 2001 From: Edouard Marquez Date: Sat, 17 Jun 2023 23:52:16 +0200 Subject: [PATCH 4/4] Prefer constants for durations --- .../lib/pages/hunger_games/question_page.dart | 2 +- .../pages/product/product_questions_widget.dart | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/smooth_app/lib/pages/hunger_games/question_page.dart b/packages/smooth_app/lib/pages/hunger_games/question_page.dart index d32d4d65d7d..ff786f43672 100755 --- a/packages/smooth_app/lib/pages/hunger_games/question_page.dart +++ b/packages/smooth_app/lib/pages/hunger_games/question_page.dart @@ -73,7 +73,7 @@ Future openQuestionPage( ), ); }, - transitionDuration: const Duration(milliseconds: 300), + transitionDuration: SmoothAnimationsDuration.medium, ); class _QuestionPage extends StatefulWidget { diff --git a/packages/smooth_app/lib/pages/product/product_questions_widget.dart b/packages/smooth_app/lib/pages/product/product_questions_widget.dart index de17023331a..429c9da01ed 100644 --- a/packages/smooth_app/lib/pages/product/product_questions_widget.dart +++ b/packages/smooth_app/lib/pages/product/product_questions_widget.dart @@ -5,6 +5,7 @@ import 'package:provider/provider.dart'; import 'package:shimmer/shimmer.dart'; import 'package:smooth_app/database/local_database.dart'; import 'package:smooth_app/generic_lib/design_constants.dart'; +import 'package:smooth_app/generic_lib/duration_constants.dart'; import 'package:smooth_app/helpers/robotoff_insight_helper.dart'; import 'package:smooth_app/pages/hunger_games/question_page.dart'; import 'package:smooth_app/query/product_questions_query.dart'; @@ -67,7 +68,7 @@ class _ProductQuestionsWidgetState extends State crossFadeState: _state is _ProductQuestionsWithoutQuestions ? CrossFadeState.showFirst : CrossFadeState.showSecond, - duration: const Duration(seconds: 5), + duration: SmoothAnimationsDuration.long, firstChild: EMPTY_WIDGET, secondChild: Builder(builder: (BuildContext context) { final AppLocalizations appLocalizations = AppLocalizations.of(context); @@ -75,6 +76,8 @@ class _ProductQuestionsWidgetState extends State // We need to differentiate with / without a Shimmer, because // [Shimmer] doesn't support [Ink] + final Color backgroundColor = Theme.of(context).colorScheme.primary; + if (_state is _ProductQuestionsWithQuestions) { return Semantics( value: appLocalizations.tap_to_answer_hint, @@ -93,7 +96,7 @@ class _ProductQuestionsWidgetState extends State ), child: Ink( decoration: BoxDecoration( - color: Theme.of(context).colorScheme.primary, + color: backgroundColor, borderRadius: ANGULAR_BORDER_RADIUS, ), padding: const EdgeInsets.all( @@ -108,11 +111,12 @@ class _ProductQuestionsWidgetState extends State value: appLocalizations.robotoff_questions_loading_hint, excludeSemantics: true, child: Shimmer.fromColors( - baseColor: GREY_COLOR, - highlightColor: WHITE_COLOR, + baseColor: backgroundColor, + highlightColor: WHITE_COLOR.withOpacity(0.5), + period: SmoothAnimationsDuration.long * 2, child: Container( decoration: BoxDecoration( - color: Theme.of(context).colorScheme.primary, + color: backgroundColor, borderRadius: ANGULAR_BORDER_RADIUS, ), padding: const EdgeInsets.all(