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.

Sign up for GitHub

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

Jump to bottom

fix: filter while selecting countries #1200 #1728

Merged
merged 6 commits into from
May 2, 2022
Merged

fix: filter while selecting countries #1200 #1728

merged 6 commits into from
May 2, 2022

Conversation

AshAman999
Copy link
Member

What

  • Let the user search in the list of countries

Screenshot

  • Before
    IMG_20220501_171520

  • After

VID_20220501171417.mp4

Fixes bug(s)

Part of

@AshAman999 AshAman999 requested a review from a team as a code owner May 1, 2022 11:51
@codecov-commenter
Copy link

codecov-commenter commented May 1, 2022 •
edited
Loading

Codecov Report

Merging #1728 (d406e98) into develop (2ea0da3) will decrease coverage by 0.58%.
The diff coverage is 0.66%.

@@            Coverage Diff             @@
##           develop   #1728      +/-   ##
==========================================
- Coverage     8.86%   8.27%   -0.59%     
==========================================
  Files          161     165       +4     
  Lines         6623    7143     +520     
==========================================
+ Hits           587     591       +4     
- Misses        6036    6552     +516
Impacted Files Coverage Δ
...h_app/lib/cards/category_cards/abstract_cache.dart 0.00% <0.00%> (ø)
...p/lib/cards/category_cards/asset_cache_helper.dart 0.00% <0.00%> (ø)
...p/lib/cards/category_cards/raster_async_asset.dart 0.00% <0.00%> (ø)
...oth_app/lib/cards/category_cards/raster_cache.dart 0.00% <0.00%> (ø)
..._app/lib/cards/category_cards/svg_async_asset.dart 0.00% <0.00%> (ø)
...smooth_app/lib/cards/category_cards/svg_cache.dart 0.00% <0.00%> (ø)
...t_cards/knowledge_panels/knowledge_panel_card.dart 0.00% <0.00%> (ø)
...t_cards/knowledge_panels/knowledge_panel_page.dart 0.00% <0.00%> (ø)
...knowledge_panels/knowledge_panel_summary_card.dart 0.00% <ø> (ø)
...s/knowledge_panels/knowledge_panel_table_card.dart 0.00% <0.00%> (ø)
... and 52 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ee733a...d406e98. Read the comment docs.

@teolemon teolemon linked an issue May 1, 2022 that may be closed by this pull request
Addition of a filter while selecting countries #1200
Closed
@teolemon teolemon added 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… countries ⚙️ Settings labels May 1, 2022
M123-dev
M123-dev reviewed May 1, 2022
View reviewed changes
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.

Heyyy, looks really good @AshAman999, I added some minor comments, please have a look at them

packages/smooth_app/lib/pages/onboarding/country_selector.dart Outdated
return StatefulBuilder(
builder: (BuildContext context,
void Function(VoidCallback fn) setState) {
return AlertDialog(
Copy link
Member

Choose a reason for hiding this comment

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

What about using our SmoothAlertDialog

packages/smooth_app/lib/pages/onboarding/country_selector.dart Outdated
title: const Text('Choose your country'),
// TODO(aman): translations
content: SizedBox(
width: 300,
Copy link
Member

Choose a reason for hiding this comment

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

A responsive width would be good

packages/smooth_app/lib/pages/onboarding/country_selector.dart Outdated
width: 300,
child: Column(
children: <Widget>[
TextField(
Copy link
Member

Choose a reason for hiding this comment

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

I guess you could use our SmoothTextFormField (don't know if spelled corectly) here, then we can keep design changes continuous. But if for some reason it doesn't work out you can leave it like this

packages/smooth_app/lib/pages/onboarding/country_selector.dart Outdated
Navigator.of(context).pop();
},
);
}),
Copy link
Member

Choose a reason for hiding this comment

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

a comma here

@AshAman999
Copy link
Member Author

@M123-dev , did the changes as requested, looks like this for now

  • image

image

M123-dev
M123-dev approved these changes May 2, 2022
View reviewed changes
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.

Looks great, thanks @AshAman999

@M123-dev M123-dev merged commit 19e3560 into openfoodfacts:develop May 2, 2022
@github-actions github-actions bot mentioned this pull request May 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@M123-dev M123-dev M123-dev approved these changes

Assignees

@AshAman999 AshAman999

Labels
countries 🤗 Onboarding We need to onboard users on how the app works, but also on content like Nutri-Score, Eco-Score… ⚙️ Settings
Projects
🤳🥫 The Open Food Facts mobile app (Android & iOS)
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Addition of a filter while selecting countries
4 participants
@AshAman999 @codecov-commenter @M123-dev @teolemon