feat: Clearer 'Packaging components' preview and modified the maximum size of the autocomplete tab. #3744

thomas-algo · 2023-02-27T22:04:47Z

What

Updated the preview of the package in the 'Packaging components ' tab (cf two first screenshots)
- Clearer
- More coherent with what is shown on the product page.
- Updated the getTitle() and created getSubTitle() function from packages/smooth_app/lib/pages/product/edit_new_packagings.dart
Autocomplete now gives hints before anything is typed. (removed from this PR, implemented in feat: 3749 - improvements for packaging suggestions #3750)
- The autocomplete now gives the most common answers for the field before user start typing. (Useful for packaging material, recycling...) (cf third screenshot)
Modified the maximum size of the autocomplete tab.
- The autocomplete tab was unreadable if the text field was too close to the keyboard, even if the page was scrolled afterward. (cf two last screenshots)
- Fixing _AssertionError 'maxOptionsHeight >= 0': is not true: The old formula could give negative values.

Screenshot

Old preview of the package in the 'Packaging components ' tab
New preview of the package in the 'Packaging components ' tab
Autocomplete now gives hints before anything is typed
Old behavior of the autocomplete max size

Stays small even when moved:

Preview is now clearer and more coherent with what is shown on the product page.

…ght fixed - The autocomplete now gives the most common answers for the field before user start typing. (Useful for packaging material, recycling...) - Fixing _AssertionError 'maxOptionsHeight >= 0': is not true: The old formula could give negative values. It could also create an unreadable autocomplete if the field was too close to the keyboard.

monsieurtanuki

¡Hi @thomas-algo!
Please have a look at my comments.
I approve some of your UI/UX changes, but I cannot see the related issue.

monsieurtanuki · 2023-02-28T07:51:36Z

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

+    if (controllerMaterial.text.isNotEmpty & controllerWeight.text.isNotEmpty) {
+      result.add('(${controllerMaterial.text} : ${controllerWeight.text}g)');
+    }
+
+    else if (controllerMaterial.text.isNotEmpty ) { // Therefore controllerWeight.text is empty
+          result.add('(${controllerMaterial.text})');
+    }
+


Suggested change

if (controllerMaterial.text.isNotEmpty & controllerWeight.text.isNotEmpty) {

result.add('(${controllerMaterial.text} : ${controllerWeight.text}g)');

}

else if (controllerMaterial.text.isNotEmpty ) { // Therefore controllerWeight.text is empty

result.add('(${controllerMaterial.text})');

}

if (controllerMaterial.text.isNotEmpty) {

if (controllerWeight.text.isNotEmpty) {

result.add('(${controllerMaterial.text} : ${controllerWeight.text}g)');

} else {

result.add('(${controllerMaterial.text})');

}

}

So you don't display the weight if the material is not specified, which is a bit strange.

Yes, this is the same behavior as in the final product page. However, it might be interesting to have the mass in both cases (Nevertheless, the case where 'material' is empty and 'weight' is filled is quit rare).

Again, I don't know where the changed display was suggested. And how rare the "the case where 'material' is empty and 'weight' is filled".
I would suggest at least to consider this very rare case with an additional test: "if weight is not empty and material is empty, let's display only weight".
By the way in English there's no space before a :. In French it's an unbreakable space.

Suggested change

if (controllerMaterial.text.isNotEmpty & controllerWeight.text.isNotEmpty) {

result.add('(${controllerMaterial.text} : ${controllerWeight.text}g)');

}

else if (controllerMaterial.text.isNotEmpty ) { // Therefore controllerWeight.text is empty

result.add('(${controllerMaterial.text})');

}

if (controllerMaterial.text.isNotEmpty & controllerWeight.text.isNotEmpty) {

result.add('(${controllerMaterial.text}: ${controllerWeight.text}g)');

}

if (controllerMaterial.text.isNotEmpty & controllerWeight.text.isEmpty) {

result.add('(${controllerMaterial.text})');

}

if (controllerMaterial.text.isEmpty & controllerWeight.text.isNotEmpty) {

result.add('(${controllerWeight.text}g)');

}

// And nothing is added if they are both empty.

Thanks for your advises, here is a version taking this case into account. I am not sure the display change was suggested elsewhere.

By the way in English there's no space before a :. In French it's an unbreakable space.

@monsieurtanuki: on the server we have a translatable string 'sep' which is an empty string in English, and a space in French and other languages. Is this something already handled in some way in the app? I didn't see something equivalent in the lib/l10n/app_en.arb file

@stephanegigandet We don't have this sep in Smoothie.
Could you list all languages that do have a space, e.g. taken from the server?

@monsieurtanuki I just checked, we only do it for French.

=head2 separator_before_colon( $l ) =head3 Arguments =head4 language code $l In some languages like French, colons have a space before them. e.g. "Valeur : 500" in French, "Value: 500" in English This function returns a non-breaking space character for those languages. =cut sub separator_before_colon ($l) { if ($l eq 'fr') { return "\N{U+00A0}"; } else { return ''; } }

Maybe we can put a similar function in Smoothie, it's likely to be useful in other places.

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

monsieurtanuki · 2023-02-28T07:56:16Z

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

                  return OpenFoodAPIClient.getSuggestions(
                    tagType!,
                    language: ProductQuery.getLanguage()!,
-                    limit: 1000000, // lower max count on the server anyway
-                    input: value.text.trim(),
+                    limit: 15,


The limit is 400 on the server. The autocomplete tab doesn't need to be that long. Moreover, if the maximum API limit on the server was increased, nothing would limit the number of items in the suggestions.

Therefore your answer is: "15 is the best compromise according to me". Please add a comment about it.

Yes, I believe that it is the best compromise, it is unnecessary to have more that 15 suggestions as approximately 5 can be seen at once on screen without scrolling.

15 sounds reasonable, but yes a comment would be good 👍🏼

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

monsieurtanuki · 2023-02-28T07:57:29Z

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

-                        (DefaultTextStyle.of(context).style.fontSize ?? 0) -
-                        // Elevation
-                        4.0,
+                    maxOptionsHeight: screenHeight/3,


Just hoping it works.

You can see the result on the third screenshot

monsieurtanuki · 2023-02-28T07:59:22Z

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

  }

+  /// Returns the packaging subtitle from the controllers
+String getSubTitle() {
+    return controllerRecycling.text;


I moved the packaging recycling info to the subtitle of the widget for readability (cf first screenshot). This function generate this subtitle.

stephanegigandet · 2023-02-28T11:15:02Z

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

@@ -40,19 +40,16 @@ class SimpleInputTextField extends StatelessWidget {
                optionsBuilder: (final TextEditingValue value) async {
                  final String input = value.text.trim();

-                  if (input.isEmpty) {


This will be great for packaging shapes and packaging materials, in order to have the most popular suggestions available without having to type the first letter. But we probably should not do it for other tagType values than packaging_shapes and packaging_materials, as it probably would be confusing for things like categories and labels. Especially as currently we return suggestions in alphabetic order for them.

Also addressed by @monsieurtanuki in #3750

The solution provided in #3750 is indeed better, I removed mine from this PR.

stephanegigandet · 2023-02-28T11:27:51Z

Hi @thomas-algo, thanks a lot for this PR. This will definitely ease a lot the work to enter packaging data.

As @monsieurtanuki suggests, it's best to first file issues about what you intend to change, so that others can comment, make suggestions etc. I'll file the corresponding issues for this PR.

@monsieurtanuki

@monsieurtanuki as addressed the issue in openfoodfacts#3750 without disturbing old default behavior of the widget.

M123-dev · 2023-02-28T18:12:00Z

Phew, that were a lot of comments in a really short time.

The automated tests are currently failing due to file formatting, please run flutter format . in packages/smooth_app
This will fix them :D

…& format

This variables where previously used in the formula of maxOptionsHeight

codecov-commenter · 2023-02-28T18:42:53Z

Codecov Report

Merging #3744 (36e226b) into develop (c281395) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #3744      +/-   ##
===========================================
- Coverage    10.41%   10.40%   -0.02%     
===========================================
  Files          272      272              
  Lines        13824    13842      +18     
===========================================
  Hits          1440     1440              
- Misses       12384    12402      +18

Impacted Files	Coverage Δ
...oth_app/lib/pages/product/edit_new_packagings.dart	`0.00% <0.00%> (ø)`
...b/pages/product/edit_new_packagings_component.dart	`0.00% <0.00%> (ø)`
.../lib/pages/product/edit_new_packagings_helper.dart	`0.00% <0.00%> (ø)`
...mooth_app/lib/pages/product/simple_input_page.dart	`0.00% <0.00%> (ø)`
...app/lib/pages/product/simple_input_text_field.dart	`0.00% <0.00%> (ø)`

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

stephanegigandet

Looks good to me. Can we merge? cc @monsieurtanuki @M123-dev

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

…er.dart

monsieurtanuki · 2023-03-02T18:19:08Z

I cannot say that I got all my answers but I see nothing too problematic in this PR.
For the record, in dart (as in many languages) the boolean "AND" operator is &&, not &.

thomas-algo added 2 commits February 27, 2023 22:11

Updated the preview of the package in the 'Packaging components ' tab.

d8aed2f

Preview is now clearer and more coherent with what is shown on the product page.

thomas-algo requested a review from a team as a code owner February 27, 2023 22:04

github-actions bot assigned thomas-algo Feb 27, 2023

github-actions bot added ✏️ Editing - Packaging input Related to the structured input of food packaging. 🥫 Product page labels Feb 27, 2023

Merge branch 'develop' into edit_packaging

234b88a

monsieurtanuki reviewed Feb 28, 2023

View reviewed changes

stephanegigandet reviewed Feb 28, 2023

View reviewed changes

stephanegigandet mentioned this pull request Feb 28, 2023

Height of the autocomplete suggestions can be too small #3747

Closed

github-actions bot added the ⭐ top pull request Top pull request. label Feb 28, 2023

github-actions bot mentioned this pull request Feb 28, 2023

👍 Top Issues Dashboard #3045

Open

stephanegigandet mentioned this pull request Feb 28, 2023

For packaging shapes and materials, display autocomplete suggestions without having to type the first character #3748

Closed

thomas-algo and others added 2 commits February 28, 2023 16:13

Removed Autocomplete now gives hints before anything is typed.

3ee60ab

@monsieurtanuki as addressed the issue in openfoodfacts#3750 without disturbing old default behavior of the widget.

Merge branch 'develop' into edit_packaging

34a0796

M123-dev linked an issue Feb 28, 2023 that may be closed by this pull request

Height of the autocomplete suggestions can be too small #3747

Closed

thomas-algo added 3 commits February 28, 2023 19:25

Comment explaining the limit of suggestions.

4cbea5f

Taking into account the case 'Material empty' and 'Weight not empty' …

e8081a0

…& format

Removing unused variables

4b7b140

This variables where previously used in the formula of maxOptionsHeight

monsieurtanuki mentioned this pull request Mar 1, 2023

feat: new localized label 'sep' as separator before colon (especially in French) #3753

Merged

Merge branch 'develop' into edit_packaging

36e226b

thomas-algo changed the title ~~feat: Clearer 'Packaging components' preview and autocomplete hint before user types~~ feat: Clearer 'Packaging components' preview and modified the maximum size of the autocomplete tab. Mar 2, 2023

stephanegigandet approved these changes Mar 2, 2023

View reviewed changes

monsieurtanuki reviewed Mar 2, 2023

View reviewed changes

packages/smooth_app/lib/pages/product/edit_new_packagings_helper.dart Outdated Show resolved Hide resolved

Update packages/smooth_app/lib/pages/product/edit_new_packagings_help…

ee90a53

…er.dart

monsieurtanuki merged commit 79e205d into openfoodfacts:develop Mar 2, 2023

github-actions bot mentioned this pull request Mar 2, 2023

chore(develop): release 4.6.0 #3741

Merged

thomas-algo deleted the edit_packaging branch March 5, 2023 21:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Clearer 'Packaging components' preview and modified the maximum size of the autocomplete tab. #3744

feat: Clearer 'Packaging components' preview and modified the maximum size of the autocomplete tab. #3744

thomas-algo commented Feb 27, 2023 •

edited

Loading

monsieurtanuki left a comment

monsieurtanuki Feb 28, 2023

thomas-algo Feb 28, 2023

monsieurtanuki Feb 28, 2023

thomas-algo Feb 28, 2023

stephanegigandet Feb 28, 2023

monsieurtanuki Feb 28, 2023

stephanegigandet Mar 1, 2023

monsieurtanuki Feb 28, 2023

thomas-algo Feb 28, 2023

monsieurtanuki Feb 28, 2023

thomas-algo Feb 28, 2023

M123-dev Feb 28, 2023

thomas-algo Feb 28, 2023

monsieurtanuki Feb 28, 2023

thomas-algo Feb 28, 2023

monsieurtanuki Feb 28, 2023

thomas-algo Feb 28, 2023

stephanegigandet Feb 28, 2023

stephanegigandet Feb 28, 2023

thomas-algo Feb 28, 2023

stephanegigandet commented Feb 28, 2023

M123-dev commented Feb 28, 2023

codecov-commenter commented Feb 28, 2023 •

edited

Loading

stephanegigandet left a comment

monsieurtanuki commented Mar 2, 2023

feat: Clearer 'Packaging components' preview and modified the maximum size of the autocomplete tab. #3744

feat: Clearer 'Packaging components' preview and modified the maximum size of the autocomplete tab. #3744

Conversation

thomas-algo commented Feb 27, 2023 • edited Loading

What

Screenshot

monsieurtanuki 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

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

stephanegigandet commented Feb 28, 2023

M123-dev commented Feb 28, 2023

codecov-commenter commented Feb 28, 2023 • edited Loading

Codecov Report

stephanegigandet left a comment

Choose a reason for hiding this comment

monsieurtanuki commented Mar 2, 2023

thomas-algo commented Feb 27, 2023 •

edited

Loading

codecov-commenter commented Feb 28, 2023 •

edited

Loading