Ensure consistent header and sub header spacing between block and shortcode output #41
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g inconsistencies between the shortcode and block output. Add a top margin to match the bottom margin of the sub header
…lds don't exist. This prevents an extra layer around a normal widget
noraconley
reviewed
Jun 28, 2024
Collaborator
noraconley
left a comment
noraconley
left a comment
There was a problem hiding this comment.
@dkotter This works really well. Just one question around a possible edge case.
mailchimp_widget.php
Outdated
| echo wp_kses_post( $after_widget ); | ||
| if ( ! empty( $after_widget ) ) { | ||
| echo wp_kses_post( $after_widget ); | ||
| } else { |
Collaborator
There was a problem hiding this comment.
I'm wondering if we should check for before_widget for the closing</div>, in the rare case that these args are being used for self-closing tags?
Collaborator
Author
There was a problem hiding this comment.
Yeah, that's a good point. There may be a scenario (hopefully edge case) where $before_widget is empty but $after_widget isn't (or vice versa) and we could end up having an opening tag with no closing or closing tag with no opening. Just pushed a fix that I think accounts for this: e9a2dc7
noraconley
approved these changes
Jun 28, 2024
Collaborator
noraconley
left a comment
noraconley
left a comment
There was a problem hiding this comment.
Your latest change handles that edge case well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the Change
As described here, there's a slight difference in spacing when using the shortcode block vs the normal block. This will vary based on the theme you're using, as this spacing isn't something we are adding. But in testing some of the core themes, what's happening here is the shortcode output does not have a wrapping container around things and there's some styling that gets applied to all direct children of the main content area.
This PR fixes things by adding in a wrapping container if a custom container isn't being used (when used as a normal widget in a widget area, there is already a wrapper so we don't want to double wrap). This removes this extra spacing. We also are adding some top margin on the sub-header to match the bottom margin that already existed there.
Partially closes #37
How to test the Change
Changelog Entry
Credits
Props @dkotter, @qasumitbagthariya
Checklist: