-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Use a modal for the reusable blocks creation flow #29040
Conversation
Size Change: +1.19 kB (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
Good catch @mtias it should be fixed |
} } | ||
> | ||
<TextControl | ||
label={ __( 'Name your reusable block' ) } |
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.
This could be just "Add name" or "Name"
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.
Sure, I just used what was shown on the issue's video :)
It is not as the PR currently stands: createtp.mp4But I think it would be good to keep these flows consistent. |
Agreed, the idea is that this PR is focused on the reusable block UI. Once it lands, the same flow will be applied to the template part as well. It's better to keep PRs small and focused to ease reviews... |
Not directly related to this PR but noticed while looking at some string changes here. Language for reusable blocks feels a bit inconsistent. How about:
— or maybe better with lowerkey r, "reusable block"? As long as it's consistent. |
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.
Thanks for woking on this Riad!
I tested and code wise looks good. I left a note for the labels thing, but we could certainly handle this in a follow up, if this decision to change all labels feel like it needs to be discussed more 🤔 .
@@ -260,7 +235,7 @@ describe( 'Reusable blocks', () => { | |||
// Save the reusable block | |||
await page.click( publishButtonSelector ); | |||
await page.waitForXPath( | |||
'//*[contains(@class, "components-snackbar")]/*[text()="Reusable Block updated."]' | |||
'//*[contains(@class, "components-snackbar")]/*[text()="Reusable block 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.
This breaks due to this: https://github.com/WordPress/gutenberg/blob/master/lib/compat.php#L266
Since we make blocks
with a lower b
, should we change the other Labels as well?
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.
It seems there's an inconsistency between Core and Gutenberg. Core menus favor capitalizing everything and Gutenberg the opposite. This is one area where the same string can be used in two different places. The issue is that at some point the guideline here should be consistent between Core and Gutenberg but it's a broader issue.
This is what I see: Creating-Reusable-blocks.mp4Should the drop down seen in the background close before the modal is seen? Btw |
Yes, the dropdown should stay open in case you cancel, the focus is moved back there. |
Late to the party but very happy to see this; thanks @youknowriad ❤️ |
closes #12940
Implements the flow discussed in #12940 for reusable blocks.
This new reusable block creation flow has three advantages:
untitled-reusable-block-xxx
This flow is likely to be applied to template parts as well.