Create Ads Module Setup CTA Banner about site-kit-wp HOT 11 CLOSED

Thanks, @benbowler. Mostly looks good to me.

• Create the primary CTA "Set up Ads" as a default Button component which links to ${ settingsURL }&slug=ads&reAuth=true with the settingsURL retrieved with the getAdminURL selector passing googlesitekit-settings.

We need to use the same approach that we use in the settings when we click on the connect button:

... we could quite easily have the Consent Mode and Ads Module widgets both rendering on the dashboard...

Yes, let's not worry about that for now. We will create a follow up task if it is needed.

from site-kit-wp.

QA Update ❌

• Tested on dev environment.
• Verified all cases mentioned under AC and QAB.
• Compared design with Figma.

@zutigrm Please find my observations below

1) Font family for the banner title is 'Google Sans Display' in Figma where as under actual implementation it is 'Google Sans Text'.

2) We don't have figma design for large tablet viewports but design under Viewports having width between 961px to 1132px is looking off with space appearing below the image.

3) In smaller tablet viewports font size is different from figma. On smaller tablets we implemented same font size as mobile viewports. Tested on iPad air and Surface Pro smaller tablet viewports.

4) The 'Maybe later' CTA is not functioning as expected according to the acceptance criteria and QA brief. Clicking on the 'Maybe later' CTA does not dismiss the banner but instead renames the CTA to "Don’t show again". Clicking on 'Don’t show again' dismisses the banner. Additionally, upon each refresh when the dashboard loads, the Ads module setup CTA banner briefly displays for a few milliseconds before hiding.

from site-kit-wp.

@mohitwp Thanks for checking it. Hm, I tested it manually on my end, as sometimes reference date doesn't affect some places. Which seems to be the case. You can edit the database, in wp_usermeta table, find the wp_googlesitekitpersistent_dismissed_prompts column and edit it.

You will see ads_module_setup_banner_prompt_dismissed_key in the data, under it, there is expires property, you can replace current timestamp with this one 1716882640 (it is 28th May)

from site-kit-wp.

Looks good. I'm guessing we don't have any GA events defined yet for this banner, but we should probably add those as well in a follow-up issue 😅

(If we get them in soon we can probably just add it to this issue's ACs.)

ACs look good, moving to IB.

from site-kit-wp.

I've created the IB today, one thing to note and discuss in the IBR is that we could quite easily have the Consent Mode and Ads Module widgets both rendering on the dashboard. We could prevent this as a one off by doing some kind of check for the Consent Mode display state within this new Widget but that's not very flexible and brings in CoMo logic to an Ads module widget, although perhaps this is acceptable for now?

We could perhaps create a follow up ticket to create a higher level component for these banners that handles the checks and only renders one at a time regardless if multiple widgets meet their display condition.

I'm also waiting on details of the responsive designs from @sigal-teller here.

from site-kit-wp.

Putting this into IBR, this comment from above still applies and is worth discussing:

I've created the IB today, one thing to note and discuss in the IBR is that we could quite easily have the Consent Mode and Ads Module widgets both rendering on the dashboard. We could prevent this as a one off by doing some kind of check for the Consent Mode display state within this new Widget but that's not very flexible and brings in CoMo logic to an Ads module widget, although perhaps this is acceptable for now?

We could perhaps create a follow up ticket to create a higher level component for these banners that handles the checks and only renders one at a time regardless if multiple widgets meet their display condition.

from site-kit-wp.

Thanks @eugene-manuilov, I've updated the IB module activation steps.

from site-kit-wp.

Thanks, @benbowler. IB ✔️

from site-kit-wp.

QA Update ⚠️

• Tested on main environment.
• Verified above reported issues 1 to 3 are resolved now.

@zutigrm I verified that on clicking the 'Maybe later' CTA banner, it gets dismissed and does not reappear when reloading the dashboard. However, it doesn't reappear even when I set the reference date in the tester plugin to over 14 days, 15 days, or 1 month later. Do I need to make some changes in the database, or is it getting permanently dismissed? Can you please check?

PASS CASES

1) Font family for the banner title is 'Google Sans Display' in Figma where as under actual implementation it is 'Google Sans Text'. - PASS

2. We don't have figma design for large tablet viewports but design under Viewports having width between 961px to 1132px is looking off with space appearing below the image. - PASS

3) In smaller tablet viewports font size is different from figma. On smaller tablets we implemented same font size as mobile viewports. Tested on iPad air and Surface Pro smaller tablet viewports. - PASS

from site-kit-wp.

QA Update ❌

• Tested on main environment.
• Verified functionality of 'Maybe later' CTA and 'Don't show' CTA.
• Note : Setting reference date in Tester plugin was not working so I tested it through db manipulation mentioned above.

@zutigrmI tested this on three different sites. According to the acceptance criteria, after clicking 'Maybe later', the CTA banner should display again in 14 days. In the database, the expiry date is correctly set to 14 days later. However, when I set a past date in the database, it forces the banner to display on the dashboard, but it still shows the 'Maybe later' CTA. It only displays the 'Don't show' CTA after appearing twice with the 'Maybe later' CTA.

Expected: On the second appearance, the banner should display the 'Don't show' CTA instead of 'Maybe later'.

from site-kit-wp.

QA Update ✅

• Tested on main environment.
• Verified 'Maybe later' and 'Don't show' CTA functionality working as per AC.
• Above issue was due to space copied with timestamp.
• Verified all above issue 1 to 4 are resolved now.

1) Font family for the banner title is 'Google Sans Display' in Figma where as under actual implementation it is 'Google Sans Text'. - PASS

2. We don't have figma design for large tablet viewports but design under Viewports having width between 961px to 1132px is looking off with space appearing below the image. - PASS

3) In smaller tablet viewports font size is different from figma. On smaller tablets we implemented same font size as mobile viewports. Tested on iPad air and Surface Pro smaller tablet viewports. - PASS

4) The 'Maybe later' CTA is not functioning as expected according to the acceptance criteria and QA brief. Clicking on the 'Maybe later' CTA does not dismiss the banner but instead renames the CTA to "Don’t show again". Clicking on 'Don’t show again' dismisses the banner. Additionally, upon each refresh when the dashboard loads, the Ads module setup CTA banner briefly displays for a few milliseconds before hiding. PASS

from site-kit-wp.