-
Notifications
You must be signed in to change notification settings - Fork 798
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
Mailchimp Block: Remove wrapper from rendered markup #19639
Conversation
…up to fix CSS issue with upgrade nudges
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Jetpack plugin:
|
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.
LGTM -- good find, what a tricky one to reproduce!
Tested on a free site to reproduce the bug, and confirmed the fix.
Caution: This PR has changes that must be merged to WordPress.com |
Great news! One last step: head over to your WordPress.com diff, D60608-code, and commit it. Thank you! |
r224811-wpcom Also, thanks for renaming the PR, Jeremy, that reads much neater than my first attempt! 😀 |
I wouldn't have changed anything, it's just that the message needed to be shorter for the squash commit :) |
Related to: #19566 and #19481
The Mailchimp block currently adds a div wrapper
<div class="components-placeholder">
in the server rendered view that appears to not be needed. This wrapper only exists in the editor UI during the loading state or when a Mailchimp connection has yet to be set up. When it exists in the server rendered view, it doesn't cause issues most of the time. However, if any other code enqueueswp-components
, then CSS that targets thecomponents-placeholder
class will be loaded, which then causes an issue rendering the block. An example is that the upgrade nudges rendered on the front end for logged in users enqueueswp-components
here.The issue can be found if the following conditions are met:
I tested this within the site editor on a site running the TT1-blocks theme.
Changes proposed in this Pull Request:
<div class="components-placeholder">
from the server-rendered view of the Mailchimp blockNote: The files changed view for this PR seems to be struggling a bit with the change here — the change is actually just removing the containing
div
and removing a level of indentation, but due to the kinds of markup removed, the algorithm to display the changes isn't having a good time displaying the change cleanly!Screenshots
Note: these screenshots show the Mailchimp block rendered on the front end of a site while logged in and running the TT1-blocks theme. The styling issue to note here is the unintended white background and black border. The other issues (unstyled buttons) are being addressed separately. The upgrade nudge rendered beneath is for another block on the same page.
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No
Testing instructions: