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

fix: #2729 - product query page - simplified top messages and buttons #2736

Merged
merged 2 commits into from
Aug 10, 2022

Conversation

monsieurtanuki
Copy link
Contributor

Impacted files:

  • product_query_page.dart: now displaying actions as app bar icon buttons (and large buttons for empty results); replaced the banner with a simple card; refactored
  • product_query_page_helper.dart: minor refactoring

What

  • The product query page had a dismissible MaterialBanner, but there was a bug and it sometimes reappeared after being dismissed
  • The solution was to remove the MaterialBanner altogether: no bug anymore of course ;)
  • It was replaced with AppBar buttons regarding actions ("world", "refresh"), and with a simple card regarding messages ("988 products")

Screenshot

type country search world search
pizza Capture d’écran 2022-08-06 à 08 23 54 Capture d’écran 2022-08-06 à 08 24 42
Swedish food Capture d’écran 2022-08-06 à 08 28 38 Capture d’écran 2022-08-06 à 08 29 03
unknown Capture d’écran 2022-08-06 à 08 29 40 Capture d’écran 2022-08-06 à 08 29 56

Fixes bug(s)

…s and buttons

Impacted files:
* `product_query_page.dart`: now displaying actions as app bar icon buttons (and large buttons for empty results); replaced the banner with a simple card; refactored
* `product_query_page_helper.dart`: minor refactoring
@codecov-commenter
Copy link

Codecov Report

Merging #2736 (a6a6839) into develop (2ea0da3) will decrease coverage by 1.57%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           develop   #2736      +/-   ##
==========================================
- Coverage     8.86%   7.28%   -1.58%     
==========================================
  Files          161     218      +57     
  Lines         6623   10569    +3946     
==========================================
+ Hits           587     770     +183     
- Misses        6036    9799    +3763     
Impacted Files Coverage Δ
...kages/smooth_app/lib/widgets/attribute_button.dart 0.00% <0.00%> (-92.00%) ⬇️
...s/smooth_app/lib/data_models/user_preferences.dart 8.73% <0.00%> (-23.57%) ⬇️
packages/smooth_app/lib/themes/smooth_theme.dart 62.26% <0.00%> (-20.72%) ⬇️
...p/lib/generic_lib/dialogs/smooth_alert_dialog.dart 15.11% <0.00%> (-19.10%) ⬇️
...mooth_app/lib/data_models/product_preferences.dart 21.68% <0.00%> (-9.75%) ⬇️
packages/smooth_app/lib/main.dart 14.65% <0.00%> (-3.24%) ⬇️
.../smooth_app/lib/pages/onboarding/welcome_page.dart 0.00% <0.00%> (-3.13%) ⬇️
.../smooth_app/lib/pages/onboarding/scan_example.dart 0.00% <0.00%> (-2.28%) ⬇️
...p/lib/pages/onboarding/consent_analytics_page.dart 0.00% <0.00%> (-1.57%) ⬇️
...ackages/smooth_app/lib/pages/scan/scan_header.dart 3.33% <0.00%> (-1.43%) ⬇️
... and 237 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@VaiTon
Copy link
Member

VaiTon commented Aug 6, 2022

Instead of printing "Showing result from the entire world" I would say in the message regarding the number of products: "20012 products found in France" or something similar and "129332 products found in the entire world". Not blocking though, just something that would make it more clear imo.

@monsieurtanuki
Copy link
Contributor Author

Instead of printing "Showing result from the entire world" I would say in the message regarding the number of products: "20012 products found in France" or something similar and "129332 products found in the entire world". Not blocking though, just something that would make it more clear imo.

I'm not against the idea, but:

  • I'm not sure we're able to give the name of the current country in the current language - perhaps a solution would be "20012 products found in the current country"?
  • anyway I coded that freewheel - all suggestions regarding the layout, colors, labels, icons are welcome, but perhaps in a separate issue
  • the point here was to PR a quick fix to a display bug, with a possible quick fix release

In short, I hoped that my PR could quickly be merged and a new release quickly... released (ideally this week-end), so that the display bug has no impact on our users.
In addition to that I'm not available in the next 2 days.

@teolemon Woud a fix release this week-end be realistic?

@teolemon
Copy link
Member

teolemon commented Aug 6, 2022

  • Can we combine the 3 sentences into one:
    • "Show 1234 results (France) from: 1 minute ago"
    • "Show 2468 results (World) from: less than 2 minute ago"
  • Search world >> "Extend your search to the world"
  • The buttons feel a little bit to wide vs text and cards (and their content)

@teolemon
Copy link
Member

teolemon commented Aug 6, 2022

  • @monsieurtanuki I released on Friday. We could do a weekend fix but the feedback we've had is actually on Multilingual products (Tracker) #2514, made more visible by the world mode.
  • eg: "I am quite new using the App and wanted to contribute a few products. Now since the last update it seems I can seek for international products. Today I tried to scan cereals by crownfield (Lidl) in Germany, type "Traube Nuss Müsli".
    The barcode is 2035 3889. Now the problem is this, in the app it is called Vollkornmüsli (whole corn cereals?) But but Traube Nuss Müsli (grape nut cereals?). All pictures are in Spanish or France, can't say for sure, but also showing the wrong product. I don't know to approach this now. Should I completely redo the page or could it be the other product is correct too? Hope you can help me with this and thanks for the great service!"

@monsieurtanuki
Copy link
Contributor Author

  • Can we combine the 3 sentences into one:
    • "Show 1234 results (France) from: 1 minute ago"
    • "Show 2468 results (World) from: less than 2 minute ago"

I would prefer not to:

  1. there are results that do not depend on the country (e.g. the products I edited). That would mean we would have to maintain labels for both "Show 1234 results (France) from: 1 minute ago" and "Show 1234 results from: 1 minute ago" (let alone the "1 result" / "2 results" variations in all languages)
  2. so far, the just downloaded results do not show a database timestamp, e.g. "2468 results (World)". That would mean we would have to maintain labels for both "2468 results (World)" and "2468 results (World) from: less than 2 minute ago" (let alone the "1 result" / "2 results" variations in all languages). Of course we could "invent" a database timestamp and display "2468 results (World) from: less than one minute ago"
  3. that said, we should indeed display the searched country - we don't for the moment.
  • Search world >> "Extend your search to the world"

Sure.

  • The buttons feel a little bit to wide vs text and cards (and their content)

Perhaps, but this is the Smoothie standard.
That said, as you want more text in the "search world" button, it can be less problematic for that button. And we can get rid of the "refresh" big button, as anyway both actions are available in the app bar. And "search world" is more interesting to show that "refresh" (where little can be hoped from an empty product list and a refresh).
This is what it would look like, button-wise:
Capture d’écran 2022-08-06 à 15 49 41

@monsieurtanuki monsieurtanuki linked an issue Aug 10, 2022 that may be closed by this pull request
Impacted files:
* `app_en.arb`: resized "world" labels - one shorter, one longer
* `app_fr.arb`: resized "world" labels - one shorter, one longer
* `product_query_page.dart`: no more display of "everything downloaded" final text; no more "refresh" large button; displays country name in the result summary
@monsieurtanuki
Copy link
Contributor Author

@VaiTon @teolemon New pushed version.

type country world
pizza Capture d’écran 2022-08-10 à 14 05 33 Capture d’écran 2022-08-10 à 14 05 48
Swedish food Capture d’écran 2022-08-10 à 14 07 23 Capture d’écran 2022-08-10 à 14 07 36
unknown Capture d’écran 2022-08-10 à 14 08 53 Capture d’écran 2022-08-10 à 14 09 19

@monsieurtanuki monsieurtanuki merged commit dee9be3 into openfoodfacts:develop Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Close button not working properly on Product search Page Pluralization issue in French
4 participants