Skip to content

Add vegan and vegetarian fields and filters to the ingredient#1124

Open
rolandgeider wants to merge 4 commits intomasterfrom
feature/vegan-flags
Open

Add vegan and vegetarian fields and filters to the ingredient#1124
rolandgeider wants to merge 4 commits intomasterfrom
feature/vegan-flags

Conversation

@rolandgeider
Copy link
Member

@rolandgeider rolandgeider commented Feb 26, 2026

Related Issue(s)

Flutter part of wger-project/wger#2217

simulator_screenshot_96BC3019-9C23-4EC6-8254-282702AA65A5

Copy link

@mubashardev mubashardev left a comment

Choose a reason for hiding this comment

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

Overall
Great PR! The implementation for filtering by dietary preferences is clean, localization is handled well, and the added test coverage in ingredient_typeahead_test.dart is excellent.

I've left a few minor non-blocking suggestions below for your consideration. Nice work!

Minor Suggestions (Non-blocking):

  1. State Management Clarity (lib/widgets/nutrition/widgets.dart):
    In the StatefulBuilder's onChanged callbacks, the setState shadowing is functional but could be slightly clearer for readability:
onChanged: (val) {
  this.setState(() { 
     _isVegan = val; // Clearly updates the parent Widget state
  });
  setState(() {}); // Signals StatefulBuilder dialog to rebuild its UI
},
  1. Query Parameters (lib/providers/nutrition.dart):
    Currently passing 'True' (capitalized) for the query parameter. While Django REST Framework typically handles this fine, standard web serialization generally favors lowercase ('true') for booleans across the wire.

@rolandgeider
Copy link
Member Author

yes you are right, I've clean up a bit the code

While this has technically nothing to do with the vegan flags, we can refactor the
language handling while we're at it. This should make the behaviour more transparent
to the user and could be expanded in the future if needed (manually selecting other
languages, etc.).
# Conflicts:
#	lib/l10n/app_de.arb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants