feat: apply price card glow up to top locations and top products (#6846)#7462
feat: apply price card glow up to top locations and top products (#6846)#7462VisheshKamble wants to merge 2 commits intoopenfoodfacts:developfrom
Conversation
github-project-automation
Bot
moved this to 💬 To discuss and validate
in 🤳🥫 The Open Food Facts mobile app (Android & iOS)
github-project-automation
Bot
moved this from 💬 To discuss and validate
to 🎊 Done
in 🤳🥫 The Open Food Facts mobile app (Android & iOS)
github-project-automation
Bot
moved this from 🎊 Done
to 🤔 To reproduce
in 🤳🥫 The Open Food Facts mobile app (Android & iOS)
|
@teolemon Hi! I’ve completed the implementation for this PR and updated the description with the required details, including issue linking. |
|
can you add a little screenshot of how it looks ? @VisheshKamble |
There was a problem hiding this comment.
Pull request overview
Introduces a shared “top-list” card container to reuse the updated Prices card styling across multiple top-list pages.
Changes:
- Added
PriceTopListCardas a shared container widget for Prices top-list items. - Migrated Top Products list items to use
PriceTopListCard. - Migrated Top Locations list items to use
PriceTopListCardand adjusted internal layout.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/smooth_app/lib/pages/prices/prices_products_page.dart | Replaces per-item SmoothCard + InkWell with the shared PriceTopListCard wrapper. |
| packages/smooth_app/lib/pages/prices/prices_locations_page.dart | Wraps location items in PriceTopListCard and restructures content into Column + Wrap. |
| packages/smooth_app/lib/pages/prices/price_top_list_card.dart | New reusable card widget encapsulating margin/padding/elevation/theme for top-list items. |
| return PriceTopListCard( | ||
| onTap: () async => PriceLocationWidget.showLocationPrices( | ||
| locationId: item.locationId, | ||
| context: context, | ||
| ), | ||
| child: Column( |
There was a problem hiding this comment.
PR description says existing behavior/actions are preserved, but this change makes the whole location card tappable and opens the location prices. Previously only the PriceCountWidget had navigation; please either remove the card-level onTap or update the PR description/UX to reflect the new interaction (and ensure it’s desired).
| PriceButton( | ||
| onPressed: () {}, | ||
| title: '${item.userCount}', | ||
| iconData: PriceButton.userIconData, | ||
| tooltip: item.userCount == null | ||
| ? null | ||
| : appLocalizations.prices_button_count_user( | ||
| item.userCount!, | ||
| ), | ||
| ), | ||
| PriceButton( | ||
| onPressed: () {}, | ||
| title: '${item.productCount}', | ||
| iconData: PriceButton.productIconData, | ||
| tooltip: item.productCount == null | ||
| ? null | ||
| : appLocalizations.prices_button_count_product( | ||
| item.productCount!, | ||
| ), | ||
| ), | ||
| PriceButton( | ||
| onPressed: () {}, | ||
| title: '${item.proofCount}', | ||
| iconData: PriceButton.proofIconData, | ||
| tooltip: item.proofCount == null | ||
| ? null | ||
| : appLocalizations.prices_button_count_proof( | ||
| item.proofCount!, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Because the card now has an onTap, the PriceButton widgets with onPressed: () {} become dead-ends: tapping them does nothing and also prevents the card tap from firing. If these buttons are meant to be informational only, consider passing onPressed: null (disabled) so taps fall through to the card; otherwise, provide real actions for them.
| start: 8.0, | ||
| end: 8.0, |
There was a problem hiding this comment.
Avoid the magic number 8.0 for horizontal margins; design_constants.dart already defines SMALL_SPACE = 8.0. Using the shared constant keeps spacing consistent and makes future spacing tweaks easier.
| start: 8.0, | |
| end: 8.0, | |
| start: SMALL_SPACE, | |
| end: SMALL_SPACE, |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #7462 +/- ##
==========================================
- Coverage 9.54% 9.09% -0.45%
==========================================
Files 325 626 +301
Lines 16411 36650 +20239
==========================================
+ Hits 1567 3335 +1768
- Misses 14844 33315 +18471 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This Pull Request will only be accepted if:
Fixes,Closes, orResolves)What
Added a shared Prices top-list card container to reuse the updated price card look and feel.
Applied the shared container to Top Products items.
Applied the shared container to Top Locations items.
Kept existing behavior and actions (navigation, counters, and action buttons) intact.
Preserved product image placeholder behavior for items without images/barcodes.
Screenshot or video
Fixes bug(s)
Closes #6846
Part of