Skip to content

Free Listings + Paid Ads: Add the boost product listings section #1649

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
Merged
merged 3 commits into from
Aug 30, 2022
Merged

Free Listings + Paid Ads: Add the boost product listings section #1649

merged 3 commits into from
Aug 30, 2022

Conversation

eason9487
Copy link
Member

@eason9487 eason9487 commented Aug 24, 2022 •
edited
Loading

Changes proposed in this Pull Request:

This PR implements a part UI of 📌 Boost product listings section in #1610.

💡 The ad previews will be added by a subsequent PR.

Screenshots:

2022-08-24 18 20 38

Detailed test instructions:

  1. Go to step 4 of the onboarding flow.
  2. Check if the visual result of the "Boost product listings with paid ads" section is close to the design in Figma.
  3. Click on the "Create a paid ad campaign" button. It should hide the footer of this section and show the other two buttons at the bottom of page.
  4. The "Skip this step for now" button and both buttons at the bottom should complete the setup and redirect the page to the Product Feed.

Changelog entry

@eason9487 eason9487 added the changelog: none Skip changelog entry for this PR label Aug 24, 2022
@eason9487 eason9487 requested a review from a team August 24, 2022 11:18
@eason9487 eason9487 self-assigned this Aug 24, 2022
@github-actions github-actions bot added changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement. labels Aug 24, 2022
@eason9487 eason9487 removed type: enhancement The issue is a request for an enhancement. changelog: add A new feature, function, or functionality was added. labels Aug 24, 2022
@eason9487 eason9487 mentioned this pull request Aug 25, 2022
Closed
35 tasks
@puntope
Copy link
Contributor

puntope commented Aug 29, 2022 •
edited
Loading

❓ When I checkout this PR and go over the setup steps I got some errors in the console. Then after finishing Step 1, i go a fatal and i cannot continue.

I tried develop and works as expected. So I merged develop and it works.

Screenshot 2022-08-29 at 16 41 44

puntope
puntope reviewed Aug 29, 2022
View reviewed changes
js/src/setup-mc/setup-stepper/setup-paid-ads/paid-ads-features-section.js
Comment on lines +17 to +38
function FeatureList() {
const featuresItems = [
{
Icon: GridiconCheckmark,
content: __(
'Promote your products across Google Search, YouTube, Display, Discover, Maps, Gmail, and more.',
'google-listings-and-ads'
),
},
{
Icon: GridiconCheckmark,
content: __(
'Set a daily budget, and only pay when someone clicks.',
'google-listings-and-ads'
),
},
{
Icon: GridiconGift,
content: __(
'Claim $500 in ads credit when you spend your first $500 with Google Ads. Terms and conditions apply.',
'google-listings-and-ads'
),
Copy link
Contributor

Choose a reason for hiding this comment

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

💅 Possible to divide the components in multiple files? 1 per component?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. However, I lean toward keeping the component here because it is not used by others and it's a plain component.

puntope
puntope reviewed Aug 29, 2022
View reviewed changes
js/src/setup-mc/setup-stepper/setup-paid-ads/setup-paid-ads.js
Comment on lines +67 to +68
loading={ completing === 'skip-ads' }
disabled={ completing === 'complete-ads' }
Copy link
Contributor

@puntope puntope Aug 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

💅 WDYT about using some constants here?

Copy link
Member Author

@eason9487 eason9487 Aug 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Considering the following two PRs are also changing related code fragments, I updated it in PR #1655 by 0e65308 and c08364f.

@puntope
Copy link
Contributor

puntope commented Aug 29, 2022

✅ LGTM

I tested locally..

Left some comments in code but they are minor.

Approved in advance.

eason9487 added a commit that referenced this pull request Aug 30, 2022
@eason9487
Use constants for the action values in the SetupPaidAds component.
0e65308 
Address #1649 (comment)
@eason9487
Copy link
Member Author

Thanks for the review, @puntope

❓ When I checkout this PR and go over the setup steps I got some errors in the console. Then after finishing Step 1, i go a fatal and i cannot continue.

I tried develop and works as expected. So I merged develop and it works.

I believe this problem relates to issue #1641 and has been fixed by #1642.

@eason9487 eason9487 merged commit a0d2df8 into feature/1610-streamlined-onboarding Aug 30, 2022
@eason9487 eason9487 deleted the add/1610-paid-ads-features-section branch August 30, 2022 03:18
@eason9487 eason9487 mentioned this pull request Aug 30, 2022
Merged
@eason9487 eason9487 added changelog: add A new feature, function, or functionality was added. and removed changelog: none Skip changelog entry for this PR labels Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@puntope puntope puntope approved these changes

Assignees

@eason9487 eason9487

Labels
changelog: add A new feature, function, or functionality was added.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eason9487 @puntope