Open zutigrm opened 2 months ago
AC ✔️
@zutigrm, could you please add more details as to what should be done there? Also, I think it is worth moving that setup error notification to its own component, no?
@eugene-manuilov Thanks, good point, I suggested moving it into new component, and expanded a bit on IB where possible
Thanks, IB ✔️
Feature Description
This issue is for the refactoring an "Error" notification in Site Kit. It should refactor the setup error notification https://github.com/google/site-kit-wp/blob/d90ac7abb002c20a2de9d79abfd08b0fc52ccc86/assets/js/components/notifications/ErrorNotifications.js#L110-L122 so that it uses the new datastore infrastructure to register and queue the notification. It should also use newer lighter "layout" and "common" components that replace the bloated BannerNotification component as per the pattern introduced in https://github.com/google/site-kit-wp/issues/9071.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
core/notifications
datastore.ErrorNotifications
) but only via the genericgetQueuedNotifications
selector.BannerNotification
component. Instead, it should be rendered using the newNotification
component wrapper and a simpler "layout" component that solely encapsulates its structure and design.Implementation Brief
assets/js/components/notifications/SetupErrorMessageNotification.js
(name should differ from existingSetupErrorNotification
which has more generic data)UnsatisfiedScopesAlert
component for an exampleid
andNotification
Notification
component passed as the propBannerNotification
component, and replace it with the newassets/js/googlesitekit/notifications/components/layout/NotificationError.js
layout using the existing props used to forBannerNotification
assets/js/googlesitekit/notifications/register-defaults.js
checkRequirements
use the existing checks fromassets/js/components/notifications/ErrorNotifications.js
- thesetupErrorMessage
variable and it's logic to determine if notification should be shown or not10
from the last added error notification (they start from 150)NOTIFICATION_AREAS.ERRORS
forareaSlug
BannerNotification
from theassets/js/components/notifications/ErrorNotifications.js
Test Coverage
assets/js/components/notifications/ErrorNotifications.stories.js
QA Brief
Changelog entry