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

fix: 4957 - SVG icons are now correctly refreshed when their URL change #5133

Merged
merged 2 commits into from
Apr 8, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • The SVG icons were not refreshed when the underlying URL changed (e.g. after a product category change).
  • The fix is to use the SVG icon widget with a URL-related key. That will trigger the refresh of the StatefulWidget when relevant.

Fixes bug(s)

Impacted files

  • asset_cache_helper.dart: added a relevant key
  • svg_cache.dart: used the helper key so that flutter does refresh the widget for different URLs
  • svg_safe_network.dart: used a key; unrelated error message improvement

Impacted files:
* `asset_cache_helper.dart`: added a relevant `key`
* `svg_cache.dart`: used the helper `key` so that flutter does refresh the widget for different URLs
* `svg_safe_network.dart`: used a `key`; unrelated error message improvement
@monsieurtanuki monsieurtanuki requested a review from a team as a code owner March 26, 2024 11:17
@teolemon
Copy link
Member

This will ensure the Nutri-Score transition, am I right ?

@monsieurtanuki
Copy link
Contributor Author

This will ensure the Nutri-Score transition, am I right ?

Only in some very very rare cases.

For the new Nutri-Score transition rather have a look at #5100 - which is only a suggestion. I haven't seen anything new about the Nutri-Score transition in #4545.

For that specific PR it's about the way flutter works: for some kinds of widgets (StatefulWidgets) we sometimes need to add a relevant key.
If I see a nutriscore icon widget on a product page, then edit the product and then go back to the product page, the nutriscore icon widget will not be refreshed. That widget is loaded asynchronously and uses a State which somehow hides its data - including the URL. flutter will say "AFAIK this widget and its data haven't changed, so I don't refresh it". If the URL changes we change the key (in this PR), therefore flutter will say "oh there's a change, I have to refresh the widget".

@teolemon
Copy link
Member

teolemon commented Apr 8, 2024

@monsieurtanuki very clear, and very glad to get rid of that very annoying bug.
I think this one is going to have a material impact on new user/contributor engagement.

@teolemon teolemon added asset cache cached assets (SVG…) for the knowledge panels pull to refresh labels Apr 8, 2024
@monsieurtanuki monsieurtanuki merged commit 6ac205c into openfoodfacts:develop Apr 8, 2024
6 checks passed
@monsieurtanuki
Copy link
Contributor Author

Thank you @teolemon for your review!
It was a very interesting issue - flutter-wise - and that will indeed give more credibility and relevancy to the app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
asset cache cached assets (SVG…) for the knowledge panels 🚥 Nutri-Score pull to refresh
Development

Successfully merging this pull request may close these issues.

Product page partially updated
2 participants