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

Free Listings + Paid Ads: Use merchant's data to composite the ad previews #1692

Conversation

eason9487
Copy link
Member

@eason9487 eason9487 commented Sep 23, 2022

Changes proposed in this Pull Request:

This PR implements an item of the 📌 Client states management and API integration in #1610.

  • Fetch the product and site data from API and preload data for compositing the ad previews.
  • Extra change: Tweak ScaledText style to prevent the font descender from getting cropped out.
    • Font descender was cropped out:
      2022-09-23 12 51 34
    • After tweak:
      2022-09-23 12 52 34

Screenshots:

Kapture.2022-09-23.at.13.07.14.mp4

Detailed test instructions:

💡 There is an additional backend check that needs to be around, and it will be adjusted in a later PR. Basically, the API integration can be tested for now. To bypass that check, temporarily replace this line with:

return $this->is_google_connected();

Updated: I rebased this PR onto #1693, so it should not need the above local adjustment.

  1. Set a logo and site title for your shop: Appearance -> Customize -> Site Identity -> edit Logo and Site Title settings -> click on Publish button.
  2. Go to step 4 of onboarding flow.
  3. Check if the product and shop data come from your shop's data.

Changelog entry

@eason9487 eason9487 requested a review from a team September 23, 2022 07:39
@eason9487 eason9487 self-assigned this Sep 23, 2022
@github-actions github-actions bot added the changelog: update Big changes to something that wasn't broken. label Sep 23, 2022
@eason9487 eason9487 force-pushed the update/1610-integrate-ad-preview-with-api branch from b257ada to b5f51d4 Compare September 23, 2022 07:42
@eason9487 eason9487 force-pushed the update/1610-integrate-ad-preview-with-api branch from b5f51d4 to 39668f8 Compare September 23, 2022 08:54
@eason9487 eason9487 changed the base branch from feature/1610-streamlined-onboarding to update/1610-do-not-refresh-mc-status-when-not-onboarded-for-product-feed-api September 23, 2022 08:55
Copy link
Contributor

@puntope puntope left a comment

Choose a reason for hiding this comment

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

✅ LGTM

I tested locally and I see the title, logo and product

Screenshot 2022-09-23 at 15 56 20

Screenshot 2022-09-23 at 15 56 10

I also edited in a block theme and worked as well.

Base automatically changed from update/1610-do-not-refresh-mc-status-when-not-onboarded-for-product-feed-api to feature/1610-streamlined-onboarding September 26, 2022 06:39
@eason9487 eason9487 merged commit 84397ce into feature/1610-streamlined-onboarding Sep 26, 2022
@eason9487 eason9487 deleted the update/1610-integrate-ad-preview-with-api branch September 26, 2022 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: update Big changes to something that wasn't broken.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants