Open joemcgill opened 2 months ago
@eason9487 or @asvinb I think this one is ready for review.
In the first image, the Google Ads account status is created but not yet connected; In the second image, the status is being updated but not yet turned to connected.
The third image shows the UI state after clicking the Edit button, but I'm not sure if it will be able to show the UI state as expected after clicking that button in the state of the first two images.
@eason9487 , @joemcgill Correct me if am wrong, but the "Edit" button will be displayed only when all the 3 accounts are connected 🤔 The brief looks good @joemcgill , except for one thing which is not clear in the designs. What happens to the "Claim Account in Google Ads" button once the user clicks on it? From the designs, it looks like there's the loading state which is rendered (Updating as label) which can be confusing when looking at the designs. We can keep the current implementation where we always show the claim button until the user has claimed his account.
Hi @asvinb
Correct me if am wrong, but the "Edit" button will be displayed only when all the 3 accounts are connected
It doesn't seem to have been designed this way from the current Figma file.
In the current implementation, each account card has a disconnect button once the connection is complete, and they don't need to wait for all three accounts to be connected before displaying it.
This makes sense because when a user just connects to a Google account and then wants to disconnect it, they shouldn't have to wait for all the other accounts to connect before they can do so.
@eason9487 and @asvinb, implementing the edit button will be handled in a separate issue since the requirement for that link are dependent on a few other pieces of UI.
The objective of this issue is only to handle part of the Ads connection flow that requires claiming a new Ads account so the connection can be completed. The result of this ticket will be to display the GoogleComboAccountCard
in the state illustrated by the screenshot below, where all three accounts are connected and set up and the onboarding stepper can continue to step 2.
I've added more details about the edit link in #2605. The implementation brief is still needed, but hopefully there are enough details to explain the product vision.
With #2567 already in progress, this is nearly unblocked. @ankitrox had some alternate ideas about how we might implement this rather than using the upsertAdsAccount
hook, which I'm happy for us to discuss. However, I think the general approach defined here still looks like a good starting point.
@eason9487 could you review this again with a focus only on the implementation of the claim accounts flow being added to this component and call out any additional concerns that we should account for before starting this work?
As per the Compact and Semi-expanded states in #2605 and the images in this ticket, will the UI of claiming flow still be placed in the top card body section regardless of the state of the card?
@eason9487 good question, since the designs do not make that clear. For the MC accounts the reclaim URL flow can display in either the top card body OR the bottom card when expanded. It could make sense to do the same here, but that might make the top card logic more complex. Let me see if I can get feedback from @michaeleleder about this.
@eason9487 I've gotten confirmation from @fblascogarma that we will always display the claim component in the top card body, regardless. If the account is connected but not claimed, and the card is expanded, the unclaimed account will likely show in it's connected state, but the flow won't be able to be completed until after the account is claimed.
I don't think we need any changes to this issue to account for the expected behavior, but may need to review things again once #2603 is implemented. Do you have any other concerns with this issue before we mark it as ready for implementation?
Going to go ahead an move this into ToDo and we can discuss any concerns async.
This work should be based off the PR for #2567 but can be started now.
Do you have any other concerns with this issue before we mark it as ready for implementation?
This conclusion doesn't seem to be certain that there is no conflict at all between the requirements.
Conflicts or uncertainties are:
we will always display the claim component in the top card body, regardless.
Following this, the experience of creating a new Google Ads account looks like this:
That is, the operations of the process will be placed in seemingly disjointed positions.
Part of https://github.com/woocommerce/google-listings-and-ads/issues/2509
As a follow-up to #2567, when a new Google Ads account is created, the account needs to be claimed before the connection process can be completed.
Clicking the "Claim account..." button will open the current pop-up flow for accepting a Google Ads account, and the card will show an updating state while the account is being accepted.
Once the Ads account has been accepted and the setup process has been completed, the following success state will be shown.
If we aren't able to confirm that the account was claimed when the user returns focus to the screen after entering the claim flow, we return to the original claim state shown in the first screenshot.
Designs: 🔗 Figma Onboarding
Acceptance Criteria
Note that the "Edit" link displayed in the screenshots is not in scope of this issue and will be handled separately.
Implementation Brief
A new sub-component,
ClaimAdsAccount
will be created in the directory for theGoogleComboAccountsCard
component to show the claim messaging and render theClaimAccountButton
button (can likely reuse the existing component from here). This component should support anisLoading
prop to control whether the button is being shown or the "Updating..." UI.The
GoogleComboAccountsCard
component will be responsible for checking whether the account needs to be claimed by checking thehasAccess
value returned fromuseGoogleAdsAccountStatus()
to determine whether this UI is visible. Additionally, once the account is claimed, theGoogleComboAccountsCard
will need to callupsertAdsAccount
to trigger the rest of the account setup process (reference). When checking the account status or updating the account, theGoogleComboAccountsCard
can setisLoading:true
.Once the
GoogleComboAccountsCard
has confirmed that we have access to the account and the setup has been completed fromuseGoogleAdsAccountStatus()
, i.e.hasAccess === true && step === ""
, the success notice should be shown instead.Test Coverage
A new set of E2E test should be added to
tests/e2e/specs/setup-mc/step-1-accounts.test.js
confirming that the invitation flow is working per the AC.Definition Questions