-
-
Notifications
You must be signed in to change notification settings - Fork 305
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
feat: Product page redesign #6262
Conversation
tiny merge conflict @PrimaelQuemerais |
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.
I will do a second pass on your code, but here's a few suggestions
packages/smooth_app/lib/pages/product/product_page/header/product_tabs.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/product_tabs.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/product_tabs.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/product_tabs.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
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. |
@PrimaelQuemerais @g123k good to merge ? |
Not yet |
packages/smooth_app/lib/pages/product/product_page/header/product_page_tabs.dart
Outdated
Show resolved
Hide resolved
value: tab, | ||
); | ||
}).toList(), | ||
onTabChanged: (ProductPageTab tab) {}, |
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.
Nothing here?
packages/smooth_app/lib/pages/product/product_page/header/product_page_tabs.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/product_page_tabs.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/product_page_tabs.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/header/reorder_bottom_sheet.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/new_product_page.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/product/product_page/new_product_page.dart
Outdated
Show resolved
Hide resolved
@teolemon I would hide tabs behind a dev mode. |
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). 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. |
We can put it in dev mode or open beta feature for a while. |
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. |
@teolemon In case you have some feedback: @PrimaelQuemerais I've tested the app and there are 3 regressions: |
2 merge conflicts @PrimaelQuemerais |
Be careful, when you auto-validate your PR. |
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
Screenshot
Part of