-
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
Enable Block Renaming support for (almost) all blocks #54426
Conversation
I am unsure as to how this might impact accessibility. Specifically I notice that the paragraph block uses a custom label for the block when the context is |
I think we may need a new API to retrieve the block's custom name. Sprinkling |
Size Change: -37 B (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
Could you explain this part more? I'm unfamiliar with it. Could you point me in the right direction of how to look into this? Thanks, Dave! |
👍 It's here gutenberg/packages/block-library/src/paragraph/index.js Lines 30 to 41 in eabcc99
|
I love this. @WordPress/gutenberg-design which blocks should support manual labeling in the list view? All of them? @getdave I think not having another abtractisation is actually good because this is an override that feels natural to read in code as this is coming from this other place, instead of magicSpell() -> label. |
If we're going to make this available to all blocks maybe we don't need a block support at all. |
I mentioned this on #50135 (comment), but I did expect the placeholder text to be the heading block's auto-label (whatever the content is for the heading block) instead of maintaining the block name. Flow:
|
Is there a case for why we wouldn't make this globally available? |
Wrapper blocks - template parts, patterns...etc. It might be unusual if you could rename these? How would we like to handle those? |
@richtabor I suppose the question is whether renaming a synced block in a local context should create an alias or update the source. Updating the source seems fine to me, probably with a Snackbar confirmation to clarify the change was global. |
I think updating reusable / synced elements global names should not work from the list view. The list view is local - it would be OK in my head if reusable / synced elements would have their global label and a local label. If you want to change the global label a modal action is better IMO so it signifies the impact of the action. The idea is to not rename Testimonials synced pattern to talking-heads when building a layout and then panic in the patterns library that you lost Testimonials b/c you didn't see the snackbar. |
That's part of the issue, renaming from list view invokes a modal. So in the case of synced blocks it might not be clear whether you're updating globally or adding a local alias, especially as Rename actions elsewhere are global. I suppose the first question to answer is whether or not to allow aliases for synced blocks. Partially synced should be considered too. It might be better for that discussion to happen elsewhere to avoid holding up this PR? |
I lean towards local-only as well. The other rename flows are modals as well, but it's clear that you're renaming a pattern/part — rather than a block name/list view. But I'm ok omitting template parts and synced patterns from rename. This traces back to the notion of whether you're affecting every instance of it or not (like invoking a focus/isolated view). |
So the consensus is make it conditionally available to all blocks save for template parts, patterns...etc? |
I'd lean towards omitting.
Seems fine to look into this if a need arises, rather than introducing potential headaches ahead of time. |
Ok now on to the technical question of how best to do this. Currently support for the feature rests on the block adding support for We could:
Any prior art for such a thing? |
I think @scruffian 's comment above makes sense, in that if all blocks support rename, it's not an opt in and therefore it's not required to create a new block support. So we only need to make sure all blocks work - by default - with metadata and they all have the name metadata available. |
|
Hi folks, it looks like this PR introduced a severe performance issue for the "block selection" metric. See https://www.codevitals.run/project/gutenberg |
Let's roll back now and then take a look at which bit is causing that. |
I have confirmed offending code is |
Outstanding WorkThe following needs to be undertaken in the WP 6.5 cycle.
Updated - I created an Issue |
@ockham Looks like all the work from this PR was backported to Core already in...
Am I right? |
Yes! AFAICS, the only part that needed backporting was the It might however make sense to move the GB "polyfill" from |
I think that's a good point and should be done for all the php code that is backported. |
✅ I updated this PR with the |
✅ Completed |
Dev noteBuilding on the foundation introduced in WordPress 6.4, all blocks (with some exceptions) can now be given custom names within the Editor. The data for this continues to be stored under the All blocks (including 3rd party blocks) will be "renameable" by default, with the only exceptions being the following blocks:
To disable the ability to rename a block, set the relevant supports property to // block.json
{
"supports": {
"renaming": false // disables ability to rename block via the Editor UI
},
} |
What?
Following on from #53735, this PR enables the ability to rename all blocks by default, with the exception of the following blocks:
core/block
core/template-part
core/pattern
core/navigation
Why?
Because it was suggested we should open up this feature.
How?
__experimentalMetadata
support is removed).__experimentalBlockRenaming
block supports.false
.Testing Instructions
Rename
.Testing Instructions for Keyboard
Screenshots or screencast
Screen.Capture.on.2023-09-22.at.13-56-24.mp4