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.

Sign up for GitHub

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

Jump to bottom

Add "Create a new campaign" modal #36130

Closed
wants to merge 14 commits into from
Closed

Add "Create a new campaign" modal #36130

wants to merge 14 commits into from

Conversation

ecgan
Copy link
Member

@ecgan ecgan commented Dec 21, 2022 •
edited

All Submissions:

Changes proposed in this Pull Request:

Part of #34909.

This PR implements the "Create a new campaign" modal.

image

image

The implemented modal here is different from the figma designs in #34909. Specifically, there are no borders within the modal component. This is to be more consistent with the WordPress Modal component which no longer has border under the modal title since PR WordPress/gutenberg#40781.

How to test the changes in this Pull Request:

Pre-conditions: Enable WooCommerce Multichannel Marketing experience in WooCommerce Settings > Advanced > Features page: /wp-admin/admin.php?page=wc-settings&tab=advanced&section=features

  1. Go to WooCommerce Marketing page: /wp-admin/admin.php?page=wc-admin&path=%2Fmarketing
  2. Click on the "Create new campaign" button.
  3. You should see the modal like the screenshot above.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@ecgan
Add CampaignsCardHeader component with modal.
0866135
@ecgan
Remove unneeded imports in RecommendedChannelsList.
3399874
@ecgan
Move RecommendedChannelsList to shared components directory.
6de95c7
@ecgan
Move useRecommendedChannels to shared hooks directory.
5f50e63
@ecgan
Display recommended channels in create campaign modal.
17f2072
@ecgan
Hide the recommended channels section if it is empty.
24d59f7
@ecgan
Display new campaign types in create campaign modal.
6b3fbdf
@ecgan
Make create campaign button open in new tab depending on createUrl or…
5caffb6 
…igin.
@ecgan
Code refactor with CreateNewCampaignModal.
894b77d
@ecgan
Better code clarity with woocommerce-marketing-campaigns-table class …
321145e 
…name.
@ecgan
Use perPage variable instead of duplicated number.
a3c8680
@ecgan
Code refactor for Campaigns component, remove unneeded CampaignsCardH…
07894d4 
…eader.
@ecgan ecgan added the focus: marketing Marketing page in WooCommerce Admin, i.e. `/wp-admin/admin.php?page=wc-admin&path=%2Fmarketing`. label Dec 21, 2022
@ecgan ecgan self-assigned this Dec 21, 2022
@ecgan
Copy link
Member Author

ecgan commented Dec 21, 2022

@nima-karimi , we need another API to get the list of campaign types when users want to create a new campaign. See CampaignType (https://github.com/woocommerce/woocommerce/pull/36130/files#diff-4b546ad9518e3bab627e59ff64c90e71996042324ef27c086ea3e1d5df284995R1-R9) for sample schema and useNewCampaignTypes (https://github.com/woocommerce/woocommerce/pull/36130/files#diff-bbefdd76fc0a5cfec2d63cf82336674d720a68319b0b347c1859f43d483aa9c1R11-R38) for sample data.

@ecgan
Copy link
Member Author

ecgan commented Dec 21, 2022

The implemented modal here is different from the figma designs in #34909. Specifically, there are no borders within the modal component. This is to be more consistent with the WordPress Modal component which no longer has border under the modal title since PR WordPress/gutenberg#40781.

@shivanarrthine , let me know what you think about the way I implement the modal. I try to stay close to the original design but without the borders. You can make the change in Figma and I can change the implementation accordingly.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2022 •
edited

Test Results Summary

Commit SHA: c89b281

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests000000NaNm NaNs
E2E Tests000000NaNm NaNs
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.

@shivanarrthine
Copy link

Thanks for raising this, @ecgan. I'm good with the change, although I wonder if it's possible to add a divider between the sections to provide some structure to the content. (quick mockup below)
mcm_modal

Also, would it be possible to hide installed extensions from the 'add new channels' list? Eg. if Google and Pinterest are installed then it shouldn't reappear as a suggested channel to be installed.

@ecgan
Copy link
Member Author

ecgan commented Dec 22, 2022

@shivanarrthine ,

although I wonder if it's possible to add a divider between the sections to provide some structure to the content. (quick mockup below)

Yup, that can be done, I'll do that. 🙂 👍

would it be possible to hide installed extensions from the 'add new channels' list? Eg. if Google and Pinterest are installed then it shouldn't reappear as a suggested channel to be installed.

Yup, that is the plan, installed channels should not show up in the "add new channels" recommendation list. And if all the recommended channels are installed and there are no more recommended channels, then the "Add channels for other campaign types" section should not show up.

@ecgan
Copy link
Member Author

ecgan commented Dec 22, 2022

@nima-karimi , we need another API to get the list of campaign types when users want to create a new campaign. See CampaignType (#36130 (files)) for sample schema and useNewCampaignTypes (#36130 (files)) for sample data.

@nima-karimi and I had a quick discussion about this (Slack DM: p1671648075729309-slack-D01D3A2V188). Currently we do not have the functionality to add campaigns in the backend, and it is a bit complicated to implement it. For the MVP, we are focusing on showing the campaigns, so we will put "campaign creation" functionality aside for future iteration.

@ecgan
Add border and change padding for recommended channels in create camp…
d0de1b5 
…aign modal.
@ecgan
Merge branch 'feature/34903-multichannel-marketing-frontend/34905-cam…
c89b281 
…paigns-card' into feature/34903-multichannel-marketing-frontend/34909-create-campaign-modal

Conflicts:
	plugins/woocommerce-admin/client/marketing/hooks/index.ts
	plugins/woocommerce-admin/client/marketing/overview-multichannel/Campaigns/Campaigns.tsx
	plugins/woocommerce-admin/client/marketing/overview-multichannel/Channels/Channels.tsx
	plugins/woocommerce-admin/client/marketing/overview-multichannel/Channels/RecommendedChannels.tsx
	plugins/woocommerce-admin/client/marketing/overview-multichannel/Channels/useChannels.ts
@ecgan ecgan mentioned this pull request Dec 28, 2022
Closed
7 tasks
@ecgan
Copy link
Member Author

ecgan commented Mar 2, 2023

Closing this PR in favor of PR #37044, because I want to change the branch name and to merge the changes into trunk branch instead of feature branch.

@ecgan ecgan closed this Mar 2, 2023
@ecgan ecgan mentioned this pull request Mar 2, 2023
Merged
9 tasks
@ecgan ecgan deleted the feature/34903-multichannel-marketing-frontend/34909-create-campaign-modal branch May 15, 2023 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ecgan ecgan

Labels
focus: marketing Marketing page in WooCommerce Admin, i.e. `/wp-admin/admin.php?page=wc-admin&path=%2Fmarketing`.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ecgan @shivanarrthine