feat: Step-by-step fast-track #4698

surajitbasak · 2023-10-04T08:58:52Z

Product Design page is now split into multiple pages

Product.Design.Recording.mp4

Let me know if I have made any mistakes or anything need to be changed further

g123k · 2023-10-05T12:24:21Z

Perfect, before reviewing your code, could you:

Ensure to be on a "full screen" page (= I mean without the bottom tabs
Add a back button, please?

surajitbasak · 2023-10-05T14:48:01Z

Okay
Do you mean the screen should be without the 'Profile' 'scan' 'list' bottom tabs
and
There should be an Arrow type back button at the top-left corner of the page.
Is my understanding correct?

g123k · 2023-10-05T15:18:28Z

Okay Do you mean the screen should be without the 'Profile' 'scan' 'list' bottom tabs and There should be an Arrow type back button at the top-left corner of the page. Is my understanding correct?

Yes exactly, basically for the first one, you juste need to use the parent Navigator.

…into prodinfodesignchange

…k/smooth-app into prodinfodesignchange

surajitbasak · 2023-10-05T20:21:14Z

I have committed the changes.

WhatsApp.Video.2023-10-06.at.1.48.22.AM.mp4

Let me know if these are okay or not :)

g123k · 2023-10-06T14:23:15Z

Perfect in terms of UI
You still have warnings to fix from the GitHub Action: https://github.com/openfoodfacts/smooth-app/actions/runs/6423997267/job/17466600750?pr=4698

g123k · 2023-10-06T14:23:50Z

packages/smooth_app/lib/cards/product_cards/smooth_product_card_not_found.dart

              );
-
+              // await Navigator.push<void>(


Why do you comment this code?

It was the previous code that opened the AddProductInformation page with the Tabs Below it.
So I Commented it and added new code Above it just to open that page from Parent Navigator.
Should I Remove that commented code?

g123k · 2023-10-06T14:25:03Z

packages/smooth_app/lib/generic_lib/buttons/smooth_large_button_with_icon.dart

@@ -42,7 +40,7 @@ class SmoothLargeButtonWithIcon extends StatelessWidget {

    return SmoothSimpleButton(
      minWidth: double.infinity,
-      padding: padding ?? const EdgeInsets.all(10),
+      padding: padding ?? const EdgeInsets.symmetric(vertical: 10),


By changing this button, it will have an impact on the whole app.
Please provide specific values when you need it.

Actually in figma design those buttons' horizontal padding was slightly high that is why i made this change.
However i will revert back to the original, no issue.

g123k · 2023-10-06T14:25:19Z

packages/smooth_app/lib/generic_lib/design_constants.dart

@@ -17,7 +17,7 @@ const double MINIMUM_TOUCH_SIZE = kMinInteractiveDimension;
 const double DEFAULT_ICON_SIZE = 24.0;

 /// Background, e.g SmoothCard
-const Radius ROUNDED_RADIUS = Radius.circular(20.0);
+const Radius ROUNDED_RADIUS = Radius.circular(10.0);


By changing this button, it will have an impact on the whole app.
Please provide specific radiuses when you need it.

Okay, in Figma the corners looked less than 20px,
However, I will revert back to the original.

g123k · 2023-10-06T14:29:04Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+          child: Column(
+            crossAxisAlignment: CrossAxisAlignment.start,
+            children: <Widget>[
+              /*if (widget.displayPictures) _buildCard(_getImageRows(context)),


Same question as before, why do you comment this code?

Oh Sorry, I will remove that comment.
That commented part is used from the Line Number : 230

g123k · 2023-10-06T14:29:35Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+
+  Widget _backButton() {
+    return Container(
+        margin: const EdgeInsets.only(left: 10, right: 10, top: 10),


Please use EdgeInsetsDirectional instead

g123k · 2023-10-06T14:30:32Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+    return Container(
+        margin: const EdgeInsets.only(left: 10, right: 10, top: 10),
+        width: 20,
+        height: 20,


ues, by convention, please write it as 20.0

g123k · 2023-10-06T14:30:51Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+          onPressed: () {
+            _onWillPop().then((bool leaveThePage) => leaveThePage ? Navigator.of(context).pop() : null);
+          },
+          child: const Text('Cancel', style: TextStyle(color: Colors.black, fontSize: 20, fontWeight: FontWeight.bold)),


Please don't hardcode i11n

surajitbasak · 2023-10-06T18:35:09Z

Hey @g123k , I have committed the Reviewed changes. Can you please go through that? Thanks :)

surajitbasak · 2023-10-06T18:38:04Z

After the changes the design looks like this

g123k

Please use the double format + trailing commas + don't hardcode values
Tkx

g123k · 2023-10-06T19:07:48Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+                child: Text(appLocalizations.new_product_additional_ecoscore,
+                    style: TextStyle(
+                        color: Theme.of(context).colorScheme.onPrimary))),
+            const SizedBox(width: 5),


g123k · 2023-10-06T19:07:52Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+          child: Row(children: <Widget>[
+            Icon(Icons.filter_2,
+                color: Theme.of(context).colorScheme.onPrimary),
+            const SizedBox(width: 15),


g123k · 2023-10-06T19:07:56Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

-        title: Text(appLocalizations.new_product_additional_ecoscore),
-        trailing: Icon(
-          _ecoscoreExpanded ? Icons.expand_less : Icons.expand_more,
+      const SizedBox(height: 15),


g123k · 2023-10-06T19:08:00Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

@@ -296,20 +390,39 @@ class _AddNewProductPageState extends State<AddNewProductPage>
    final Attribute? attribute = _getAttribute(Attribute.ATTRIBUTE_ECOSCORE);
    return <Widget>[
      AddNewProductTitle(appLocalizations.new_product_title_ecoscore),
+      const SizedBox(height: 15),


g123k · 2023-10-06T19:08:03Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

  List<Widget> _getNutriscoreRows(final BuildContext context) {
    final AppLocalizations appLocalizations = AppLocalizations.of(context);
    final Attribute? attribute = _getAttribute(Attribute.ATTRIBUTE_NUTRISCORE);
    return <Widget>[
      AddNewProductTitle(appLocalizations.new_product_title_nutriscore),
+      const SizedBox(height: 15),


g123k · 2023-10-06T19:12:10Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+            icon: const Icon(Icons.arrow_back)));
+  }
+
+  Widget _getButtons({String doneBtnText = 'Next'}) {


No hardcoded sentence, please

g123k · 2023-10-06T19:12:25Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+      children: <Widget>[
+        ElevatedButton(
+          style: ElevatedButton.styleFrom(
+              minimumSize: Size(MediaQuery.of(context).size.width * 0.35, 40),


g123k · 2023-10-06T19:12:55Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+              minimumSize: Size(MediaQuery.of(context).size.width * 0.35, 40),
+              backgroundColor: Theme.of(context).colorScheme.secondary,
+              shape: RoundedRectangleBorder(
+                  borderRadius: BorderRadius.circular(20))),


double + trailing comma

g123k · 2023-10-06T19:13:12Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+                  fontSize: 20,
+                  fontWeight: FontWeight.bold)),


double + trailing comma

g123k · 2023-10-06T19:13:20Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+                  fontSize: 20,
+                  fontWeight: FontWeight.bold)),
+        ),
+        const SizedBox(width: 10),


codecov-commenter · 2023-10-07T08:52:44Z

Codecov Report

Merging #4698 (cc382f6) into develop (8ac2fa9) will decrease coverage by 0.04%.
Report is 1 commits behind head on develop.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #4698      +/-   ##
==========================================
- Coverage     9.90%   9.86%   -0.04%     
==========================================
  Files          310     310              
  Lines        15815   15871      +56     
==========================================
  Hits          1566    1566              
- Misses       14249   14305      +56

Files	Coverage Δ
packages/smooth_app/lib/themes/color_schemes.dart	`85.71% <ø> (ø)`
...s/product_cards/smooth_product_card_not_found.dart	`0.00% <0.00%> (ø)`
.../smooth_app/lib/helpers/image_field_extension.dart	`0.00% <0.00%> (ø)`
.../smooth_app/lib/pages/navigator/app_navigator.dart	`0.00% <0.00%> (ø)`
..._app/lib/pages/product/add_new_product_helper.dart	`0.00% <0.00%> (ø)`
...th_app/lib/pages/product/add_new_product_page.dart	`0.00% <0.00%> (ø)`

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

surajitbasak · 2023-10-08T11:08:02Z

Hey @g123k , I have made the changes & Committed, Let me know if any other changes need to be made :)

g123k

A few last comments

packages/smooth_app/lib/pages/product/add_new_product_page.dart

g123k · 2023-10-12T08:51:33Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

-                const SizedBox(height: MINIMUM_TOUCH_SIZE),
-              ],
-            ),
+        body: SafeArea(


Why do you get rid of the Scaffold?
One of the goals is to have a consustent color for the status bar on Android and iOS

@surajitbasak Any reason?

Any reason for?
This the way all screens are implemented in the app

g123k · 2023-10-12T08:52:27Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+                decoration: BoxDecoration(
+                  gradient: LinearGradient(
+                    colors: <Color>[
+                      Theme.of(context).colorScheme.inversePrimary,


When you call of(context) several times, please ensure to cache this (by storing it in a variable)

g123k · 2023-10-12T08:53:41Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+                margin: EdgeInsets.zero,
+                elevation: 15.0,
+                child: SizedBox(
+                  height: MediaQuery.of(context).size.height * 0.1,


Now that we use the latest version of Flutter, you can simplify by using MediaQuery.sizeOf()

g123k · 2023-10-12T08:54:08Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+  Widget _backButton() {
+    return Container(
+      margin: const EdgeInsetsDirectional.only(
+          start: 10.0, end: 10.0, top: 10.0, bottom: 0.0),


Please add the missing trailing comma at the end

g123k · 2023-10-12T08:55:01Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+        ElevatedButton(
+          style: ElevatedButton.styleFrom(
+            minimumSize: Size(MediaQuery.of(context).size.width * 0.35, 40.0),
+            backgroundColor: Theme.of(context).colorScheme.secondary,


Same comment here for of() + MediaQuery

g123k · 2023-10-12T08:55:27Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+        ElevatedButton(
+          style: ElevatedButton.styleFrom(
+            minimumSize: Size(MediaQuery.of(context).size.width * 0.35, 40.0),
+            backgroundColor: const Color(0xFF341100),


Please make this color a constant

g123k · 2023-10-12T08:55:48Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+          onPressed: () {
+            _pageController.nextPage(
+                duration: SmoothAnimationsDuration.short,
+                curve: Curves.easeOut);


Please add the trailing comma

… used

surajitbasak · 2023-10-12T09:44:56Z

Hey @g123k , I have committed the changes, Let me know if it's okay or not :)

g123k · 2023-10-12T12:43:58Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

+
+  @override
+  void didChangeDependencies() {
+    print('Mummy di');


No print please
and why do you need to fetch the colorScheme here?

Edit: Just get it at the beginning of the build() method

surajitbasak · 2023-10-12T14:08:13Z

@g123k , i made the changes, please check once :)

g123k · 2023-10-16T08:21:00Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

  }

  @override
  Widget build(BuildContext context) {
-    final AppLocalizations appLocalizations = AppLocalizations.of(context);
+    _colorScheme = _colorScheme = Theme.of(context).colorScheme;


Perfect, please just remove the attribute from the class, as it's not used anymore

g123k · 2023-10-16T08:21:30Z

packages/smooth_app/lib/pages/product/add_new_product_page.dart

-                const SizedBox(height: MINIMUM_TOUCH_SIZE),
-              ],
-            ),
+        body: SafeArea(


@surajitbasak Any reason?

g123k

Just 2 comments

g123k · 2023-10-24T08:13:28Z

Just 2 comments

Do you make any progress on this PR @surajitbasak?
Do you need any help?

surajitbasak · 2023-10-25T11:57:37Z

Just 2 comments

Do you make any progress on this PR @surajitbasak? Do you need any help?

No i didn't make any further changes,
And yes you can help me to guide if you want me to add the App bar on that page.

M123-dev · 2023-10-30T16:33:29Z

packages/smooth_app/lib/pages/product/add_new_product_helper.dart

+        style: const TextStyle(
+          fontSize: 18.0,
+          fontWeight: FontWeight.bold,
+        ),


That wont look good on small devices, please use

Theme.of(context).textTheme.displaySmall

or another one which is most fitting and edit it with .copyWith()

teolemon · 2023-11-09T17:03:27Z

@surajitbasak could you fix the 3 little merge conflicts, so that we can rebase the PR ?

surajitbasak · 2023-11-10T20:01:06Z

@surajitbasak could you fix the 3 little merge conflicts, so that we can rebase the PR ?

I have resolved those merge conflicts, Please let me know if i need to do anything else :)

g123k

OK, let's go for this PR.
In the case, where there are some changes required, I will open a new PR.

surajitbasak added 2 commits October 3, 2023 19:41

Product Info Design Changed

58c03f9

Product Design Page splitted

019f694

surajitbasak requested a review from a team as a code owner October 4, 2023 08:58

github-actions bot assigned surajitbasak Oct 4, 2023

github-actions bot added 🥫 Product page Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. dependencies 🌐 l10n labels Oct 4, 2023

Merge branch 'develop' into prodinfodesignchange

76a59d8

surajitbasak mentioned this pull request Oct 4, 2023

Fast track: implement the final design #4671

Open

M123-dev requested a review from g123k October 4, 2023 20:39

surajitbasak added 4 commits October 6, 2023 00:46

Back Button Added

44b4bb7

Product info page fullscreen, and Back button Added

a058c5a

Merge branch 'develop' of https://github.com/openfoodfacts/smooth-app …

621759a

…into prodinfodesignchange

Merge branch 'prodinfodesignchange' of https://github.com/surajitbasa…

8541924

…k/smooth-app into prodinfodesignchange

teolemon changed the title ~~Product Info Design has been split.~~ feat: Step-by-step fast-track Oct 6, 2023

g123k mentioned this pull request Oct 6, 2023

Fast track: add a button to close the edit page #4670

Closed

g123k requested changes Oct 6, 2023

View reviewed changes

Made Review Changes

cc382f6

g123k requested changes Oct 6, 2023

View reviewed changes

used the double format + trailing commas + removed hardcode values

70ccdc8

g123k requested changes Oct 12, 2023

View reviewed changes

Theme.of(Context) cached, added trailiing commas, Mediaquery.sizeof()…

929b9d2

… used

g123k reviewed Oct 12, 2023

View reviewed changes

Print Removed and Colorsceme is now under build function

b5cbed0

g123k requested changes Oct 16, 2023

View reviewed changes

github-actions bot added the PR: needs rebase label Oct 25, 2023

M123-dev reviewed Oct 30, 2023

View reviewed changes

surajitbasak added 4 commits November 11, 2023 00:02

Text style Edited with copywith()

0ab01d1

Text style Edited with copywith()

4b5ccee

3 Merge Conflicts resolved

ec09a32

Resolved 3 Merge conflicts

c349163

github-actions bot removed the PR: needs rebase label Nov 10, 2023

github-actions bot added PR: needs rebase and removed PR: needs rebase labels Nov 13, 2023

g123k approved these changes Nov 15, 2023

View reviewed changes

github-actions bot added the PR: needs rebase label Nov 15, 2023

Merge branch 'develop' into prodinfodesignchange

8b84963

github-actions bot added PR: needs rebase product page and removed PR: needs rebase labels Nov 15, 2023

teolemon merged commit bbf84d1 into openfoodfacts:develop Nov 15, 2023

github-actions bot mentioned this pull request Nov 15, 2023

chore(develop): 🚀 Open Food Facts Mobile app - Release 4.12.0. #4737

Merged

teolemon mentioned this pull request Dec 8, 2023

FastTrack regression in production #4880

Closed

feat: Step-by-step fast-track #4698

feat: Step-by-step fast-track #4698

Conversation

surajitbasak commented Oct 4, 2023

g123k commented Oct 5, 2023

surajitbasak commented Oct 5, 2023

g123k commented Oct 5, 2023

surajitbasak commented Oct 5, 2023

g123k commented Oct 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

surajitbasak commented Oct 6, 2023

surajitbasak commented Oct 6, 2023

g123k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 7, 2023

Codecov Report

surajitbasak commented Oct 8, 2023

g123k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

surajitbasak commented Oct 12, 2023

g123k Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

surajitbasak commented Oct 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g123k left a comment

Choose a reason for hiding this comment

g123k commented Oct 24, 2023

surajitbasak commented Oct 25, 2023

Choose a reason for hiding this comment

teolemon commented Nov 9, 2023

surajitbasak commented Nov 10, 2023

g123k left a comment

Choose a reason for hiding this comment

g123k Oct 12, 2023 •

edited

Loading