Enhancement/9071 Refactor Banner Notification Component

[jimmymadon]

Summary

Addresses issue:

• Refactor the GatheringDataNotification to use a newly refactored version of the BannerNotification component. #9071

Relevant technical choices

• I have only included components and props that are absolutely essential for the GatheringDataNotification. As we continue to refactor, we can add more props to existing notifications or decide to create new components completely. This way we won't add anything that isn't being "used" and we won't over-engineer for the future.

PR Author Checklist

• My code is tested and passes existing unit tests.
• My code has an appropriate set of unit tests which all pass.
• My code is backward-compatible with WordPress 5.2 and PHP 7.4.
• My code follows the WordPress coding standards.
• My code has proper inline documentation.
• I have added a QA Brief on the issue linked above.
• I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

• Run the code.
• Ensure the acceptance criteria are satisfied.
• Reassess the implementation with the IB.
• Ensure no unrelated changes are included.
• Ensure CI checks pass.
• Check Storybook where applicable.
• Ensure there is a QA Brief.

Merge Reviewer Checklist

• Ensure the PR has the correct target branch.
• Double-check that the PR is okay to be merged.
• Ensure the corresponding issue has a ZenHub release assigned.
• Add a changelog message to the issue.

[github-actions]

Build files for 163da6a have been deleted.

[eugene-manuilov]

Mostly looks good to me. Left one tiny comment.

[aaemnnosttv]

Chiming in with just a few observations but leaving the review to @eugene-manuilov

[aaemnnosttv]

Let's delegate this to the layout components. As a general rule, we should avoid splitting grid components between different components. This also gives the most flexibility to the notification and avoids the need to later add props for passing to the grid at this level.

[aaemnnosttv]

How about we extract some reusable components here? Maybe NotificationTitle and NotificationDescription?

[eugene-manuilov]

Ok, two last comments from me and it should be good to go.

[eugene-manuilov]

LGTM

[eugene-manuilov]

@jimmymadon, not sure why but VRT images check still doesn't work after I have restarted it 3 times. Can you check what is going on with it?

[jimmymadon]

@jimmymadon, not sure why but VRT images check still doesn't work after I have restarted it 3 times. Can you check what is going on with it?

I've got 25 test failures on my local that are unrelated to this PR. Should we create a separate issue to fix those?

[eugene-manuilov]

@jimmymadon, not sure why but VRT images check still doesn't work after I have restarted it 3 times. Can you check what is going on with it?

I've got 25 test failures on my local that are unrelated to this PR. Should we create a separate issue to fix those?

Hm... I have just merged #9193 and its VRT task went well: https://github.com/google/site-kit-wp/actions/runs/10401785631/job/28808369192. Not sure why it doesn't work here... Perhaps ask @zutigrm to help you troubleshoot that?

[jimmymadon]

@eugene-manuilov After merging develop one more time, the VRTs all seem to pass now. But I'm pretty sure they are still unstable and @zutigrm has created a new issue as per this thread to investigate the old fix further.