Expand the logic of the `ACRNotificationCTAWidget` component for existing KMW users with manual set up
Feature Description
The ACRSubtleNotifciation component logic should be expanded so it surfaces in the dashboard within key metrics widget area for the users who have setup KMW manually, once ACR events are detected. CTA should open selection panel
FIgma design can be found here
Refer to the Subtle banner notification for detected events section of the design doc
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Following the implementation in 9371
ACRDashboardSubtleNotificationcomponent is updated to show/surface:- If key metrics were setup using manual selection
- And initial set of conversion events have been detected
- Said component should match the figma design
- Clicking the
Maybe laterCTA should dismiss the banner - Clicking the
Select metricsCTA should open the selection panel - Check the Subtle banner notification for detected events section of the design doc
Implementation Brief
- [ ] Update
ACRNotificationCTAWidgetcomponent- Banner should render if:
-
isUserInputCompletedselector fromCORE_USERstore isfalse - And
isKeyMetricsSetupCompletedistrue - And
getDetectedEventsAnalytics setting value is not an empty array - Following above conditions, include the CTA variation/callback:
-
Maybe laterCTA callback should behave in a same way as implementation in 9371 -
Select metricsCTA callback should open Selection panel https://github.com/google/site-kit-wp/blob/79b4417581b1cca3d718f7e55127681e6e05fc6f/assets/js/components/KeyMetrics/ChangeMetricsLink.js#L48
-
-
- Banner should render if:
- [ ] In
In ACRDashboardSubtleNotificationcomponent- Include two props:
ctaLabelandctaCallback - Apply
ctaLabelandctaCallbackto the primary button, and move existingAdd metricslabel and it's callback to theACRNotificationCTAWidgetto be used as a props forACRDashboardSubtleNotificationcomponent following the initial conditions used for that banner variation.
- Include two props:
Test Coverage
- Update tests for
ACRNotificationCTAWidget(basic coverage added in 9344), to verify that banner is showing when conditions are met, and not showing otherwise for example. Also tests for new CTA's variation and their callbacks
QA Brief
Changelog entry
@zutigrm AC ✅ Moving to IB
Thanks, @zutigrm, mostly looks good to me.
- Ensure that conditions apply only when component is shown outside of
VIEW_CONTEXT_SETTINGScontext
Which conditions? The ones from the next point? Plus instead of using negative statement we should explicitly say for which context it should be displayed. In other words, instead of component is shown outside of ... context, we should say component is shown in A, B, C, D context.
Thanks @eugene-manuilov I updated IB to list the contexts
Which conditions? The ones from the next point?
I expanded on this sentence to clarify that it should apply for conditions in next points
@zutigrm ok, since we updated #9371, we need to update IB for this ticket as well. Instead of adding conditions to the notification component, we need to update the widget to render different notifications based on aforementioned conditions. I think we can probably re-use the notification component from #9371 because it has similar content, right?
@eugene-manuilov Thanks, updated.
I think we can probably re-use the notification component from https://github.com/google/site-kit-wp/issues/9371 because it has similar content, right?
Yes that's the idea, that component will be reused and just conditions would be expanded
- Following above conditions, include the content variation matching this banner in Figma
@zutigrm the only difference between two versions of the notification is the CTA button:
So, there is no need to add any additional logic to it. Instead we need to add two props to the notification to set CTA label and callback. And then in the widget, we need to use different CTA labels and callbacks based on the defined criteria.
@eugene-manuilov THanks, IB updated
Thanks, IB ✔
QA Update ⚠
Hi @benbowler , referring to the QAB where it mentions about setting up the Tweak extension. I am not 100% clear on its usage and when is it being triggered. Can you clarify on that? Also, what are the data to be filled for the response payload, request payload and status code?
@kelvinballoo To chip in here, since you have the tester plugin I sent you, you won't need to use tweak, surfacing the callout by forcing the events will suffice, since once you click on it it will be dismissed without usage of extensions
QA Update ⚠
ITEM 1: ⚠ When clicking on 'Maybe Later' (Dismiss), the banner will disappear as expected. However, after reloading the page, the banner reappears. Is it because of the tester plugin functionality? Or we need to build a logic that after it's dismissed, it won't re-appear again?
Similarly when we click on 'Select metrics', the banner will disappear. However, after reloading the page, it will re-appear.
Unless these will be tackled under a separate ticket?
https://github.com/user-attachments/assets/b569a6f8-1665-4e86-a56e-c92d52608fdb
Select metrics scenario:
https://github.com/user-attachments/assets/efeb640e-7913-48af-a3f2-d960310372fa
ITEM 2: ⚠ The margin above the banner is supposed to be 12px based on Figma. In our implementation, it's 24px.
Implementation:
ITEM 3: ⚠ Let's say we have the following scenario:
- Some metrics are not selected. The banner will appear.
- All the metrics related to the particular event are now checked.
- The banner disappears accordingly.
- The thing now is that if I uncheck one of the metrics, the banner will resurface again. I assume that's not expected. Are we handling this in a separate ticket?
https://github.com/user-attachments/assets/3be67366-facc-428b-a931-c0bf9664c148
Tested the following and they were as expected: ✅
-
Scenario 1 - Select metrics banner appears accordingly
https://github.com/user-attachments/assets/3b478e5a-10c6-4f34-a67c-89413c3d280e
-
When clicking on Select metrics, the panel selection will appear accordingly.
https://github.com/user-attachments/assets/a66e8945-77a7-4ecf-a664-fb0343838a31
-
Scenario 2 - The banner appears again when not all the metrics were selected.
https://github.com/user-attachments/assets/3c6fc5f3-7b25-4573-9bcc-0fb95c612186
-
Scenario 3 - If all the relevant metrics are selected, the banner will not appear.
https://github.com/user-attachments/assets/7c2c99e8-c516-4b8f-ae7b-1cc4769c8550
-
Tested other scenarios pertaining to purchases and leads (contact, generate lead, submit lead form). Behaviours are similar as above.
https://github.com/user-attachments/assets/1c2ff58a-0eaf-429d-bf48-baacb4efcebe
https://github.com/user-attachments/assets/ce457962-568b-4390-b9ac-1362a49a2830
@kelvinballoo Thanks for you observations. I can answer items 1 and 3:
ITEM 1: ⚠ When clicking on 'Maybe Later' (Dismiss), the banner will disappear as expected. However, after reloading the page, the banner reappears. Is it because of the tester plugin functionality? Or we need to build a logic that after it's dismissed, it won't re-appear again?
Similarly when we click on 'Select metrics', the banner will disappear. However, after reloading the page, it will re-appear.
Unless these will be tackled under a separate ticket?
Yes, this is due to the tester plugin, as long as you have forced new events, it will override the transient, and banner will re-appear on page load, this is expected. To simulate the dismissal, in real usage, after you dismiss the banner, disable override of the new events - as dismissal removed that transient. In this case on reload you will not see the banner any more
ITEM 3: ⚠
Let's say we have the following scenario:
Some metrics are not selected. The banner will appear. All the metrics related to the particular event are now checked. The banner disappears accordingly. The thing now is that if I uncheck one of the metrics, the banner will resurface again. I assume that's not expected. Are we handling this in a separate ticket?
Interesting observation. Basically banner will be visible until user clicks on one of the CTAs, select metrics will dismiss the banner, as well as maybe later. Once dismissed, transient will be deleted, so if user adds new metrics through select metrics CTA, and then de-selects them banner will not re-appear, if banner was not actioned and users chooses new metrics, saves them and then de-select one, banner would re-appear. As it is there to notify user about new events until dismissed. In this far edge case, the user would just need to dismiss it. Not sure if should consider additional scenario, WDYT?
@kelvinballoo thinking more about item 3, it is better if we dismiss the banner, since even after several days, or week + if user deselect the item, banner will re-appear if not dismissed, which would be a bit confusing. We will have new badges appearing next to the metrics, but still yeah, let's dismiss this.
@benbowler We can update https://github.com/google/site-kit-wp/blob/548bf6b5e11b200b4928fe0eeab5ed4f35c478e2/assets/js/components/KeyMetrics/MetricsSelectionPanel/Footer.js#L144 callback to run same check we use for banner to appear shouldShowCalloutForUserPickedMetrics, and if it is false and there are conversionReportingEventsChange?.newEvents, invoke the dismissal action
If it will take more time we can also make it as a follow up issue
@kelvinballoo After checking the item 2, we have unified margin on all widget area headers (which is a global element) which is 24px. For consistency doesn't seem like something we should change, in the Figma, spacing differs only for this scenario of notification showing, and by default without notification it seems to be mistakenly reduced in Figma. We can omit this one from changing.
I have opened a follow up issue (#9870) for item 3 since it is not scoped as part of this issue which only works on adding the callout variation, and so we can properly define this new change.
QA Update ✅
Thanks @zutigrm
ITEM 1 is tested good after doing the steps to remove the detection of the events.
https://github.com/user-attachments/assets/9c358c17-9368-4717-9e29-b31956c4b51e
Noted on ITEM 2 and 3.
Tested the following and they were as expected as well: ✅
-
Scenario 1 - Select metrics banner appears accordingly
https://github.com/user-attachments/assets/3b478e5a-10c6-4f34-a67c-89413c3d280e
-
When clicking on Select metrics, the panel selection will appear accordingly.
https://github.com/user-attachments/assets/a66e8945-77a7-4ecf-a664-fb0343838a31
-
Scenario 2 - The banner appears again when not all the metrics were selected.
https://github.com/user-attachments/assets/3c6fc5f3-7b25-4573-9bcc-0fb95c612186
-
Scenario 3 - If all the relevant metrics are selected, the banner will not appear.
https://github.com/user-attachments/assets/7c2c99e8-c516-4b8f-ae7b-1cc4769c8550
-
Tested other scenarios pertaining to purchases and leads (contact, generate lead, submit lead form). Behaviours are similar as above.
https://github.com/user-attachments/assets/1c2ff58a-0eaf-429d-bf48-baacb4efcebe
https://github.com/user-attachments/assets/ce457962-568b-4390-b9ac-1362a49a2830