Comments (11)
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 thesettingsURL
retrieved with thegetAdminURL
selector passinggooglesitekit-settings
.
We need to use the same approach that we use in the settings when we click on the connect button:
site-kit-wp/assets/js/components/settings/SetupModule.js
Lines 60 to 69 in e626cfc
... 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.
Recording.993.mp4
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.
![image](https://private-user-images.githubusercontent.com/10414808/334861639-2ea9d6b7-b8cb-404d-a7d0-1b8aee5b91be.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjM4NjYwMTUsIm5iZiI6MTcyMzg2NTcxNSwicGF0aCI6Ii8xMDQxNDgwOC8zMzQ4NjE2MzktMmVhOWQ2YjctYjhjYi00MDRkLWE3ZDAtMWI4YWVlNWI5MWJlLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MTclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODE3VDAzMzUxNVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTU1ZWQ2ZDczNmZmYjA5NTMwZmVjMzlmMTI2ODMzMzA2M2E0ZDM4Yzc5OGY3YjRjNzgxYWQwYzkxYjMyMDNlNjgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.uzbl5kHkN1HwqaJujVj_8mCF2FbJM8whkuOeplt7mNo)
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)
![image](https://private-user-images.githubusercontent.com/10414808/334862208-19170c58-d7e8-4624-9ebb-da856d91b395.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjM4NjYwMTUsIm5iZiI6MTcyMzg2NTcxNSwicGF0aCI6Ii8xMDQxNDgwOC8zMzQ4NjIyMDgtMTkxNzBjNTgtZDdlOC00NjI0LTllYmItZGE4NTZkOTFiMzk1LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA4MTclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwODE3VDAzMzUxNVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWM3YWM1OWQ3NTRkNDZmZDdkZTE0MTVkNWY4ZDg5YTgwZjcxMWNjOGE1MzM4Y2Y2OTYzZDc1MTAwZDVhODNkNTkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.NtSMgKtxsQWQqs2LbZ9nNDHavt24Gmr0AoilLUYpqBs)
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?
'Maybe later' CTA screencast
Recording.1005.mp4
Banner not reappearing when I set the reference date in tester plugin to something over 14 days, 15days or 1 month.
Recording.1006.mp4
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
- 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'.
Recording.1009.mp4
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
- 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
'Maybe later' CTA
Recording.1005.mp4
'Don't Show' CTA
Recording.1012.mp4
from site-kit-wp.
Related Issues (20)
- Creation of `Top cities driving purchases` ACR KMW
- Creation of `Top cities driving purchases` ACR KMW
- Creation of `Top device driving purchases` ACR KMW
- Creation of `Top device driving purchases` ACR KMW
- Creation of `Top traffic source driving purchases` ACR KMW
- Glitches / errors when changing the audience selection in mobile viewports
- Impossible to stop tagmanager code placement HOT 1
- Agency with hundreds of client Google Analytics accounts unable to assign correct profiles HOT 2
- Remove `conversionInfra` feature flag
- Remove localization for module homepage URLs
- Improve the `resetPublications` action HOT 6
- Requests made for module settings on dashboard
- Implement Internal Measurement/Tracking Following ACR KMW Updates
- Enhance compatibility with Docker locally for E2E
- Refactor the `ZeroDataNotification` to use the new lighter `Notification` component. HOT 2
- Release 1.134.0
- Add "Edit in Reader Revenue Manager" link for the RRM module
- Sort Storybook menu items HOT 1
- Enhance feature activation for BC
- Revert the Fix For VRT failures
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from site-kit-wp.