-
Notifications
You must be signed in to change notification settings - Fork 19
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
Restaurant_card #1311
Restaurant_card #1311
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1311 +/- ##
=======================================
Coverage 12% 12%
=======================================
Files 266 266
Lines 7194 7194
=======================================
Hits 802 802
Misses 6392 6392 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The design looks like the Figma one! Just an important thing below
class _RestaurantCardState extends State<RestaurantCard> { | ||
bool isFavorite = false; | ||
|
||
void _toggleFavorite() { | ||
setState(() { | ||
isFavorite = !isFavorite; | ||
}); | ||
} | ||
|
||
IconData? _getIconForMenuItemType(String type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extend a StatelessWidget
instead.
The isFavorite
state should be outside this widget and passed as a simple boolean argument. Also add the possibility to pass void function to run on favorite icon click.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okok! I'll fix that 😊
Color _getIconColorForMenuItemType(String type) { | ||
return Theme.of(context).colorScheme.primary; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks a little redundant. I would remove that and put the theme property directly on line 116.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense !!
Change the color of the text to black; |
@clarapbsousa can you take a look into this? |
Closed via #1398 |
Closes #1274
Created new Restaurante card, with all the options
Review checklist
whatsnew/whatsnew-pt-PT
changelog.md
with the changeEstablishments and meals
Favourite