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
Add introduction banner card into multichannel marketing page #37110
Conversation
…ture/34904-marketing-introduction-banner Conflicts: plugins/woocommerce-admin/client/marketing/hooks/index.ts plugins/woocommerce-admin/client/marketing/overview-multichannel/Channels/RecommendedChannels.tsx plugins/woocommerce-admin/client/marketing/overview-multichannel/MarketingOverviewMultichannel.tsx
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## trunk #37110 +/- ##
==========================================
- Coverage 45.8% 45.8% -0.0%
Complexity 17196 17196
==========================================
Files 429 429
Lines 64900 64909 +9
==========================================
- Hits 29703 29701 -2
- Misses 35197 35208 +11
|
Test Results SummaryCommit SHA: a2b02fc
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
…ture/34904-marketing-introduction-banner
Co-authored-by: Eason <eason.su.tw@gmail.com>
isOptionsUpdating will return true for any option update, not just our option here. This causes issue as shown in demo video in #37110 (comment). We can just depend on getOption. When we update an option, it will be updated immediately in wp.data store before making API request to update the option in database (see https://github.com/woocommerce/woocommerce/blob/c5564a15c11eae5b30de7ab6e37edbd973a29ebe/packages/js/data/src/options/actions.ts#L44).
@eason9487 , thanks so much for the review! I have addressed the code review comments. This is now ready for another round of review. 🙂 🙏 |
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.
Thanks for the update. The test results are correct and as expected.
However, I would still suggest addressing the almost duplicate images that can be eliminated in this PR. because it doesn't need much time to fix. Otherwise, it might take more effort to merge a known problem, open a follow-up, and then re-arrange improvement later. But it should be fine to proceed with the current implementation if there is not much time left for this project.
The remaining comments are minor suggestions and won't block this PR.
...s/woocommerce-admin/client/marketing/overview-multichannel/MarketingOverviewMultichannel.tsx
Outdated
Show resolved
Hide resolved
plugins/woocommerce-admin/client/marketing/overview-multichannel/Channels/Channels.tsx
Outdated
Show resolved
Hide resolved
...merce-admin/client/marketing/overview-multichannel/IntroductionBanner/IntroductionBanner.tsx
Outdated
Show resolved
Hide resolved
@eason9487 , thanks, I took your suggestions and made the changes, and your review approval has been dismissed. Could you do a review and approve again please? 🙏 |
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.
Thanks for the all work. Testing well, and LGTM.
All Submissions:
Changes proposed in this Pull Request:
Closes #34904. Previous draft PR is PR #36220.
In this PR, we add the introduction banner card at the top of the multichannel marketing page.
There are two "types" of introduction banner card: one with the "Create a campaign" and "Add channels" buttons, and one without.
Screenshot of introduction banner card without the "Create a campaign" and "Add channels" buttons:
Screenshot of introduction banner card with the "Create a campaign" and "Add channels" buttons:
Users can dismiss the card by clicking at the top right "X" button. We store the dismissed state by using a WP Option
woocommerce_marketing_overview_multichannel_banner_dismissed
. This approach is similar to the existingwoocommerce_marketing_overview_welcome_hidden
WP Option for the WelcomeCard in the existing Marketing page.The "Creata a campaign" button is displayed when there is at least one registered channel. Clicking it will display the "Create a campaign" modal.
The "Add channels" button is displayed when there is at least one registered channel and one recommended channel. When users click on the "Add channels" button, the page will scroll down to the "Add channels" button in the Channels card, allowing users to install a new channel.
How to test the changes in this Pull Request:
Pre-conditions:
/wp-admin/admin.php?page=wc-settings&tab=advanced§ion=features
woocommerce_marketing_overview_multichannel_banner_dismissed
in the database./wp-admin/admin.php?page=wc-admin&path=%2Fmarketing
.Other information:
pnpm --filter=<project> changelog add
?FOR PR REVIEWER ONLY: