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

feat: 3835 - country selector similar to language selector #3936

Merged
merged 2 commits into from
May 7, 2023

Conversation

monsieurtanuki
Copy link
Contributor

@monsieurtanuki monsieurtanuki commented May 6, 2023

What

  • The country selector is now similar to the language selector. In particular, the selected country is on top.
  • 2 minor visual bugs are also fixed: the "preferences" app bar title is now on 2 lines if needed, and there's no more overflow on the "welcome" page. In both cases, that impacted small screens- (cf. screenshots).
  • As a side effect, I refactored OnboardingPage. I was in a "Am I crazy or what?" mode for several days and tried everything to fix my issue. Was there a flaw in the provider? Spoiler alert: no there wasn't, I just coded something inappropriately that cleared the data I'd just entered...
  • We don't need the refactoring I mention, but the code is a bit clearer anyway.

Screenshots

country selector language selector
Screenshot_2023-05-06-18-44-16 Screenshot_2023-05-06-18-44-28
overflow bug (before) overflow bug (fixed)
Screenshot_2023-05-05-19-04-07 Screenshot_2023-05-05-19-08-52
title overflow bug (before) title overflow bug (fixed)
Screenshot_2023-05-05-19-15-52 Screenshot_2023-05-05-19-18-55

Fixes bug(s)

Files

  • consent_analytics_page.dart: minor refactoring
  • country_selector.dart: now using the preferences for initial country code; same UI as language selector
  • main.dart: minor refactoring
  • next_button.dart: minor refactoring
  • onboarding_flow_navigator.dart: refactoring around OnboardingPage
  • permissions_page.dart: minor refactoring
  • product_query.dart: minor refactoring
  • user_preferences.dart: minor refactoring
  • user_preferences_dev_mode.dart: minor refactoring
  • user_preferences_page.dart: minor UI fix (app bar title on 2 lines)
  • user_preferences_settings.dart: added the "selected language" parameter; minor refactoring
  • welcome_page.dart: fixed a visual overflow bug

Impacted files:
* `consent_analytics_page.dart`: minor refactoring
* `country_selector.dart`: now using the preferences for initial country code; same UI as language selector
* `main.dart`: minor refactoring
* `next_button.dart`: minor refactoring
* `onboarding_flow_navigator.dart`: refactoring around `OnboardingPage`
* `permissions_page.dart`: minor refactoring
* `product_query.dart`: minor refactoring
* `user_preferences.dart`: minor refactoring
* `user_preferences_dev_mode.dart`: minor refactoring
* `user_preferences_page.dart`: minor UI fix (app bar title on 2 lines)
* `user_preferences_settings.dart`: added the "selected language" parameter; minor refactoring
* `welcome_page.dart`: fixed a visual overflow bug
@monsieurtanuki monsieurtanuki requested a review from a team as a code owner May 6, 2023 17:09
@github-actions github-actions bot added 📈 Analytics We use Sentry and Matomo, with an opt-in system 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… labels May 6, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 6, 2023

Codecov Report

Merging #3936 (bb3e89c) into develop (69858ba) will increase coverage by 0.01%.
The diff coverage is 1.41%.

@@             Coverage Diff             @@
##           develop    #3936      +/-   ##
===========================================
+ Coverage    10.99%   11.01%   +0.01%     
===========================================
  Files          264      264              
  Lines        13051    13032      -19     
===========================================
  Hits          1435     1435              
+ Misses       11616    11597      -19     
Impacted Files Coverage Δ
...s/smooth_app/lib/data_models/user_preferences.dart 15.20% <0.00%> (-0.51%) ⬇️
packages/smooth_app/lib/main.dart 12.87% <0.00%> (ø)
...p/lib/pages/onboarding/consent_analytics_page.dart 0.00% <0.00%> (ø)
...oth_app/lib/pages/onboarding/country_selector.dart 0.87% <0.00%> (+0.87%) ⬆️
...s/smooth_app/lib/pages/onboarding/next_button.dart 4.54% <0.00%> (ø)
...ib/pages/onboarding/onboarding_flow_navigator.dart 0.00% <0.00%> (ø)
...oth_app/lib/pages/onboarding/permissions_page.dart 0.00% <0.00%> (ø)
.../smooth_app/lib/pages/onboarding/welcome_page.dart 0.00% <0.00%> (ø)
...b/pages/preferences/user_preferences_dev_mode.dart 0.00% <0.00%> (ø)
...b/pages/preferences/user_preferences_settings.dart 6.33% <0.00%> (-0.25%) ⬇️
... and 2 more

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

Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @monsieurtanuki, great to have it behave the same

);
case OnboardingPage.CONSENT_PAGE:
return _wrapWidgetInCustomBackNavigator(
context,
page,
ConsentAnalyticsPage(getBackgroundColor(page)),
ConsentAnalyticsPage(backgroundColor),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not from you but I am wondering why don't we just have the color inside the page

@monsieurtanuki monsieurtanuki merged commit fb164cb into openfoodfacts:develop May 7, 2023
@monsieurtanuki
Copy link
Contributor Author

Thank you @M123-dev for your review!

Regarding the colors being defined outside of the pages, I wouldn't mind if it was coded differently but after all, that forces developers to think about a different color for each onboarding page. Let's keep it that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📈 Analytics We use Sentry and Matomo, with an opt-in system 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score…
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Usability improvement for language & country selectors
3 participants