Skip to content
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

Add new mode to contentOnly locking to allow insertion of new inner blocks #52018

Open
Tracked by #60021
fabiankaegy opened this issue Jun 28, 2023 · 14 comments
Open
Tracked by #60021
Labels
[Feature] Block Locking The API allowing for the ability to lock/unlock blocks [Type] Enhancement A suggestion for improvement.

Comments

@fabiankaegy
Copy link
Member

fabiankaegy commented Jun 28, 2023

contentOnly locking is super powerful. There are some instances though, where actual inner blocks are used for content. An example of this is the core list or also the core buttons block. It would be great to be able to say that editors should be able to add additional list items or additional buttons even when the container block has it's templateLock set to contentOnly

CleanShot.2023-06-28.at.10.41.03.mp4
@fabiankaegy fabiankaegy added [Type] Enhancement A suggestion for improvement. [Feature] Block Locking The API allowing for the ability to lock/unlock blocks labels Jun 28, 2023
@fabiankaegy
Copy link
Member Author

@noisysocks @jorgefilipecosta Do you maybe have any insights as to how difficult it would be to allow for this kind of behavior in the content only locked mode?

@noisysocks
Copy link
Member

noisysocks commented Aug 7, 2023

@noisysocks @jorgefilipecosta Do you maybe have any insights as to how difficult it would be to allow for this kind of behavior in the content only locked mode?

The difficulty is in designing a coherent API for extenders 😀

I think it might be better to move away from "templateLock": "contentOnly" and instead have something that controls the block editing mode via block attributes. The editing mode is inherited from the parent which means you can disable a container and then re-enable specific children.

@bvlgn
Copy link

bvlgn commented Aug 16, 2023

This request makes so much sense IMHO

To me this sound as a must have to make the content only mode really usable.
Users do expect that they can add and remove (content only) list items.
The same will possibly be true for other collection type (custom) blocks.
I would love to see an API that allows to set this behaviour as an option or even the default in some cases like mentioned by fabiankaegy.

I can also imagine that we would allow users to add/move/remove arbitrary blocks to e.g. a group which has contentOnly set. All blocks that would be added would of course run in contentOnly mode.

Maybe we could introduce an attribute that overrides the default behaviour of contentOnly.
These defaults are now:

  1. every child will run in contentOnly editing mode
  2. inserting of new child blocks isn't allowed
  3. no moving of child blocks
  4. no removing of child blocks

2,3 and 4 should be able to be overriden using e.g. a templateAllow attribute, with (combinable) values:

  • insertDefault - allow inserting and moving of the default child blocks (content only editable)
  • insertAny - allow inserting and moving of any child blocks (content only editable)
  • move
  • remove

Further more I would love to see the disabled option also added to templateLock, just like with useBlockEditingMode.

🤞

@fabiankaegy
Copy link
Member Author

CC: @annezazu @SaxonF Tagging you here because we talked about this in the WordPress 6.5 Hallway hangout yesterday. I believe this is a major limitation of the contentOnly locking which I fear will cause us a larger headache down the road with the way Pattern Overrides are currently implemented and slated to go into 6.5: #53705

@fabiankaegy
Copy link
Member Author

Also tagging @talldan to ensure you saw #52018 (comment) as it relates to the block connections work :)

@bacoords
Copy link
Contributor

bacoords commented Apr 4, 2024

+1 to this, especially where a block is just a wrapper for a specific single inner block (Button, List, Gallery, (Columns is a weird one)).

Some ideas:

  1. We could start with a parameter on <InnerBlocks /> that allows you to override this in custom blocks: inserterWhenLocked={true}
  2. The templateLock attribute itself could be expanded to take an object of values. So instead of contentOnly, if you could say templateLock: { content: true, inserter: false, move: false, delete: false } and "contentOnly" would just be a shorthand for a default set of configurations.
  3. We add a complement to the templateLock attribute that is not for the wrapping block but for any blocks inside of a content lock. So if the outermost block has templateLock:"contentOnly" you could go inside and modify a button's block attributes with some sort of templateLockOverrides:{inserter: true} or something.

Partially synced patterns are going to need to consider the use case for repeatable elements. There are plenty of synced patterns that might want to have some flexibility in the number of buttons, bullet points, etc. And that's not even including more complicated use cases where you might want a user to be able to duplicate/delete entire columns with blocks inside- instead of having to make separate pattern designs (Pattern with three testimonial block quotes, Pattern with four testimonial block quotes, etc).

@SaxonF
Copy link
Contributor

SaxonF commented Apr 4, 2024

Apologies for missing this @fabiankaegy ! I regularly come across the need to add/remove elements from a repeatable container, even in other tools like Figma. I touched on this a little bit in the [advancing content only issue ].

