Skip to content
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: Product page redesign #6262

Merged
merged 14 commits into from
Feb 4, 2025

Conversation

PrimaelQuemerais
Copy link
Member

@PrimaelQuemerais PrimaelQuemerais commented Jan 21, 2025

What

This PR introduces a new the layout for the product page (WIP)

Knowledge panels are now grouped in tabs which can be toggled and reordered.

Remaining tasks

  • Move reorder button to the end of the TabBar (instead of "For me" tab)
  • Save customizations locally (suggestions are welcome, how to conciliate hardcoded tabs and knowledge panel tabs)
  • Try to merge new generic reorder_bottom_sheet with ProductFooterSettings sheet
  • Implement UI elements from POC such as TabBar scroll hints, or border radius on selected tab indicator
  • Add ability to hide tabs

Screenshot

Simulator Screenshot - iPhone 16 Pro Max - 2025-01-21 at 15 40 42

Part of

@teolemon
Copy link
Member

tiny merge conflict @PrimaelQuemerais

Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

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

I will do a second pass on your code, but here's a few suggestions

@codecov-commenter
Copy link

codecov-commenter commented Jan 22, 2025

Codecov Report

Attention: Patch coverage is 0% with 250 lines in your changes missing coverage. Please review.

Project coverage is 5.91%. Comparing base (4d9c7fc) to head (59cbd30).
Report is 715 commits behind head on develop.

Files with missing lines Patch % Lines
...product/product_page/header/product_page_tabs.dart 0.00% 95 Missing ⚠️
...duct/product_page/header/reorder_bottom_sheet.dart 0.00% 71 Missing ⚠️
...b/pages/product/product_page/new_product_page.dart 0.00% 66 Missing ⚠️
...b/pages/preferences/user_preferences_dev_mode.dart 0.00% 6 Missing ⚠️
.../lib/data_models/preferences/user_preferences.dart 0.00% 5 Missing ⚠️
...ges/smooth_app/lib/pages/product/summary_card.dart 0.00% 4 Missing ⚠️
...generic_lib/bottom_sheets/smooth_bottom_sheet.dart 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #6262      +/-   ##
==========================================
- Coverage     9.54%   5.91%   -3.64%     
==========================================
  Files          325     474     +149     
  Lines        16411   28169   +11758     
==========================================
+ Hits          1567    1666      +99     
- Misses       14844   26503   +11659     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@teolemon
Copy link
Member

@PrimaelQuemerais @g123k good to merge ?

@g123k
Copy link
Collaborator

g123k commented Jan 22, 2025

@PrimaelQuemerais @g123k good to merge ?

Not yet

@teolemon teolemon requested a review from g123k January 24, 2025 11:55
value: tab,
);
}).toList(),
onTabChanged: (ProductPageTab tab) {},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing here?

@g123k
Copy link
Collaborator

g123k commented Jan 24, 2025

@teolemon I would hide tabs behind a dev mode.
We need more tabs for a general release

@teolemon
Copy link
Member

I count at least 3 to 4 tabs, health, environnement, report, possibly for me. Is it not enough ?

@g123k
Copy link
Collaborator

g123k commented Jan 24, 2025

I count at least 3 to 4 tabs, health, environnement, report, possibly for me. Is it not enough ?

If we want to publish a new release before the upcoming French TV show, the current page is OK (+ new icons).
This product page with tabs (which is not new, as stated in the title) needs to have "real" tabs content and not something like "Ok, it's just like before, but now with tabs". It's a terrible approach.

My goal for the past few weeks has been to have a better product for end users, and please don't revert to that state.
Tabs are OK, but only when they're fully ready.

@teolemon
Copy link
Member

teolemon commented Jan 25, 2025

We can put it in dev mode or open beta feature for a while.

@PrimaelQuemerais
Copy link
Member Author

I've implemented the comments from @g123k's review and added a toggle in dev mode settings to enable or disable the tab view.

@g123k
Copy link
Collaborator

g123k commented Jan 27, 2025

I've implemented the comments from @g123k's review and added a toggle in dev mode settings to enable or disable the tab view.

Ok, I will check that.
You have a formatting issue: https://github.com/openfoodfacts/smooth-app/actions/runs/12985779999/job/36211292979?pr=6262

@g123k
Copy link
Collaborator

g123k commented Jan 27, 2025

@teolemon In case you have some feedback:
https://github.com/user-attachments/assets/69a43113-7d7f-46a0-87c6-8f5617f7df62

@PrimaelQuemerais I've tested the app and there are 3 regressions:

  • White status bar
    IMG_7DCB4D313DCB-1

  • The background shouldn't be white for the "non-tabs" version

  • The Pull to refresh is weirdly placed: it should be on top of the screen
    IMG_2A57D1D77AF6-1

@teolemon
Copy link
Member

teolemon commented Jan 27, 2025

3 different top level grammars in 3 different tabs

@teolemon
Copy link
Member

teolemon commented Feb 3, 2025

2 merge conflicts @PrimaelQuemerais

@PrimaelQuemerais PrimaelQuemerais merged commit 90fb5d4 into openfoodfacts:develop Feb 4, 2025
8 checks passed
@g123k
Copy link
Collaborator

g123k commented Feb 4, 2025

Be careful, when you auto-validate your PR.
Furthermore, we squash PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants