-
Notifications
You must be signed in to change notification settings - Fork 198
Fix: Remove CampaignWelcomeBanner script and update admin-notices package #8006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@JoshuaHungDinh what's the reason that we need JS to render the element? We've been able to display other notices that appear after the add plugins button so just curious why this isn't going into the expected area. |
@jon, It can be placed in a couple of locations through the package. The closest being after the h1 |
@JoshuaHungDinh ohh I see - so maybe that actually should be accounted for in the admin notices package because that's a common scenario and this doesn't quite work as expected. @JasonTheAdams what do you think about having the admin notice package take the "add plugin" button into consideration when choosing |
Ah yeah, sounds like an edge case AdminNotices isn't taking into consideration. I recommend making a patch in that package. I'll be happy to review the PR! |
Reference: stellarwp/admin-notices#14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Resolves GIVE-2599
Description
The current welcome banner is rendering immediately after the wp-header-end element in an attempt to fix a layout issue on the plugins page. However, since other plugins may also use an element with the same class, the banner can end up being injected into unintended areas of the UI, causing layout conflicts. This PR removes the related JS file that handles the replacement and its webpack config. The necessary changes to handle the placement have been made in the AdminNotices package & our composer files have been updated.
A hover state was also added the to main CTA of the banenr.
Affects
Campaigns Welcome Banner
Visuals
Testing Instructions
Pre-review Checklist
@unreleased
tags included in DocBlocks