-
-
Notifications
You must be signed in to change notification settings - Fork 287
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: Wrap list names on product page #3647
feat: Wrap list names on product page #3647
Conversation
Hi @prathamsoni11! |
|
You seem to be very lucky with your name choices: they all seem to fit magically on the first rows. |
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.
👍
@prathamsoni11 Sorry about that, but we already tried with smaller buttons in |
@monsieurtanuki like this I guess |
I mean it's exactly what I had imagined, so I don't have a critical view on it, if I have a lot of lists (with mostly small names) everything is better than tanking the whole width for each. When I understand your concerns, you mean cases like this (amazing ms paint image): Where the line break will cause irregularities, looking at the docs of a package which provides a WrapSuper widget with more features it looks like the Wrap widget already distributes widgets so that they fit in the best way possible, not just in the provided order. Even if not we could use the beforementioned package with |
@prathamsoni11 the ci is complaining about a unused import https://github.com/openfoodfacts/smooth-app/actions/runs/4021677946/jobs/6910758389#step:7:1 After that we can merge |
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 good, thanks @prathamsoni11
But the tests are still complaining
https://github.com/openfoodfacts/smooth-app/actions/runs/4090853782/jobs/7054555673#step:8:1
I honestly don't know why. Did you test with flutter 3.0.7, or with Wrap instead of WrapSuper
No i have to test with 3.0.7. After testing I’ll update you |
Oh, I mean 3.7.0 😅 |
Checked with 3.7.0 working fine and no issues |
with with |
@prathamsoni11 That looks better with |
@prathamsoni11 We've recently upgraded to % flutter upgrade
Flutter is already up to date on channel stable
Flutter 3.7.1 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 7048ed95a5 (3 days ago) • 2023-02-01 09:07:31 -0800
Engine • revision 800594f1f4
Tools • Dart 2.19.1 • DevTools 2.20.1 |
Codecov Report
@@ Coverage Diff @@
## develop #3647 +/- ##
==========================================
- Coverage 9.45% 9.45% -0.01%
==========================================
Files 269 269
Lines 13571 13576 +5
==========================================
Hits 1283 1283
- Misses 12288 12293 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Thank you @prathamsoni11! |
Your welcome @monsieurtanuki 😁 |
What
Screenshot
Part of