It would also be nice to mark blocks that can easily be toggled on/off eg I may not need a subheading from a pattern

@fabiankaegy
Copy link
Member Author

@SaxonF Yeah the ability to toggle on / off something is a common need.

I today removed content only locking from a pattern because the client sometimes needs to remove a CTA button from that section. And that's just not possible currently.

Long term I think it would also be super cool to allow for users to switch between color schemes like light & dark of a pattern. Like described in the section specific theme.json task.

This all is why I'm really hoping we build pattern overrides in a way where we can make it really granular :)

@talldan
Copy link
Contributor

talldan commented Apr 30, 2024

I thought of a potential technical solution for this in terms of pattern overrides.

Right now pattern overrides work by 'bubbling' the content attribute values of inner blocks like paragraph and heading to the parent pattern block where they're stored in a metadata.content attribute.

Potentially we could do the same for lists, with the wrapper blocks (list, gallery, buttons) also working the same way. They have a metadata.content attribute and the values from their children bubble up into an array. In turn those parents also bubble the array value up to the parent pattern block. Possibly it would only work like this when the parent block has a binding of some kind.

This might work ok for container blocks that only support one inner block type, the list items are interchangeable, so the position of the data doesn't matter so much.

cc @SantosGuillamot @gziolo @kevin940726 - would be interested to hear your thoughts.

@talldan
Copy link
Contributor

talldan commented Apr 30, 2024

On writing it out, just realized one potential issue with a solution like this is that we need the markup of the inner blocks (e.g. a list item) to be able to carry out the dynamic injection of values on the server 🤔

Potentially you could take the first inner block and replicate it, but a limitation would be needing at least one inner block defined in the pattern.

@fabiankaegy
Copy link
Member Author

@youknowriad @mtias @richtabor Coming back to our conversation earlier today. I do think there may be merit to the idea of introducing a new templateLock option that would open up the more rich editing experience for the items within.

We currently have:

  • false
  • all
  • insert
  • contentOnly

all and insert can always get overwritten by nested blocks. Content only is the only one that doesn't allow for this.

Why not introduce a new mode or use one of the existing ones to allow opening up editing of descendants again?

We could add that templateLock="nonContent" would allow insertion, deletion, and moving for that block list again.

Paired with allowedBlocks this can restrict either the list/social-icons/buttons blocks to allow insertion of additional items within. But also in theory get used on a group to allow more general content to be inserted again.

@talldan
Copy link
Contributor

talldan commented Dec 18, 2024

I've referenced this issue for the various blocks this affects in #65778.

I did start exploring this in #65836, but there were more pressing things at the time. Not sure if it still works, I remember hitting on some writing flow issues that I wasn't sure how to solve.

We could add that templateLock="nonContent" would allow insertion, deletion, and moving for that block list again.

Paired with allowedBlocks this can restrict either the list/social-icons/buttons blocks to allow insertion of additional items within. But also in theory get used on a group to allow more general content to be inserted again.

Could it also be the other way around, that insertion is allowed by default for these kinds of blocks, and then templateLock: insert prevents it?

I think there are likely to be more cases where that's a good default - e.g. users don't want to be stuck with the default provided social icons in a contentOnly pattern.

@fabiankaegy
Copy link
Member Author

@talldan Thanks for linking that. I can certainly help with testing / light reviews :)

Something that has me a bit concerned at the moment is that it seems #65204 (comment) introduced a workaround that patches this behavior, particularly for the core list block in the global write mode.

I mean, it's great to see that the list block can accept new list items in this mode and remains selectable. But I worry that this is not thinking about the issue wholistically and is setting a bad precedent.

Could it also be the other way around, that insertion is allowed by default for these kinds of blocks, and then templateLock: insert prevents it?

I think there are likely to be more cases where that's a good default - e.g. users don't want to be stuck with the default provided social icons in a contentOnly pattern.

I think the only argument against that I can come up with is backwards compatibility. Other than that, I would agree that it would probably be the better default.

@talldan
Copy link
Contributor

talldan commented Dec 18, 2024

I mean, it's great to see that the list block can accept new list items in this mode and remains selectable. But I worry that this is not thinking about the issue wholistically and is setting a bad precedent.

Ah, ok, I think I know what you mean. I noticed that as well, though I don't think it's related to the code you mentioned?

I'm pretty sure that PR only moved that line of code, I left a comment here to show where it was originally:
https://github.com/WordPress/gutenberg/pull/65204/files/a783818c3205e9ad45b8f508e3f3bccb754bc256#r1889776195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block Locking The API allowing for the ability to lock/unlock blocks [Type] Enhancement A suggestion for improvement.
Projects
Development

No branches or pull requests

6 participants