Patterns: disable editing of synced patterns in post editor unless children have connected content set
#56574
Conversation
|
Size Change: +371 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
The problem with setting the pattern to disabled with Actually ignore the above for now - I have a better plan |
|
Keen to hear what the plan is. I guess an option if disabling the pattern block container itself is too much is to instead disable each of the top-level inner blocks that make up the pattern. That should stop the pattern from being editable while still allowing interactivity with the pattern block. |
glendaviesnz
changed the title
|
@talldan I just pushed a change that gets around the issues caused by setting |
| @@ -2928,6 +2928,19 @@ export const getBlockEditingMode = createRegistrySelector( | |||
| if ( ! clientId ) { | |||
| return 'default'; | |||
| } | |||
There was a problem hiding this comment.
If we go with this approach we probably just need to abstract this check out into its own method maybe.
There was a problem hiding this comment.
I guess an alternative to modifying this selector could be to add some code to the pattern block that:
- Gets a list of inner blocks of the pattern.
- Iterates through each one calling
setBlockEditingMode( clientId, 'disabled' | 'contentOnly' )depending on the presence of a connection.
Having said that, probably best to try to see whether there's a way to solve the issue with the sibling inserter first. If that's not possible, probably have to go back to the idea of adding a new editing mode or introducing a way to make the modes more granular.
|
One thing I need to tidy up is the in-between-inserter which is still displaying as the parent block is not disabled. |
|
Flaky tests detected in e73f531. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7217594435
|
glendaviesnz
added
[Type] Enhancement
| @@ -38,6 +45,15 @@ import { unlock } from '../lock-unlock'; | |||
| const { useLayoutClasses } = unlock( blockEditorPrivateApis ); | |||
| const fullAlignments = [ 'full', 'wide', 'left', 'right' ]; | |||
|
|
|||
There was a problem hiding this comment.
The following method should also check for block support of __experimentalConnections but for ease of testing without having to merge with connections PRs just checking for attributes for now.
|
@talldan fixing the in-between inserter was easy, and this approach is much tidier if handled in the pattern block rather than the selector as you suggested, so have modified to do this. I think there is still a bit of tidy up that can be done, but if you have time to give it a test it would be good to get another opinion on how sound the approach looks now before I finish this off. |
|
@SaxonF we probably need some good communication around this if we go forward with this approach as it is a reasonably big change to the default edit interface of a block that has been around for a long time. The one thing that I think is still missing is an easy way back from the standard pattern editor if a user/theme does not have site editor access. If you log in as an author user and add a new pattern then click Edit in the pattern toolbar to see the flow. The browser back button works as expected, but I wonder if it needs something like in the site editor header? |
|
There will be a few e2e tests to fix once we have finalised the approach here. |
|
One question is whether we want to merge this into trunk independent of the partial syncing work so people get used to the fact that the full source pattern editing location is changing ahead of the partial syncing being available. Or put it behind the connections experimental flag so the change in editing location can be released in tandem with the partial syncing? |
|
This is working well in testing for me. I had a (partially) synced pattern with connections left over from testing #56495, and I confirmed that I could still edit the paragraph text, so that seems to be working really nicely. I think there would still be a task to add something like this to the Inspector (like there is when editing a page in content mode in the site editor), but it could be a follow-up PR: For Fully synced patterns (with no connections) the entire pattern wasn't editable. It's nice that List View doesn't show the inner blocks (I guess it doesn't need to, so it's great to keep things concise). I did notice the pattern could still be renamed in the inspector, so that might be something to remove, and at the same time make sure patterns can be renamed in the pattern editors. |
| innerBlocks.forEach( ( block ) => { | ||
| const editMode = isPartiallySynced( block ) | ||
| ? 'contentOnly' | ||
| : 'disabled'; | ||
| setBlockEditingMode( block.clientId, editMode ); | ||
| } ); |
There was a problem hiding this comment.
May need to do this recursively for 'contentOnly' as a content block could be nested in a group 😓
There was a problem hiding this comment.
ah, true, done.
|
@talldan the editable content should now display in the right inspector panel for patterns, the same as for
have removed that as it can be renamed both in site editor and wp-admin |
glendaviesnz
changed the title
connected content set
I wonder if we should consider having a way to edit it directly in the editor view, as otherwise users have to jump around a bit. @SaxonF Would be great to get your thoughts on that. |
|
Thinking about this more thanks to the prompting of this larger convo, I wanted to share some of where I think I’m getting stuck:
We always aim to be iterative so sharing this to aid that. |
|
Great discussion, thanks for the input everyone, really helpful. We will take all that input and see what we can come up with as the best way forward for this. It will be at least a week before we get back to this due to some other priorities. Feel free to add any other ideas you might have in the meantime. |
|
(The below comment might be outside of what is meant for this specific PR.) Actually this will be very helpful when adding a prebuilt full page/CPT pattern into the page/post/CPT. One will need to define which blocks can be: moved and/or edited. As a pattern there might be block areas with images, heading text etc that can only be moved around and not edited. Adding in a link to a discussion I made on the Github repo for WooCommerce. |
|
Thank you for your work on this @glendaviesnz!
@talldan - This is one of the UX challenges I keep going back and forth on. As an agency building sites for customers with brand guidelines or design systems, this is amazing. It also feels like a good foundation for partially synced patterns. On the flip side, as a person with my own single-user sites, I just want the freedom to edit the styles and content. I still have a dream of someday having an
@glendaviesnz - Unless we're releasing partial syncing in the next release, I like the idea of it being released independently. This would allow for extended user testing and feedback. We may also uncover additional gotchas like @annezazu is surfacing with focus mode. |
… the pattern block mode
…efault with partial syncing
glendaviesnz
force-pushed
the
try/disable-synced-pattern-edit-2
branch
from
2eefa36
to
decc373
Compare
|
I have moved this change behind the partial syncing experimental flag so it is at least available for testing that work while we continue to iterate on the UX |
|
I have explored editing the source pattern inline in the editor, but I think it will be throw most users as the reverting to the source pattern content makes it look like their local customisations have been wiped: inline.mp4The redirect to focus mode is a better option I think, but we need to do it with client-side routing the same as the site editor template part focused mode to avoid the prompt to save that we get with the current full redirect to the site editor. We can explore this as a follow-up to this PR since this functionality is behind an experimental flag. |
|
Closing in favour of #57036 which keeps the global pattern editing in the post editor instead of redirecting to the site editor. |
|
Going to reopen this as it may take us a while to get the approach in #57036 finalised and it would be good to have the experiment fully functional in the mean time. Even though the edit flow in this PR is not perfect it is behind the experimental flag. |
|
Closing again, we are going to keep working in #57036 as a better approach. |
What?
Locks pattern editing in the post editor to only child blocks that have had content connections set.
N.B. This PR changes the paradigm for editing synced patterns in the editor and moves the editing of a pattern's content that causes global changes to the site editor or the wp-admin pattern editing interfaces (details of why below).
Why?
As part of the work on allowing partial editing of synced patterns we want to disable full editing of the synced pattern entities within the block editor.
This will make it easier to indicate to users when they are editing just a section of the pattern for that instance of the pattern only, versus editing the global source pattern instance.
This would also fix #54442 as the edit button only shows if the user has permission to update the pattern. It also fixes #32353
How?
Sets each of the patterns innerBlocks edit mode to
disabledusingsetBlockEditingModeunless the block connected attributes are set in which case the block is set tocontentOnly.Important note - limiting editing to
contentOnlyin the post editor is 🤞 just the starting point. We are keeping it restricted to contentOnly to keep things simple while we set up the initial APIs for implementing partial syncing. The hope is that we can later extend it out to things like innerBlocks, limited block style attributes, etc. when/if the complexities around doing so are resolved.Testing Instructions
Editbutton in the block toolbar redirects to site editor pattern edit canvas for blocked based theme and admin user, or post editor for entity for non-block themes or non-admin usersEditbutton does not display if the user does not have edit permission for the given patterncontentOnly. Also check that theContentpanel appears in right inspector panel when block or nested content selected.Screenshots or screencast
pattern-lock-edit.mp4