Blocks: Update placeholders to use the withNotices HOC#14884
Conversation
|
Thank you for the great PR description! When this PR is ready for review, please apply the Scheduled Jetpack release: April 7, 2020. |
|
This looks good to me. I forgot to take a screenshot, but with Gutenberg 7.3 and WordPress 5.2 the notice was still indented. That's an odd combination though. Vanilla WordPress 5.2 looks like this: WordPress 5.3 looks like this: And with the latest Gutenberg, it looks as in the screenshots above. These all seem acceptable to me, and the indentation I was seeing with an older version of Gutenberg is probably not an issue. |
|
Should we also update the Eventbrite and Pinterest blocks? We should also update the block scaffolding... |
Done in #14895 |
Tracking this in #14896 |
| 'jetpack' | ||
| ) } | ||
| </> | ||
| const setErrorNotice = () => { |
There was a problem hiding this comment.
As I noted here, I wonder if we should extract this into a shared function maybe?
There was a problem hiding this comment.
I think this is a good idea, but lets do it in another PR. I have created an issue to track it here: #14899
|
Caution: This PR has changes that must be merged to WordPress.com |
|
r203925-wpcom |
* Initial changelog entry * Changelog: add #14904 * Changelog: add #14910 * Changelog: add #14913 * Changelog: add #14916 * Changelog: add #14922 * Changelog: add #14924 * Changelog: add #14925 * Changelog: add #14928 * Changelog: add #14840 * Changelog: add #14841 * Changelog: add #14842 * Changelog: add #14826 * Changelog: add #14835 * Changelog: add #14859 * Changelog: add #14884 * Changelog: add #14888 * Changelog: add #14817 * Changelog: add #14814 * Changelog: add #14819 * Changelog;: add #14797 * Changelog: add #14798 * Changelog: add #14802 * Changelog: add #13676 * Changelog: add #13744 * Changelog: add #13777 * Changelog: add #14446 * Changelog: add #14739 * Changelog: add #14770 * Changelog: add #14784 * Changelog: add #14897 * Changelog: add #14898 * Changelog: add #14968 * Changelog: add #14985 * Changelog: add #15044 * Changelog: add #15052 * Update to remove Podcast since it remains in Beta * Changelog: add #14803 * Changelog: add #15028 * Changelog: add #15065 * Changelog:add #14886 * Changelog: add #15118 * Changelog: add #14990 * Changelog: add #14528 * Changelog: add #15120 * Changelog: add #15126 * Changelog: add #15049 * Chanegelog: add #14852 * Changelog: add #15090 * Changelog: add #15138 * Changelog: add #15124 * Changelog:add #15055 * Changelog: add #15017 * Changelog: add #15109 * Changelog: add #15145 * Changelog:add #15096 * Changelog:add #15153 * Changelog: add #15133 * Changelog: add #14960 * Changelog: add #15127 * Changelog: add #15056 * Copy current changelog to changelog archive. * Clarify changelog description


Changes proposed in this Pull Request:
withNoticesHOC instead of manually handling the error notices.Pros: it's the Core Gutenberg way of handling placeholder notices; manual notices are incorrectly positioned on recent Gutenberg versions (see #14833 for a CSS-only fix).
Cons: notices created with
withNoticesare always dismissable, while all our manual notices aren't. I don't think it's a big deal to be honest.Note
Other blocks already use the
withNoticesHOC, but some in a slightly peculiar way.According to the Gutenberg docs,
withNoticesexposes anoticeUIprop that is passed to the Placeholder through thenoticesprop.E.g.
Some blocks instead use the
noticeUIoutside the Placeholder (the notice would appear above the block content—but still inside its container), while passing anoticesprop to the Placeholdernoticesprop.E.g.
jetpack/extensions/blocks/map/edit.js
Line 399 in 0fd1874
jetpack/extensions/blocks/map/edit.js
Line 492 in 0fd1874
I'm not exactly sure why this happens, and for example why this doesn't end up displaying two notices, one in the Placeholder, and one above the block content.
I've chosen to not touch these blocks, to avoid unexpected regressions.
Testing instructions:
Proposed changelog entry for your changes:
withNoticeshigher-order component.