feat: Country selector redesign

[g123k]

Hi everyone!

I've totally redesigned the country picker to switch from a Dialog to a full screen.
In a near future, it will simplify the addition of the country detection (based on the user IP).

The screen has two modes:

• Auto-validate: a click on a country with close the screen + save it
• Manual validation: the user clicks on a country and has to validate it

In terms of code, the UI doesn't manage any data 🎉
Loading and reordering countries are done in an Isolate.
The UI is fully accessible + dark mode compatible.

There are also new Widgets from the POC (Top bar, bottom bar…).

Video with manual validation:
https://github.com/openfoodfacts/smooth-app/assets/246838/c0c901ec-aeae-49a4-821b-33b93e1a5f5b

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 1.21107% with 571 lines in your changes missing coverage. Please review.

Project coverage is 7.05%. Comparing base (4d9c7fc) to head (b0efd2e).
Report is 251 commits behind head on develop.

Files	Patch %	Lines
...preferences/country_selector/country_selector.dart	1.89%	207 Missing ⚠️
...es/country_selector/country_selector_provider.dart	2.67%	109 Missing ⚠️
...ages/smooth_app/lib/widgets/v2/smooth_topbar2.dart	0.00%	88 Missing ⚠️
.../smooth_app/lib/widgets/v2/smooth_buttons_bar.dart	0.00%	62 Missing ⚠️
...es/smooth_app/lib/widgets/v2/smooth_scaffold2.dart	0.00%	56 Missing ⚠️
...ckages/smooth_app/lib/helpers/provider_helper.dart	0.00%	28 Missing ⚠️
...ages/smooth_app/lib/pages/prices/emoji_helper.dart	0.00%	6 Missing ⚠️
...th_app/lib/pages/guides/helpers/guides_header.dart	0.00%	4 Missing ⚠️
.../smooth_app/lib/pages/onboarding/welcome_page.dart	0.00%	4 Missing ⚠️
packages/smooth_app/lib/themes/theme_provider.dart	0.00%	3 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #5483      +/-   ##
==========================================
- Coverage     9.54%   7.05%   -2.50%     
==========================================
  Files          325     395      +70     
  Lines        16411   20814    +4403     
==========================================
- Hits          1567    1469      -98     
- Misses       14844   19345    +4501     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[monsieurtanuki]

Hi @g123k!
Honestly I'm not able to fully review this PR.
Too complex for me, especially for a PR that doesn't solve an issue.
My main concern is about the maintenance of an open-source project using a not-that-popular language.
If needed I can give more details about my impressions.

[g123k]

Hi @g123k! Honestly I'm not able to fully review this PR. Too complex for me, especially for a PR that doesn't solve an issue. My main concern is about the maintenance of an open-source project using a not-that-popular language. If needed I can give more details about my impressions.

It is actually a requirement for an upcoming feature: being able to manage the country, from outside of this picker.
The example is the auto-detection of the country based on the user IP, which will be prompted on the homepage.

I acknowledge that this PR also contains some V2 widgets, but once again, we will need them.

My only concern is about your statement about a "not-that-popular language". Could you elaborate a bit more?
What's done here is just a ValueNotifier with classes representing the values.
Also the use of Consumer is recommended in the Flutter docs:
https://docs.flutter.dev/data-and-backend/state-mgmt/simple

[monsieurtanuki]

@g123k About my "not-that-popular language" statement: I mean that flutter is not as widespread as java or python, where we can find tons of developers and tons of experts.
The more flutter specific we code, the less newcomers are likely to be able to contribute - or they just lousily copy/paste from stackoverflow.

More specifically, regarding the current PR:

• you decided to move the country_selector.dart file and to transform it into a 600-line file - not nice for a reviewer, as the file is very big and detecting the changes is now very acrobatic
• you introduce an autoValidate option that we don't seem to need
• you introduce a UI discrepancy between our selectors, e.g. language selector and country selector
• you introduce an Isolate for just sorting a list of 200 items

I don't say that the video is not good looking.
I'm just saying that I don't have the brain bandwidth to review a code that I think makes the code harder to maintain and whose added value is not obvious.
I believe you could have coded in a much more simple and elegant way. Here I assume that you coded like you were teaching how to use isolates and providers.

[g123k]

Hi @monsieurtanuki, my answers are just below 🙂

@g123k About my "not-that-popular language" statement: I mean that flutter is not as widespread as java or python, where we can find tons of developers and tons of experts.
Mmm… Flutter is the leading cross-platform framework
https://www.jetbrains.com/lp/devecosystem-2023/development/#mobile_crossplatform_frmwrk_two_years
In terms of syntax, Dart is pretty much similar to the “old” Java.
The only difference is on the Flutter framework itself, where we have to careful manage the BuildContext and all that stuff.
And honestly, how many contributors do we have today?

The more flutter specific we code, the less newcomers are likely to be able to contribute - or they just lousily copy/paste >from stackoverflow.

On this particular point, I’m sorry, but I don’t agree with you.
Using Provider is well documented on the official website: https://docs.flutter.dev/data-and-backend/state-mgmt/simple, where we have an example pretty much similar to mine.
The point of this PR is not to redesign to Dialog, but to prepare upcoming features.
For example, we want to change the language from the tagline, hence the fact that the Widget should absolutely not handle any logic.

More specifically, regarding the current PR:

• you decided to move the country_selector.dart file and to transform it into a 600-line file - not nice for a reviewer, as the file is very big and detecting the changes is now very acrobatic

One thing the app is clearly missing today is splitting Widgets into sub-widgets. This is a recommended pattern to gain performance (@see the Element and Render trees)
Each part of the UI is split: the button, the Dialog, an item in the list…

• you introduce an autoValidate option that we don't seem to need

Similar to my point just above: we need something more generic

• you introduce a UI discrepancy between our selectors, e.g. language selector and country selector

True, but my point is to use this PR to introduce V2 widgets.
So this is just the first step, and the entire app will follow.

• you introduce an Isolate for just sorting a list of 200 items

This computation is done on a Screen with multiple sub-widgets and once again, it can be inserted anywhere in the app (e.g.: the tagline, with the camera on top of it)

I don't say that the video is not good looking. I'm just saying that I don't have the brain bandwidth to review a code that I think makes the code harder to maintain and whose added value is not obvious.

Added values are:

• The UI and the logic are split
• We have clear states (loading, error…)
• There are optimizations (Isolate, subwidgets…)

I believe you could have coded in a much more simple and elegant way. Here I assume that you coded like you were teaching how to use isolates and providers.

To be honest, if it was my code (= personal repo), it would be just 70-80% of what I’d like to do. I’ve already done it in many other applications, and the other developers have never criticized it.
This pattern explained just before is simply what we need to deploy in the entire app: Flutter good practices.
I can understand that it’s totally different from what the app is today.
But the POC follows this pattern.

In general, I can add to the wiki or somewhere else a list of good practices for newcomers if that can help to clarify things.

I would like not to loose too much time on arguing. I want the application to adhere to the recommended Flutter patterns and to enhance maintainability in the long run. Surely, there are things that can be modified, but not the general pattern.

[monsieurtanuki]

Hi @g123k!
I understand your point of view, but it's not mine at all.
Life is short, and I cannot spend time re-coding things just for the sake of the latest coding standards. Nor reviewing related code.
I'm done with this PR.
Have a nice day!

[teolemon]

+1 on my side @g123k, you're the one Smoothie-side calling the shots.

• We need to get the logic of settings separated and streghtened, so that we can use it in context throughout the app.
• We need to get moving on delivering the PoC, which does include more Flutter standard practises.

[g123k]

OK, thank you both for your feedback 👍
I've spent a lot of time checking to make sure everything works.
So I'll take the liberty to merge this PR.