-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Additional lock options for template_lock
option with block templates
#5208
Comments
Does it make sense to propose a "remove" level and an "insert" level knowing that if you insert blocks, it makes sense to me to be able to remove them and the opposite is true? |
I'm sorry I'm not sure I'm understanding your question and maybe your question is due to a misunderstanding of the proposal in the issue. Currently the documentation has this as the options for template_lock:
My interpretation of the documentation is that either currently template_lock option prevents removal of blocks or the insertion of new blocks, but the The problem here is the language of the options (assuming my interpretation of the documentation is correct). In part, what I'm proposing has two levels: At the global template level:
At the template block level (blocks that are a part of the template only):
I think your question is under the assumption that Does that help clarify things? |
You're right, I misunderstood the issue originally. My bad. Right now, there's no way to say that a block was part of the template or not, and my personal opinion at the moment is that adding this complexity is not worth-it. Technically, this is way too complex to implement, it requires constant matching between templates and blocks (and this would always be error prone) because the template do not identify the blocks and saving a post don't identify which blocks come from the template or not. |
The template blocks are registered right now as a part of the template feature? So technically, that is known. Are you opposing the entire proposal or just the per block template lock settings? It appears you're mainly opposed to the per block block template settings (but haven't really addressed the first item which is "1. Change lock state to be accumulative instead of switch." which is a change at the global template_lock setting level). With this first part of the proposal, I think we at least need a way to have a template defined default blocks that are included, but still provide some flexibility in how those blocks behave at the template level (can/cannot move, can/cannot remove, can insert new blocks that aren't a part of the template). Otherwise the template loses some of its flexibility. As far as per block template settings go, let's look at removal controls for instance I see that the removeBlock control is being composed via the editor context and thus is only receiving the I realize there's complexity involved but I do think there are going to be cases where plugin/theme authors will want/need this functionality. At a minimum, whether its via template settings or individual block registration props, blocks need to be able to have some control over whether they can be moved or removed once they are added to editor by a template. Let me describe a scenario (which I just alluded to in the proposal) that might help highlight why I created this issue. I'm a developer on the team that does the Event Espresso plugin and currently our plugin uses the classic editor interface for event authors to create/edit their events. A significant part of this interface is the creation of dates and times for the events along with tickets/prices etc. via a metabox. The data model is that datetimes are saved to their own table and related to events and tickets are saved to their own tables and related to datetimes. The data model requires that every event has at least one datetime and one ticket. One way I'm considering we could integrate with gutenberg is the ticket editor could be a block. We'd have a default template for the event post type that inserts a Where this breaks down is:
Anyways, in part, this proposal is because I do see potential use-cases for it, but its also part of a discovery process in determining what approach we'll be able to take with integrating EE into Gutenberg. Obviously if the gutenberg team decides this proposal would make things too complicated and fragile and do not agree that there's sufficient use-case for the functionality, then we'll have to consider some other approach to do what we need in order to integrate. |
No, it's not because of the fact that you can insert/move blocks you don't know what blocks are part of the template or not unless you try to synchronize the template with the blocks constantly.
I'm not opposing the per block template lock settings but I'm opposing the way it's proposed here, I think all the usecases for this settings can be overcome using the nested lock settings we're exploring. You can lock everything and if you want an area to be "unlocked", you use a nested container block with an unlocked template.
I'm not restricting this point neither I'm saying it's already accumulative in a way:
I'm not denying the use-case, I think it's legit, but I think it can be overcome using nested blocks locking (which is yet to come) without adding this granularity which creates complexity. And I'm not entirely against adding a per block lock for some features like "this block can't be removed" but I prefer to wait until we get the nested locking working and see its concrete limits. |
So just reviewing a bit of the code: I see here that the export function BlockRemoveButton( { onRemove, onClick = noop, isLocked, small = false, uids = [] } ) {
const isBlockLocked = () => {
if (isLocked) {
return true;
}
return uids.some(function(uid){
//new method exposing whether the block instance exists in the store containing references to template settings
//for blocks created on load.
if (templateHasBlock(uid)) {
//new method retrieving the settings object for the block in the template block settings store.
{ blockLock } = getTemplateBlockSettings(uid);
if (blockLock === 'remove') {
return true;
}
}
return false;
});
};
if ( isBlockLocked() ) {
return null;
}
const label = __( 'Remove' );
return (
<IconButton
className="editor-block-settings-menu__control"
onClick={ flow( onRemove, onClick ) }
icon="trash"
label={ small ? label : undefined }
>
{ ! small && label }
</IconButton>
);
} |
sorry I was working on the code example before I saw your reply :) |
And actually, in looking at the existing selectors. I see there already is a |
By the way, I realize the team has an identified list of things they are working on. I'm willing to give a go at implementing this if there's openness to the idea. But of course I don't want to spend the time if its a definite no-go for this approach :) |
It's definitely not a no-go from me, I just like iterative approaches :) especially when introducing complex features like this. Also, opinions are my own and might be good to have other opinions here @mtias @aduth We iterated like this: 1- global templates At this point it would be good to make sure that these features are not sufficient to address the different use cases. Potential future steps could include what this issue suggests, per block locking. |
Also, you may want to check this PR #5162 to understand what I mean by the complexity of matching templates and blocks. |
Oh I see what you are referring now when you are talking about "matching templates and blocks". You are referencing how template definitions might change between when a post was first created with a template (and saved) and when its loaded after the shape of the template has changed. This also applies to if someone registers a template for a post type that includes plugin blocks but that plugin is no longer active when the post content is loaded. So ya, with that consideration, I think we may want to keep global template state pretty much as is (but nested blocks could provide the ability for unlocking the "editor" instance in a nested block so new blocks can be added). However, I think having per block locks is still relatively manageable outside of the templates. If I get some time this weekend I may work on experimenting with per block locks as described above and see how it goes. |
I think this will prove to be very complex to implement and I'd prefer diving into this when there is a demonstrable use-case for implementing. |
I have what I think is a use-case for these options, unless someone has an alternative use: I have a single post type to generate user profile pages. In each page I have some custom meta fields ( photo, name, job_title, introduction). I have added these to my custom post type via functions.php and I have added a template to add my "intro" block to the post page by default. My intro block has variables that are set to use the new meta fields via source: "meta". This all works well, except for the fact that the admin can simply remove the block. Using 'template_lock' allows me to stop users removing it, but then they are unable to add any new blocks. |
This is now partially addressed by #32457 |
Issue Overview
The current state of the template feature (see #3588) has two options for locking the template which are controlled via the
template_lock
option with the template registration:all
: which locks the position of all blocks and prevents any insertion of new blocks.insert
: which allows the blocks in the template to be moved around but no insertion of new blocks.This issue is a request/proposal that additional iterations of the template feature include the following changes.
1. Change lock state to be accumulative instead of switch.
Instead of
all
orinsert
. Allow the lock types to be accumulatively applied and describe what specifically is locked. So you'd have something like this for the options:insert
: restricts any new blocks from being addedremoval
: restricts blocks in the template from being removed.move
: restricts blocks in the template from being moved.So if someone wanted to have the template default to the current blocks but still allow them to be moved and removed but not allow the insertion of new blocks they could do something like this:
If we wanted to prevent insertion of new blocks and the removal of existing blocks but still allow existing blocks to be moved around we'd do:
2. Allow block specific locking.
With this, we'd allow specific blocks to be locked. In this case they'd only have two
template_lock
options:removal
, ormove
. This way authors of templates could choose to restrict only a subset of blocks in a template. If they had the blocks restricted from moving, then that would just mean that in relation to all other blocks in the template that are also locked from moving, the locked block would keep its position.Why?
One use case I've been thinking of for a plugin is where a template defines the following:
Related tickets
Todos
The text was updated successfully, but these errors were encountered: