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: Allow to share and view lists on web #3757

Merged
merged 6 commits into from
Mar 9, 2023
Merged

Conversation

M123-dev
Copy link
Member

@M123-dev M123-dev commented Mar 8, 2023

What

Allow to share lists with friends, as well as opening it on the web

fixes: #2631
related to: #2627

This will not work with large list (URLs will break when exceeding 2048 characters) but thats the best we can offer right now until we have a better method

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Hi @M123-dev!

I cannot say that it does not work, but there are still pending questions:

  • the URL has a limited length - there will be a limited number of products
  • potential localization issue - should we add the language, the country?

@monsieurtanuki
Copy link
Contributor

@M123-dev The localization is problematic, because if I land on an EN page and change the country to France, I land on the French home page (I mean, with the "normal" French products, not the shared products).

@teolemon
Copy link
Member

teolemon commented Mar 9, 2023

Can you log how often list sharing is used in Matomo ?

@stephanegigandet
Copy link
Contributor

@github-actions github-actions bot added the 📈 Analytics We use Sentry and Matomo, with an opt-in system label Mar 9, 2023

// TODO(m123): Move this to off-dart
Uri shareProductList(List<String> barcodes) {
final StringBuffer buffer = StringBuffer();
Copy link
Contributor

Choose a reason for hiding this comment

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

why not something like

final String commaSeparatedBarcodes = barcodes.join(',');

That's simpler and would get rid of the extra comma at the end.

@teolemon teolemon merged commit 777f54c into develop Mar 9, 2023
@teolemon teolemon deleted the feat-share-list branch March 9, 2023 17:05
@M123-dev
Copy link
Member Author

M123-dev commented Mar 9, 2023

I just added localization as well as some analytics for it.

I initially thought that .getUri already handles localization but it turns out it does not

@M123-dev M123-dev restored the feat-share-list branch March 11, 2023 09:38
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 🌐 l10n 🥫 Product page User lists
Development

Successfully merging this pull request may close these issues.

Allow to share product lists by URLs
4 participants