Refactor `CoreSiteBannerNotifications` to use the new Notifications datastore

[zutigrm]

Feature Description

This issue should refactor CoreSiteBannerNotifications 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 CoreSiteBannerNotifications component should be refactored so that it is registered and rendered (queued) using the new core/notifications datastore.
• This notification component should not be called directly (i.e. in BannerNotifications) but only via the generic getQueuedNotifications selector.
• 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 BannerNotification component. Instead, it should be rendered using the new Notification component wrapper and a simpler "layout" component that solely encapsulates its structure and design.

Implementation Brief

• [ ]

Test Coverage

QA Brief

Changelog entry

[eugene-manuilov]

AC ✔️

[benbowler]

Morning @jimmymadon looking at the purpose of the CoreSiteBannerNotifications component, I have a few questions as it's not a standard notification.

It is related to the core/site notifications store assets/js/googlesitekit/data/create-notifications-store.js. I looked into this store and I can't see any calls to addNotification in the code base any more.

Should we consider removing this legacy store and components as part of this ticket?

[benbowler]

CC @eugene-manuilov @aaemnnosttv, do you agree we can remove the legacy assets/js/googlesitekit/data/create-notifications-store.js createNotificationsStore as the core of this ticket or are there other elements of the app that I missed that require this infrastructure?

[benbowler]

Looking at the IB for #9300, the AdSense module implements the module level notification store to handle AdSense notifications in the AdSenseAlerts component. This means that createNotification store cannot immediately be removed without refactoring this and other uses of the notification store within modules.

[benbowler]

Removing my assignment incase anyone else want to take up this discussions while I'm out but there are some outstanding questions to get this implemented.

[jimmymadon]

Having discussed this and #9300 with @aaemnnosttv, we decided to hold off on these issues to prevent "nested" queues of notifications. This is because these notification components call a getNotifications selector which fetches one or more notifications and maps through them to render them. We might be better off creating these as "on-demand" notifications and so, we will wait till #9453 is completed first before we amend the AC/IB here.

[tofumatt]

Estimate here is high because it will involve test removal and triggering server notifications for testing/QA. The engineering effort involved is likely a bit closer to a 7. 🤔

[eugene-manuilov]

@tofumatt, let's be more specific as to what we need to do in IB. Which core notifications should be added and where exactly? Please, update IB to have enough details to understand how we need to implement this ticket.

[tofumatt]

@eugene-manuilov The core notifications that will be registered aren't specific, since they're server notifications that are sent from the service.

I've updated the IB, but we're essentially changing the creation of notification components from notifications.map to using registerNotification.

But you're right we'll want to create a ServerNotification component I didn't mention, so I've added that to the IB 👍🏻

[eugene-manuilov]

Thanks, @tofumatt. IB ✔

[mohitwp]

QA Update ❌

• Tested on dev environment.
• Verified and compared notification behaviour and design with latest environment.

@tofumatt

Issue > On the dev environment, an additional margin is being applied to the notice title. As a result, unnecessary extra space is appearing above and below the title.

Latest

Dev -

[tofumatt]

The margins there aren't additional, they seem to be standard margins set in our styles, but the top margins do seem a bit much. I've adjusted them to look a bit less awkward re: top margins. But the bottom ones should remain:

[mohitwp]

QA Update ✅

• Tested on dev environment.
• Verified and compared notification behaviour and design with latest environment.
• Verified that notification getting dismiss when we click on the CTA's.
• Verified that Confirm and dismiss notification GA4 events are getting track when we click on the CTA's.
• Verified that above reported issue is resolved now.

https://github.com/user-attachments/assets/90fd666f-c8bc-4bc1-b091-ec33450e9f82