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

fix: duplicate entries in search query #3289

Merged
merged 11 commits into from
Nov 9, 2022
Merged

fix: duplicate entries in search query #3289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
84271b9
fix : handle backtap for select products list
anshpathania7 Sep 15, 2022
c326fb2
Merge branch 'develop' of https://github.com/anshpathania7/smooth-app…
anshpathania7 Sep 15, 2022
c081d7e
chore : formatting
anshpathania7 Sep 15, 2022
60ca73a
chore : reverted git
anshpathania7 Sep 15, 2022
012de5f
chore
anshpathania7 Sep 15, 2022
b403a26
Merge branch 'develop' into develop
anshpathania7 Sep 15, 2022
29ad5be
Merge branch 'develop' of https://github.com/openfoodfacts/smooth-app…
anshpathania7 Sep 17, 2022
f1d6129
Merged from upstream branch
anshpathania7 Sep 17, 2022
381998f
Merge branch 'develop' of https://github.com/openfoodfacts/smooth-app…
anshpathania7 Nov 4, 2022
b9d175e
fix : trimming search query String
anshpathania7 Nov 9, 2022
f4407fc
Merge branch 'develop' into fix/product-query-search
anshpathania7 Nov 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,4 +253,4 @@ app_tzl.arb
app_val.arb
app_vec.arb
app_vls.arb
app_zea.arb
app_zea.arb
3 changes: 2 additions & 1 deletion packages/smooth_app/lib/pages/scan/search_page.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ void _performSearch(
String query, {
EditProductQueryCallback? editProductQueryCallback,
}) {
if (query.trim().isEmpty) {
query = query.trim();
Copy link
Member

Choose a reason for hiding this comment

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

Just curious , we aren't handling the "redbull" and "Redbull" as two different things here

Copy link
Contributor

Choose a reason for hiding this comment

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

@AshAman999 The fix was only about spaces, not lower / upper case.
"redbull" and "Redbull" will be considered as different.

Copy link
Member

Choose a reason for hiding this comment

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

So we're going to handle that case or leave it as it is for now ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AshAman999 I think "redbull" and "Redbull", both should be treated as separate keywords as on UI side they don't look same (also There could be some brands having capital keywords?).

In search history, "redbull" and " redbull " were treated as separate keywords but to user they would look same (because couldn't tell difference between whitespaces), hence duplicate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pops a question in my mind that should there be a some sort of suggestion in search box ? if user inputs something like "REdbUll" or "r ed bul l", then should there be suggestion for correct word?

Maybe this could be a feature?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah some kind of local autocomplete would make sense. Though as far as I can remember we only store 5 or so last searches atm.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you guys have suggestions beyond this PR feel free to open a new issue, so that we can merge this successful PR first.
Seeing several times the exact same text (e.g. "redbull") (modulo the spaces) was a bit problematic, and that's what the PR was about. Done.

if (query.isEmpty) {
return;
}

Expand Down