-
Notifications
You must be signed in to change notification settings - Fork 21
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: Add the product feed status section UI #1638
Free Listings + Paid Ads: Add the product feed status section UI #1638
Conversation
…ds setup step of onboarding flow
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.
LGTM.
✅ The UI looks close to the design.
✅ The tooltip was displayed when hovering on the # products
.
/** | ||
* Product status statistics on Google Merchant Center | ||
* | ||
* @typedef {Object} ProductStatistics |
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.
💅 Since the type of ProductStatistics
is defined, it would be nice to add some doc comments in the below function that is using this type.
export function* receiveMCProductStatistics( mcProductStatistics ) { |
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.
Added in 4566683.
/* | ||
const { data, hasFinishedResolution } = useAppSelectDispatch( | ||
'getMCProductStatistics' | ||
); | ||
*/ | ||
// TODO: Replace the dummy data with the above code later to use the adjusted API. |
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.
❔ Just wanted to make sure my understanding is right. The API mc/product-statistics
would only be returning data after we call mc/settings/sync
, so the comment Replace the dummy data with the above code later to use the adjusted API.
means we would make a POST request to mc/settings/sync
later, right?
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.
Yes, as per my test from the frontend side, it has to call mc/settings/sync
before getting statistics from mc/product-statistics
, so the frontend side will call mc/settings/sync
when entering step 4. And the mc/product-statistics
seems to check whether the MC_SETUP_COMPLETED_AT
flag is set before returning, thus, we will need the related adjustment in this project as well.
Changes proposed in this Pull Request:
This PR implements the UI part of 📌 Product feed status section in #1610.
getNumberOfSyncProducts
function.ProductFeedStatusSection
component and add it to the paid ads setup step of onboarding flow.Screenshots:
📷 Normal view
📷 Hover on the "# products" text
Detailed test instructions:
Changelog entry