Open zutigrm opened 2 weeks ago
AC ✔️
@zutigrm, could you please expand IB to provide more details what exactly needs to be moved to the checkRequirements
function, what should be deleted from the component, etc.
@eugene-manuilov thanks. It is relatively small and straightforward component, I expanded a bit where possible.
Thanks, @zutigrm. IB ✔️
Feature Description
This issue is the first issue to refactor an "Error" notification in Site Kit. It should refactor the
SetupErrorNotification
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 bloatedBannerNotification
component as per the pattern introduced in #9071.Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
SetupErrorNotification
component should be refactored so that it is registered and rendered (queued) using the newcore/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/SetupErrorNotification.js
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
transfer the existing check inassets/js/components/notifications/SetupErrorNotification.js
- https://github.com/google/site-kit-wp/blob/b6c7c59c9bece36c8b0138d2e3d4f05a2f09479a/assets/js/components/notifications/SetupErrorNotification.js#L51-L5410
from the last added error notification (they start from 150)NOTIFICATION_AREAS.ERRORS
forareaSlug
SetupErrorNotification
from theassets/js/components/Header.js
Test Coverage
assets/js/components/notifications/ErrorNotifications.stories.js
QA Brief
Changelog entry