Refactor `GoogleTagIDMismatchNotification` to use the new Notifications datastore
Feature Description
This issue should refactor GoogleTagIDMismatchNotification so that it uses the new datastore infrastructure to register and queue the notification. It should also incorporate a new "layout"
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- The
GoogleTagIDMismatchNotificationcomponent should be refactored so that it is registered and rendered (queued) using the newcore/notificationsdatastore. - This notification component should not be called directly (i.e. in
BannerNotifications) but only via the genericgetQueuedNotificationsselector. - The refactored component should not contain any business logic that hides it / prevents it from rendering. This logic should be contained in a callback function defined during registration.
- The component should not use the bloated
BannerNotificationcomponent. Instead, it should be rendered using the newNotificationcomponent wrapper and a simpler "layout" component that solely encapsulates its structure and design.
Implementation Brief
-
[ ] Update
assets/js/components/notifications/GoogleTagIDMismatchNotification.js:- The component should except new props
{ id, Notification }. - Combine the rendering of both variants of the component, wrapping the new render with
Notificationcomponent from the component props.- https://github.com/google/site-kit-wp/blob/cc981876c3b654437e965cb66f8a09c9d110dba1/assets/js/components/notifications/GoogleTagIDMismatchNotification.js#L167-L236
- Use the existing layout
assets/js/googlesitekit/notifications/components/layout/NotificationError.js.- Conditionally pass the title, description and action props based on the values of
newAnalyticsPropertyandnewGoogleTagID. - Update
NotificationError, adding a new prophideIcon, if true hide theBannerIconand pass true in this component.
- Conditionally pass the title, description and action props based on the values of
- The component should except new props
-
[ ] Update
assets/js/googlesitekit/notifications/register-defaults.js:checkRequirementsshould use this rendering logic:- https://github.com/google/site-kit-wp/blob/cc981876c3b654437e965cb66f8a09c9d110dba1/assets/js/components/notifications/BannerNotifications.js#L72-L74
isDismissibleshould befalse.- For priority, should be
150as this is an error. - Use
NOTIFICATION_AREAS.BANNERS_ABOVE_NAVforareaSlug.
-
[ ] Remove all references to
GoogleTagIDMismatchNotificationinassets/js/components/notifications/BannerNotifications.js.
Test Coverage
- Update the stories in
assets/js/components/notifications/GoogleTagIDMismatchNotification.stories.jsconfirm both variants render correctly based on the existing render logic in the new notification infrastructure.
QA Brief
Changelog entry
AC ✔️
- Update
assets/js/googlesitekit/notifications/register-defaults.js:
@benbowler, let's move the GoogleTagIDMismatchNotification notification to be defined and registered in the Analytics module since this is the module that notification belongs to.
- Update
assets/js/components/notifications/GoogleTagIDMismatchNotification.js:
@benbowler, this component should be moved to the Analytics module.
IB ✔
QA Update ⚠
- Tested on dev environment.
- Verified
GoogleTagIDMismatchNotificationcomponents. - Verified both Banner A and Banner B of Google tag ID mismatch notifications are showing.
- Verified both Banner A and Banner B have correct copy.
- Verified new Storybook Scenarios created under
GoogleTagMismatchIdNotification. - Verified all other notifications on the site especially those that have both, a Primary (CTA Button) and Secondary (CTA Link).
- Verified that new GA events for each CTA action similar to other notification refactoring issues -
view_notification, confirm_notification, and dismiss_notification. - Note: I found an issue related to AdSense—'Earning at Risk' notification appears when setting up Tag Manager. However, this issue also exists in the latest environment, so I will create a separate ticket for it.
QUESTION @jimmymadon, I noticed that after reloading twice, Banner A appears, but it takes 5-6 seconds( On 5G network) to display on the main dashboard, whereas Banner B appears immediately.
Is there a way to make Banner A display faster, similar to Banner B?
https://github.com/user-attachments/assets/b1414d0c-583c-44f0-95ff-cc430b062b35
Google ID mismatch Notifications
Banner A
Banner Scenario B
https://github.com/user-attachments/assets/67819825-3343-4cbd-a0d0-ae6a4459cabd
https://github.com/user-attachments/assets/6b936746-cca0-44b4-9146-f3f121201157
All other notifications
Storybook Stories
@mohitwp I've tested this but unfortunately, there isn't much we can do immediately here to speed this up. Even the legacy notice took quite a while to load - just that it happened without an additional page reload. We are planning to revamp the way these time consuming banners load on the main dashboard in the upcoming Notification Centre epic.
I will raise this in my 1:1 with @aaemnnosttv later today. Considering this isn't a regression and this banner itself is quite "rare", we can let this move forward and tackle slower loading notifications separately.
QA Update ✅
Thanks @jimmymadon ! I will create a separate ticket for this.
- Tested on dev environment.
- Verified
GoogleTagIDMismatchNotificationcomponents. - Verified both Banner A and Banner B of Google tag ID mismatch notifications are showing.
- Verified both Banner A and Banner B have correct copy.
- Verified new Storybook Scenarios created under
GoogleTagMismatchIdNotification. - Verified all other notifications on the site especially those that have both, a Primary (CTA Button) and Secondary (CTA Link).
- Verified that new GA events for each CTA action similar to other notification refactoring issues -
view_notification, confirm_notification, and dismiss_notification. - Note: I found an issue related to AdSense—'Earning at Risk' notification appears when setting up Tag Manager. However, this issue also exists in the latest environment, so I will create a separate ticket for it.
Google ID mismatch Notifications
Banner A
Banner Scenario B
https://github.com/user-attachments/assets/67819825-3343-4cbd-a0d0-ae6a4459cabd
https://github.com/user-attachments/assets/6b936746-cca0-44b4-9146-f3f121201157
All other notifications
Storybook Stories
Update : I created separate issues for my observations reported above - #10488 and #10489