-
Notifications
You must be signed in to change notification settings - Fork 808
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
Change the selector to target blocks that have a warning class #14732
Conversation
Caution: This PR has changes that must be merged to WordPress.com |
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: March 3, 2020. |
I haven't had the chance to look at that yet, but in this case I'd probably note here that this PR is dependent on that other one. |
As the HOC was used as part of a filter, the className was getting set by the last filter that was run. This was a lucky accident for OpenTable as it meant that it wasn't affected by the bug being fixed in #14732 This change passes through the className prop when the block names don't match, which means that if it's been set by a previous filter, it is preserved.
It's being fixed by #14730! This does fix the problem and I think it was caused by the wrapper div that was removed in WordPress/gutenberg#19593 I've discovered the reason OpenTable wasn't affected by this, and fixed it in #14738, but this should be merged first |
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.
Do you think you could rebase this PR so we can test it with the fix that has now been merged?
Thank you!
1bc4bc7
to
9606e68
Compare
Rebased ✅ |
scruffian, Your synced wpcom patch D39099-code has been updated. |
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.
Works for me! ✨
Unrelated to this PR, I've noticed that, when unavailable, the block outputs just a link.
Anchors being inline elements, they might not respect some theme styles setting the blocks' width and margins.
I've incidentally tried this diff on a site using the Maywood theme and it looks like this:
The green box is the intended post content width (set with max-width
and margin: auto
), while the red box with the Calendly link is the screen's 100% width.
I think we should do something like this 🤔
{
save: () => (<div><a href={ url }>{ url }</a></div> ),
}
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.
Works for me. I was able to add a calendly URL and see the correct embed. I deselected the block and reselected and was able to change all settings and interact with the block.
As the HOC was used as part of a filter, the className was getting set by the last filter that was run. This was a lucky accident for OpenTable as it meant that it wasn't affected by the bug being fixed in #14732 This change passes through the className prop when the block names don't match, which means that if it's been set by a previous filter, it is preserved.
r203259-wpcom |
Dependent on #14730
Changes proposed in this Pull Request:
with-has-warning-is-interactive-class-names
component, so that they still target the block in edit mode.Is this a new feature or does it add/remove features to an existing part of Jetpack?
Testing instructions:
Proposed changelog entry for your changes: