Skip to content

Update block scaffolding to use the withNotices HOC#14895

Merged
scruffian merged 2 commits intomasterfrom
update/block-scaffolding-notices
Mar 5, 2020
Merged

Update block scaffolding to use the withNotices HOC#14895
scruffian merged 2 commits intomasterfrom
update/block-scaffolding-notices

Conversation

@scruffian
Copy link
Copy Markdown
Member

In of #14884 we realised we were doing notices wrongly. We should update the block scaffolding to do it the right way.

Changes proposed in this Pull Request:

  • Update the block scaffolding to use the withNotices HOC

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • change to existing feature

Testing instructions:

  • Run yarn docker:wp jetpack scaffold block "Cool block"
  • Open the editor
  • Try adding a "Cool block" to the editor.
  • Check that the block loads correctly

Proposed changelog entry for your changes:

  • no changelog

@scruffian scruffian added [Status] Needs Review This PR is ready for review. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Mar 5, 2020
@scruffian scruffian requested review from a team March 5, 2020 13:00
@scruffian scruffian self-assigned this Mar 5, 2020
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Mar 5, 2020

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 7, 2020.
Scheduled code freeze: March 31, 2020

Generated by 🚫 dangerJS against 4bee796

Comment on lines +24 to +27
const setErrorNotice = () => {
noticeOperations.removeAllNotices();
noticeOperations.createErrorNotice( __( 'Put error message here.', 'jetpack' ) );
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of me wonders if we shouldn't extract this into a shared function available to all blocks? Ideally, that function could also handle all kinds of notices, not just erors. Something like this:

export function displayNotice( message, noticeOperations, type = 'error' ) {
	noticeOperations.removeAllNotices();
	noticeOperations.createNotice( { status: type, content: message } );
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might be worth trying in some blocks, but I'd rather we only move stuff into the scaffolding when we've already had success with it in other blocks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I suggested we give that a try in #14884 (comment)

Until then, we should be able to merge your PR and revisit once we have some blocks using it.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Mar 5, 2020
Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu>
@jeherve jeherve added this to the 8.4 milestone Mar 5, 2020
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! [Type] Janitorial and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 5, 2020
@scruffian scruffian merged commit 3627e15 into master Mar 5, 2020
@scruffian scruffian deleted the update/block-scaffolding-notices branch March 5, 2020 16:31
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Type] Janitorial

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants