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

Handle openfoodfacts.org links (Add deeplinking towards the product page) #920

Closed
15 of 17 tasks
Tracked by #1265 ...
teolemon opened this issue Jan 11, 2022 · 28 comments · Fixed by #3995
Closed
15 of 17 tasks
Tracked by #1265 ...

Handle openfoodfacts.org links (Add deeplinking towards the product page) #920

teolemon opened this issue Jan 11, 2022 · 28 comments · Fixed by #3995
Assignees
Labels
🔗 Deeplinking Ensure people browsing Open Food Facts on the mobile web have a bridge to the mobile apps distribution 🎨 Mockups available Some mockups are available for this issue. Please check everything is ok before starting coding. 🎯 P0 🥫 Product page
Milestone

Comments

@teolemon
Copy link
Member

teolemon commented Jan 11, 2022

Who for

  • Potential users
  • Current users

What

  • We have many product pages on the web
  • There are in-browser mechanisms to propose installing the native app for those who visit the page in a browser, or directly from Search Engines
  • Allow Smoothie to handle openfoodfacts.org links

Design doc with links to implementations in the classic apps

Why

  • Convenient for existing users
  • Chrome might propose app install as a result after a few visits.
  • Google Search might propose app install in some conditions.

Part of

Mockup

Code pointer

Status

At the moment we handle these urls:

  • Android
    • /product
    • /additive
    • /allergen
    • /category
    • /cgi/search.pl
    • /contributor
    • /country
    • /label
    • /manufacturing-place
    • /origin
    • /packaging
    • /state
    • /store
  • iOS:
    • ...

Note that we should support those URLs in plural (list of all existing additives), and in singular (list of products for a given additive)

Urls we'd like to support

Depends on

@M123-dev
Copy link
Member

M123-dev commented Jan 11, 2022

Note: Its not straight forward, we have to switch to either named routes or router 2.0

@teolemon teolemon added 🔗 Deeplinking Ensure people browsing Open Food Facts on the mobile web have a bridge to the mobile apps 🥫 Product page 🎯 P1 labels Jan 12, 2022
@teolemon teolemon changed the title Add deeplinking towards the product page Allow Smoothie to handle openfoodfacts.org links (Add deeplinking towards the product page) Jan 15, 2022
@teolemon teolemon added the 🎨 Mockups available Some mockups are available for this issue. Please check everything is ok before starting coding. label Mar 9, 2022
@teolemon teolemon added 🎯 P0 and removed 🎯 P1 labels Mar 16, 2022
@g123k
Copy link
Collaborator

g123k commented Mar 23, 2022

You're right @M123-dev, before handling deep links, we have to migrate the app named routes or Navigator 2.
I create a separate issue for migration.

@g123k g123k mentioned this issue Mar 23, 2022
2 tasks
@g123k
Copy link
Collaborator

g123k commented Mar 23, 2022

@teolemon Could you make the document publicly readable, please?

@teolemon
Copy link
Member Author

Fixed @g123k

@g123k
Copy link
Collaborator

g123k commented Mar 23, 2022

On Android, here are all the supported links:

  • /product
  • /additive
  • /allergen
  • /category
  • /cgi/search.pl
  • /contributor
  • /country
  • /label
  • /manufacturing-place
  • /origin
  • /packaging
  • /state
  • /store

@g123k g123k self-assigned this Mar 28, 2022
@teolemon teolemon added this to the V 1 milestone Mar 31, 2022
@teolemon teolemon modified the milestones: V1, V1.1 May 24, 2022
@VaiTon VaiTon changed the title Allow Smoothie to handle openfoodfacts.org links (Add deeplinking towards the product page) Handle openfoodfacts.org links (Add deeplinking towards the product page) Jul 24, 2022
@teolemon
Copy link
Member Author

We run Google AdGrants campaigns (free Adwords for NGOs), and the console says deeplinking will improve conversion rates:
image

@g123k
Copy link
Collaborator

g123k commented Aug 16, 2022

I would like this PR to be merged first: flutter/packages#2416
It will ease the migration.

Can we wait for it? Or should I start with a fork containing it?

@monsieurtanuki
Copy link
Contributor

Before any code is started, I would like to understand the goal, the strategy, and the impact of waiting/not waiting for flutter/packages#2416 to be merged. I'm old-fashioned, sorry about that ;)

As far as I understand, the goal is to allow deep-linking, and the most relevant way to do that is "named routes" or "router 2.0". Btw are both solution equivalent? Is that just the same concept with a different name?

Does that mean that

  • all current Smoothie pages are supposed to be accessed via a named route, e.g. /history? Of course there are parameters too I guess, like /product/1234
  • we have to connect "manually" Smoothie named routes and the website URLs via some regexp?
  • same implementation on Android and iOS?
  • is there a way we can code that gradually (in successive PRs), like convert to named routes first, then to router 2.0 (if needed/relevant), then deep linking on Android, then deep-linking on iOS?

@g123k
Copy link
Collaborator

g123k commented Aug 16, 2022

So let me explain: to achieve the goal of having deep-links, the best solution is to migrate to Navigator 2.0.
As we all know, Navigator 2.0 requires hundreds of lines of code and the use of external libraries is encouraged by Google itself. The most popular solution is from an ex-Google employee: go_router.

I had started a few weeks ago an implementation (never pushed, my fault), but I was soon stuck with codes like this:

var res = Navigator.of(context).push()
// Do something with res

Basically go_router doesn't allow receiving a result from a closing route. The only is by tricking and making the code totally awful and unreadable. And the PR I mentioned finally fixes that issue.

@monsieurtanuki
Copy link
Contributor

@g123k Thank you for your answer!

That's why I wanted to discuss the matter before any code writing: it looks like what causes problems is:

  • either the choice of Navigator 2.0
  • or the fact that, if we choose Navigator 2.0, we cannot pop/return a value (before the merge you indicated anyway)

My comments (a bit late in the evening to my standards) are:

  • perhaps there is one less problematic solution than Navigator 2.0 (I'm just speculating)
  • or perhaps we should rethink our current use of pop in order to return a value - either by not returning values and just notifying listeners, or by using a slightly different solution for specific pages that really need to return a value (that could be converted to dialogs?), a bit like what happened today with @simonbengtsson's WillPopScope where 2 specific solutions were implemented for the problematic 2 specific cases.

The first step would be to list the pop that return a value, and see what we could do with them.
Assuming that they can be converted to dialogs, we won't have to wait for the flutter merge.
And we can start migrating to Navigator 2.0, then (or simultaneously) deep-linking.

How does that sound?

@monsieurtanuki
Copy link
Contributor

Working on step one: transforming all await Navigator.push<MyObject?>( calls into await Navigator.push<void>( when the pop/return value is not used.

Current status:

% grep -r "await Navigator.push" *
cards/data_cards/image_upload_card.dart:          final bool? refreshed = await Navigator.push<bool>(
cards/product_cards/smooth_product_card_not_found.dart:                    final String? result = await Navigator.push<String>(
cards/product_cards/product_title_card.dart:                await Navigator.push<Product?>(
cards/product_cards/smooth_product_card_found.dart:            await Navigator.push<Widget>(
pages/scan/scan_header.dart:                      await Navigator.push<Widget>(
pages/scan/scan_product_card.dart:    await Navigator.push<Widget>(
pages/product/edit_product_page.dart:                      await Navigator.push<Product?>(
pages/product/edit_product_page.dart:                      final bool? refreshed = await Navigator.push<bool>(
pages/product/edit_product_page.dart:                      await Navigator.push<Product?>(
pages/product/edit_product_page.dart:                      await Navigator.push<Product?>(
pages/product/edit_product_page.dart:                      await Navigator.push<Product?>(
pages/product/edit_product_page.dart:        await Navigator.push<Product>(
pages/product/edit_product_page.dart:        await Navigator.push<Product>(
pages/product/product_image_gallery_view.dart:        final File? photoUploaded = await Navigator.push<File?>(
pages/product/add_ingredients_button.dart:          await Navigator.push<bool>(
pages/product/add_nutrition_button.dart:          await Navigator.push<Product>(
pages/product/common/product_list_page.dart:                    await Navigator.push<Widget>(
pages/product/summary_card.dart:              await Navigator.push<Product?>(
pages/product/summary_card.dart:                await Navigator.push<Widget>(
pages/product/new_product_page.dart:              await Navigator.push<void>(
pages/product/add_category_button.dart:          await Navigator.push<Product>(
pages/product/add_new_product_page.dart:          final File? finalPhoto = await Navigator.push<File?>(
pages/product/add_new_product_page.dart:          final Product? result = await Navigator.push<Product?>(
pages/product/add_new_product_page.dart:          final Product? result = await Navigator.push<Product?>(
pages/all_user_product_list_page.dart:                    await Navigator.push<void>(
pages/question_page.dart:                            await Navigator.push<Widget>(
pages/user_management/login_page.dart:                            final bool? registered = await Navigator.push<bool>(

@monsieurtanuki
Copy link
Contributor

Now I need your opinion guys.

We still have 8 cases where "pages" are pushed and a returned value is expected:

cards/data_cards/image_upload_card.dart:          final bool? refreshed = await Navigator.push<bool>(
cards/product_cards/smooth_product_card_not_found.dart:                    final String? result = await Navigator.push<String>(
pages/product/edit_product_page.dart:                      final bool? refreshed = await Navigator.push<bool>(
pages/product/product_image_gallery_view.dart:        final File? photoUploaded = await Navigator.push<File?>(
pages/product/add_new_product_page.dart:          final File? finalPhoto = await Navigator.push<File?>(
pages/product/add_new_product_page.dart:          final Product? result = await Navigator.push<Product?>(
pages/product/add_new_product_page.dart:          final Product? result = await Navigator.push<Product?>(
pages/user_management/login_page.dart:                            final bool? registered = await Navigator.push<bool>(

We need to code those 8 cases differently - as they are now we won't be able to switch to named routes (where no value is returned).
And if you have a look at those 8 cases, it's about a place where the user inputs data (e.g. editing a product or logging in). Which has nothing to do with deep-linking - unless we want to be able to connect directly to the "edit product picture 15" page, but from what I saw in the Android URL list this is not the case.

Therefore I suggest to switch those 8 cases from a page to a dialog or - perhaps better - to a bottom sheet.

MaterialPageRoute / fullscreenDialog: true (current mode) showModalBottomSheet / isScrollControlled: true (suggested mode)
Capture d’écran 2022-08-20 à 17 58 04 Capture d’écran 2022-08-20 à 18 07 07

The UI difference is minimal, and it takes 1 minute to switch from one mode to the other.

Are you guys OK with this idea of converting pages that return values, to bottom sheets?
After that we would be able to migrate to named routes, or Navigator 2.0.

@M123-dev
Copy link
Member

M123-dev commented Aug 20, 2022

AFAIK named routes and Router 2.0 implementations like go_router are completely different and for deeplinking we likely need router 2.0 so we can skip named routes. I just checked and it looks like we can (with what seems like replacing routes) indeed somewhat pop a value

https://stackoverflow.com/questions/71359432/flutter-go-router-how-to-return-result-to-previous-page


https://gorouter.dev/user-input

@monsieurtanuki
Copy link
Contributor

@M123-dev If you ask me, from what I read I prefer the dynamic named routes solution: it's easier to implement, and it allows deep-linking all the same.
That said, I barged into this issue 4 days ago because I assumed that there was something I could be helpful with, considering that pages returning values were a problem (cf. @g123k and flutter/packages#2416)
If actually that's not a problem that's good news, I'm done here.
Now you guys are all set for the implementation!

@teolemon
Copy link
Member Author

@monsieurtanuki monsieurtanuki removed their assignment Aug 25, 2022
@teolemon
Copy link
Member Author

teolemon commented Aug 25, 2022

FYI: flutter/packages#2416, they want to change stuff before they merge, so it's going to take a little more time

@M123-dev
Copy link
Member

Heyy just one little detail I've seen in the Medium article for flutter 3.3 go_router now supports async push.

The latest version (4.3) enables apps to redirect using asynchronous code, and includes other breaking changes described in the migration guide.

Can't say if that allows to pop values but still interesting to know

@teolemon
Copy link
Member Author

teolemon commented Sep 1, 2022

"It reduces the amount of code needed", dixit Edouard

@teolemon
Copy link
Member Author

@monsieurtanuki does the move to Flutter 3.7 change things on this one ?

@monsieurtanuki
Copy link
Contributor

@teolemon I think we were alright even before, as since months ago we no longer expect pop'ed values from our navigator calls. Except for minor cases that are now handled as "full screen dialog"s rather than as pages.

@g123k
Copy link
Collaborator

g123k commented Mar 31, 2023

If that's ok for you, it will take back this issue and implement it. I will start with the products.

@monsieurtanuki
Copy link
Contributor

@g123k That's perfect: I started 30 minutes ago with only preliminary works, and I reassign this issue to you.

For what it's worth:

% grep -r "builder: (BuildContext context) =>" *
cards/data_cards/image_upload_card.dart:          builder: (BuildContext context) => ProductImageGalleryView(
cards/product_cards/smooth_product_card_not_found.dart:                    builder: (BuildContext context) =>
cards/product_cards/smooth_product_card_found.dart:                  builder: (BuildContext context) => ProductPage(product),
knowledge_panel/knowledge_panels/knowledge_panel_card.dart:              builder: (BuildContext context) => SmoothBrightnessOverride(
pages/preferences/user_preferences_connect.dart:                  builder: (BuildContext context) => ScaffoldMessenger(
pages/preferences/user_preferences_connect.dart:                      builder: (BuildContext context) => Scaffold(
pages/preferences/user_preferences_dev_mode.dart:              builder: (BuildContext context) => SmoothAlertDialog(
pages/preferences/user_preferences_dev_mode.dart:                builder: (BuildContext context) => const OfflineDataPage(),
pages/preferences/user_preferences_dev_mode.dart:              builder: (BuildContext context) => const OfflineTaskPage(),
pages/preferences/user_preferences_dev_mode.dart:              builder: (BuildContext context) =>
pages/preferences/user_preferences_food.dart:      builder: (BuildContext context) => SmoothAlertDialog(
pages/preferences/user_preferences_account.dart:          builder: (BuildContext context) => const LoginPage(),
pages/preferences/user_preferences_account.dart:              builder: (BuildContext context) => const LoginPage(),
pages/preferences/user_preferences_account.dart:                builder: (BuildContext context) => AccountDeletionWebview(),
pages/preferences/user_preferences_account.dart:                  builder: (BuildContext context) => const LoginPage(),
pages/preferences/user_preferences_user_lists.dart:          builder: (BuildContext context) => const AllUserProductList(),
pages/preferences/abstract_user_preferences.dart:          builder: (BuildContext context) => UserPreferencesPage(
pages/scan/scan_header.dart:                          builder: (BuildContext context) =>
pages/scan/scan_product_card.dart:        builder: (BuildContext context) => ProductPage(product),
pages/scan/search_page.dart:        builder: (BuildContext context) => ProductPage(fetchedProduct.product!),
pages/product/nutrition_add_nutrient_button.dart:          builder: (BuildContext context) => StatefulBuilder(
pages/product/add_simple_input_button.dart:              builder: (BuildContext context) => SimpleInputPage(
pages/product/nutrition_page_loaded.dart:        builder: (BuildContext context) => NutritionPageLoaded(
pages/product/edit_product_page.dart:                      builder: (BuildContext context) =>
pages/product/edit_product_page.dart:                      builder: (BuildContext context) => EditOcrPage(
pages/product/edit_product_page.dart:                      builder: (BuildContext context) => EditNewPackagings(
pages/product/edit_product_page.dart:                      builder: (BuildContext context) => EditOcrPage(
pages/product/edit_product_page.dart:            builder: (BuildContext context) => SimpleInputPage(
pages/product/edit_product_page.dart:            builder: (BuildContext context) => SimpleInputPage.multiple(
pages/product/add_ocr_button.dart:              builder: (BuildContext context) => EditOcrPage(
pages/product/product_image_viewer.dart:        builder: (BuildContext context) => SmoothAlertDialog(
pages/product/product_image_viewer.dart:        builder: (BuildContext context) => UploadedImageGallery(
pages/product/product_image_viewer.dart:          builder: (BuildContext context) => CropPage(
pages/product/common/product_refresher.dart:      builder: (BuildContext context) => SmoothAlertDialog(
pages/product/common/product_refresher.dart:                builder: (BuildContext context) => const LoginPage(),
pages/product/common/product_dialog_helper.dart:                  builder: (BuildContext context) => AddNewProductPage(barcode),
pages/product/common/product_dialog_helper.dart:        builder: (BuildContext context) => SmoothAlertDialog(
pages/product/common/product_query_page.dart:                  builder: (BuildContext context) => PersonalizedRankingPage(
pages/product/common/product_query_page_helper.dart:        builder: (BuildContext context) => ProductQueryPage(
pages/product/summary_card.dart:                      builder: (BuildContext context) =>
pages/product/summary_card.dart:                        builder: (BuildContext context) =>
pages/product/summary_card.dart:        builder: (BuildContext context) => KnowledgePanelPage(
pages/product/new_product_page.dart:                    builder: (BuildContext context) =>
pages/product/new_product_page.dart:                  builder: (BuildContext context) =>
pages/product/add_new_product_page.dart:            builder: (BuildContext context) => AddBasicDetailsPage(
pages/image/uploaded_image_gallery.dart:                  builder: (BuildContext context) => CropPage(
pages/all_user_product_list_page.dart:                        builder: (BuildContext context) =>
pages/image_crop_page.dart:    builder: (BuildContext context) => StatefulBuilder(
pages/image_crop_page.dart:      builder: (BuildContext context) => CropPage(
pages/product_list_user_dialog_helper.dart:        builder: (BuildContext context) => _UserEmptyLists(daoProductList),
pages/product_list_user_dialog_helper.dart:      builder: (BuildContext context) => _UserLists(
pages/user_management/sign_up_page.dart:      builder: (BuildContext context) => SmoothAlertDialog(
pages/user_management/login_page.dart:                              builder: (BuildContext context) =>
pages/user_management/login_page.dart:                                builder: (BuildContext context) =>
pages/onboarding/onboarding_flow_navigator.dart:      builder: (BuildContext context) => getPageWidget(context, page),
pages/onboarding/onboarding_flow_navigator.dart:        builder: (BuildContext context) => SmoothScaffold(
smooth_category_picker_example.dart:                        builder: (BuildContext context) =>
widgets/tab_navigator.dart:          builder: (BuildContext context) => child,
% grep -r "builder: (final BuildContext context) =>" *
pages/offline_tasks_page.dart:                      builder: (final BuildContext context) =>
pages/preferences/user_preferences_dev_mode.dart:      builder: (final BuildContext context) => SmoothAlertDialog(
pages/product/portion_calculator.dart:      builder: (final BuildContext context) => SmoothAlertDialog(
pages/product_list_user_dialog_helper.dart:      builder: (final BuildContext context) => SmoothAlertDialog(
pages/product_list_user_dialog_helper.dart:      builder: (final BuildContext context) => SmoothAlertDialog(

@M123-dev
Copy link
Member

M123-dev commented Apr 1, 2023

What kind of solution do you plan to use @g123k a self build one or a package like go_router

@g123k
Copy link
Collaborator

g123k commented Apr 1, 2023

What kind of solution do you plan to use @g123k a self build one or a package like go_router

My idea is to use go_router, as it's the most commonly used package.

@M123-dev
Copy link
Member

M123-dev commented Apr 1, 2023

Sounds good 👍🏼

@g123k
Copy link
Collaborator

g123k commented Apr 10, 2023

The config file (assets links) is now deployed on the website
openfoodfacts/openfoodfacts-server#8306 (comment)

@g123k g123k linked a pull request May 22, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔗 Deeplinking Ensure people browsing Open Food Facts on the mobile web have a bridge to the mobile apps distribution 🎨 Mockups available Some mockups are available for this issue. Please check everything is ok before starting coding. 🎯 P0 🥫 Product page
Development

Successfully merging a pull request may close this issue.

4 participants