-
-
Notifications
You must be signed in to change notification settings - Fork 259
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
Update scan page as per the UX spec (well, roughly) #659
Changes from all commits
5c4f815
ca59975
308feb1
a4be0af
fffd378
17e6cdf
f48e785
c75c5f0
a8258e4
a132e53
996bf5d
4e7428d
75fde2e
03753fa
9e32738
b773e19
60a75a1
0c7326c
c271b90
058fc9f
a983b6c
b85f7fe
7eb49a5
fa59490
771bef5
70b693c
a82aedc
88bbb66
8ba7bd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,114 +1,140 @@ | ||
import 'package:flutter/material.dart'; | ||
import 'package:flutter_gen/gen_l10n/app_localizations.dart'; | ||
import 'package:flutter_svg/flutter_svg.dart'; | ||
import 'package:provider/provider.dart'; | ||
import 'package:qr_code_scanner/qr_code_scanner.dart'; | ||
import 'package:smooth_app/data_models/continuous_scan_model.dart'; | ||
import 'package:smooth_app/pages/personalized_ranking_page.dart'; | ||
import 'package:smooth_app/widgets/smooth_product_carousel.dart'; | ||
import 'package:smooth_ui_library/smooth_ui_library.dart'; | ||
import 'package:smooth_ui_library/util/ui_helpers.dart'; | ||
|
||
class ContinuousScanPage extends StatelessWidget { | ||
final GlobalKey _scannerViewKey = GlobalKey(debugLabel: 'Barcode Scanner'); | ||
|
||
@override | ||
Widget build(BuildContext context) { | ||
final ContinuousScanModel model = context.watch<ContinuousScanModel>(); | ||
final AppLocalizations localizations = AppLocalizations.of(context)!; | ||
final Size screenSize = MediaQuery.of(context).size; | ||
final double carouselHeight = screenSize.height / 2; | ||
final double viewFinderBottomOffset = carouselHeight / 2.0; | ||
return Scaffold( | ||
appBar: AppBar(toolbarHeight: 0.0), | ||
body: Stack( | ||
children: <Widget>[ | ||
Container( | ||
alignment: Alignment.center, | ||
color: Colors.black, | ||
child: Padding( | ||
// Double the offset to account for the vertical centering. | ||
padding: EdgeInsets.only(bottom: 2 * viewFinderBottomOffset), | ||
child: SvgPicture.asset( | ||
'assets/actions/scanner_alt_2.svg', | ||
width: 60.0, | ||
height: 60.0, | ||
color: Colors.white, | ||
return LayoutBuilder( | ||
builder: (BuildContext context, BoxConstraints constraints) { | ||
final ContinuousScanModel model = context.watch<ContinuousScanModel>(); | ||
final Size screenSize = MediaQuery.of(context).size; | ||
final Size scannerSize = Size( | ||
screenSize.width * 0.6, | ||
screenSize.width * 0.33, | ||
); | ||
final double carouselHeight = | ||
constraints.maxHeight / 1.81; // roughly 55% of the available height | ||
final double buttonRowHeight = areButtonsRendered(model) ? 48 : 0; | ||
final double availableScanHeight = | ||
constraints.maxHeight - carouselHeight - buttonRowHeight; | ||
// Padding for the qr code scanner. This ensures the scanner has equal spacing between buttons and carousel. | ||
final EdgeInsets qrScannerPadding = EdgeInsets.only( | ||
top: | ||
(availableScanHeight - scannerSize.height) / 2 + buttonRowHeight); | ||
final double viewFinderBottomOffset = carouselHeight / 2.0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not entirely sure if this is needed.... (?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was added by @slava-sh a while ago, but I don't know what it's exact purpose is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will try and test on an iOS device, (device_preview renders fine without this offset but maybe there's some issue on iOS) |
||
return Scaffold( | ||
appBar: AppBar(toolbarHeight: 0.0), | ||
body: Stack( | ||
children: <Widget>[ | ||
Container( | ||
alignment: Alignment.center, | ||
color: Colors.black, | ||
child: Padding( | ||
padding: qrScannerPadding, | ||
child: SvgPicture.asset( | ||
'assets/actions/scanner_alt_2.svg', | ||
width: 60.0, | ||
height: 60.0, | ||
color: Colors.white, | ||
), | ||
), | ||
), | ||
), | ||
SmoothRevealAnimation( | ||
delay: 400, | ||
startOffset: Offset.zero, | ||
animationCurve: Curves.easeInOutBack, | ||
child: QRView( | ||
overlay: QrScannerOverlayShape( | ||
// We use [SmoothViewFinder] instead of the overlay. | ||
overlayColor: Colors.transparent, | ||
// This offset adjusts the scanning area on iOS. | ||
cutOutBottomOffset: viewFinderBottomOffset, | ||
SmoothRevealAnimation( | ||
delay: 400, | ||
startOffset: Offset.zero, | ||
animationCurve: Curves.easeInOutBack, | ||
child: QRView( | ||
overlay: QrScannerOverlayShape( | ||
// We use [SmoothViewFinder] instead of the overlay. | ||
overlayColor: Colors.transparent, | ||
// This offset adjusts the scanning area on iOS. | ||
cutOutBottomOffset: viewFinderBottomOffset, | ||
), | ||
key: _scannerViewKey, | ||
onQRViewCreated: model.setupScanner, | ||
), | ||
key: _scannerViewKey, | ||
onQRViewCreated: model.setupScanner, | ||
), | ||
), | ||
SmoothRevealAnimation( | ||
delay: 400, | ||
startOffset: const Offset(0.0, 0.1), | ||
animationCurve: Curves.easeInOutBack, | ||
child: Column( | ||
mainAxisAlignment: MainAxisAlignment.center, | ||
children: <Widget>[ | ||
Padding( | ||
// Double the offset to account for the vertical centering. | ||
padding: EdgeInsets.only(bottom: 2 * viewFinderBottomOffset), | ||
child: SmoothViewFinder( | ||
boxSize: Size( | ||
screenSize.width * 0.6, | ||
screenSize.width * 0.33, | ||
SmoothRevealAnimation( | ||
delay: 400, | ||
startOffset: const Offset(0.0, 0.1), | ||
animationCurve: Curves.easeInOutBack, | ||
child: Column( | ||
mainAxisAlignment: MainAxisAlignment.start, | ||
children: <Widget>[ | ||
Padding( | ||
padding: qrScannerPadding, | ||
child: SmoothViewFinder( | ||
boxSize: scannerSize, | ||
lineLength: screenSize.width * 0.8, | ||
), | ||
lineLength: screenSize.width * 0.8, | ||
), | ||
), | ||
], | ||
], | ||
), | ||
), | ||
), | ||
SmoothRevealAnimation( | ||
delay: 400, | ||
startOffset: const Offset(0.0, -0.1), | ||
animationCurve: Curves.easeInOutBack, | ||
child: Column( | ||
mainAxisAlignment: MainAxisAlignment.end, | ||
children: <Widget>[ | ||
AnimatedOpacity( | ||
opacity: model.isNotEmpty ? 1.0 : 0.0, | ||
duration: const Duration(milliseconds: 50), | ||
child: Row( | ||
mainAxisAlignment: MainAxisAlignment.spaceEvenly, | ||
children: <Widget>[ | ||
ElevatedButton.icon( | ||
icon: const Icon(Icons.cancel_outlined), | ||
onPressed: model.clearScanSession, | ||
label: const Text('Clear'), | ||
), | ||
ElevatedButton.icon( | ||
icon: const Icon(Icons.emoji_events_outlined), | ||
onPressed: () => _openPersonalizedRankingPage(context), | ||
label: Text(localizations.myPersonalizedRanking), | ||
), | ||
], | ||
), | ||
), | ||
Padding( | ||
padding: const EdgeInsets.symmetric(vertical: 8.0), | ||
child: SmoothProductCarousel( | ||
SmoothRevealAnimation( | ||
delay: 400, | ||
startOffset: const Offset(0.0, -0.1), | ||
animationCurve: Curves.easeInOutBack, | ||
child: Column( | ||
mainAxisAlignment: MainAxisAlignment.start, | ||
children: <Widget>[ | ||
_buildButtonsRow(context, model), | ||
const Spacer(), | ||
SmoothProductCarousel( | ||
showSearchCard: true, | ||
height: carouselHeight, | ||
), | ||
), | ||
], | ||
], | ||
), | ||
), | ||
], | ||
), | ||
); | ||
}); | ||
} | ||
|
||
Widget _buildButtonsRow(BuildContext context, ContinuousScanModel model) { | ||
final ButtonStyle buttonStyle = ButtonStyle( | ||
shape: MaterialStateProperty.all<RoundedRectangleBorder>( | ||
RoundedRectangleBorder( | ||
borderRadius: BorderRadius.circular(18.0), | ||
), | ||
), | ||
); | ||
return AnimatedOpacity( | ||
opacity: areButtonsRendered(model) ? 0.8 : 0.0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it would make sense to display the "clear" button when we only have one product. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm good point, I'm still not sold on this app behavior to be honest, because both buttons are very interdependent in functionality and the functionality is super unclear to the user unless they experiment with the buttons.
So I personally think it makes sense to pair and show them together, but I will bring it up with Pierre, Stephane and others this week |
||
duration: const Duration(milliseconds: 50), | ||
child: Padding( | ||
padding: const EdgeInsets.symmetric( | ||
vertical: VERY_SMALL_SPACE, horizontal: MEDIUM_SPACE), | ||
child: Row( | ||
mainAxisAlignment: MainAxisAlignment.spaceBetween, | ||
children: <Widget>[ | ||
ElevatedButton.icon( | ||
style: buttonStyle, | ||
icon: const Icon(Icons.cancel_outlined), | ||
onPressed: model.clearScanSession, | ||
// TODO(jasmeet): Internationalize | ||
label: const Text('Clear'), | ||
), | ||
ElevatedButton.icon( | ||
style: buttonStyle, | ||
icon: const Icon(Icons.emoji_events_outlined), | ||
onPressed: () => _openPersonalizedRankingPage(context), | ||
// TODO(jasmeet): Internationalize | ||
label: Text('Compare ${model.getBarcodes().length} Products'), | ||
), | ||
), | ||
], | ||
], | ||
), | ||
), | ||
); | ||
} | ||
|
@@ -126,4 +152,7 @@ class ContinuousScanPage extends StatelessWidget { | |
); | ||
await model.refresh(); | ||
} | ||
|
||
bool areButtonsRendered(ContinuousScanModel model) => | ||
model.hasMoreThanOneProduct; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it right that both use the width
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, they are supposed to be proportional to one another