fix: Image extraction screen #3026
Conversation
…into image-extraction-screen
There was a problem hiding this comment.
Hi @Puja-Kumari-909!
Please have a look at my comments; nothing very hard to fix.
packages/smooth_app/lib/pages/product/edit_ingredients_page.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/edit_ingredients_page.dart
Outdated
Show resolved
Hide resolved
| children: <Widget>[ | ||
| const Icon( | ||
| Icons.image_not_supported, | ||
| size: 200, |
There was a problem hiding this comment.
Where does that 200 come from?
If I see your screenshots, there seems to be already a space / positioning in place (on both pictures the refresh/upload photo button is at the same place).
I suggest that you just fill the space (Expand?) with the icon instead of specifying a fixed size of 200.
That said, if 200 already appears somewhere else in the code (for positioning purposes), please make it a static const.
(not fan of the margin either, for the same reasons: why 0.25?)
There was a problem hiding this comment.
Hello @monsieurtanuki,
Thanks for the review. I changed most of the things you asked for.
I removed the 200, instead used the size object.height / 4. And also for 0.25 I used the same size object. Should be consistent for all screen size.
packages/smooth_app/lib/pages/product/edit_ingredients_page.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thank you @Puja-Kumari-909, and congratulations for your first PR here!
I'm a bit puzzled to see size.height * 0.25 and then 5 lines lower size.height / 4, but that's OK ;)
|
Congrats on your first PR here @Puja-Kumari-909 🥳 |
|
Thanks to all three of you @M123-dev @monsieurtanuki @AshAman999 for making my first time contribution smooth :) |
What
Screenshot(Before and After)
Fixes bug(s)