Patterns: edit source pattern in focus mode in post and site editors #57036
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +798 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
This comment was marked as outdated.
This comment was marked as outdated.
|
Flaky tests detected in f1bb02c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7293648421
|
glendaviesnz
commented
Dec 14, 2023
glendaviesnz
commented
Dec 14, 2023
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/experiments-page.php |
|
Looking at the video I expected to see the lock icon in the toolbar of the block that could not be edited. So that one would have to click the lock icon and directly unlock it to gain access to editing the specific block. |
I did discuss the lock icon with @SaxonF but we decided that this was a slightly different paradigm to the other locking options so we wanted to avoid using it, ie. this is about different editing contexts, instance versus global, more than locking. It is something that can be discussed further though for sure once we get this merged and people test it. @SaxonF do you have anything to add about this? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
talldan
reviewed
Dec 15, 2023
packages/edit-post/src/components/sidebar/settings-header/index.js
Outdated
Show resolved
Hide resolved
packages/edit-post/src/components/header/mode-switcher/index.js
Outdated
Show resolved
Hide resolved
Comment on lines
+96
to
+97
| ( _selectedBlockName === 'core/block' && | ||
| window.__experimentalPatternPartialSyncing ) |
There was a problem hiding this comment.
I think we should make sure we consider reviewing this and the similar code (in use-in-between-inserter.js and selectors.js code down the road, as there's a lot of hard coding of values to bend things into a working state. Feels like there should be a better way.
The gist of the problem seems to be that none of the block editing modes seem to be suitable for patterns.
There was a problem hiding this comment.
I'd love to understand the problem more here.
There was a problem hiding this comment.
At the moment the PR uses useBlockEditingMode to prevent editing inside the pattern block, but it doesn't support exactly what's needed for the pattern block. 'disabled' locks down too much. 'contentOnly' also isn't right. At the moment the PR sets the inner blocks of the pattern to 'disabled' or 'contentOnly' as needed, but that still results in things like the sibling inserter showing.
This part still needs some investigation.
This comment was marked as outdated.
This comment was marked as outdated.
glendaviesnz
commented
Dec 18, 2023
| @@ -87,6 +90,9 @@ function Header( { | |||
| }; | |||
| }, [] ); | |||
|
|
|||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Can't we just simplify, if there's history show the document bar, otherwise don't? Alternatively we could also do like the site editor (which we should probably align anyway) and always show the document bar.
There was a problem hiding this comment.
@SaxonF do you have any views on always showing the document bar in the post editor?
3f86a29
to
ec4698f
Compare
I added an idea here but will add here as well for visibility. Here's a concept that highlights editable parts of a synced pattern or template on hover. You can also see when editing a page we can highlight and select the surrounding template much like a pattern. pattern-edit-select.mp4template-edit-selection.mp4 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…ditor instead of trying to reuse useLink
0550495
to
6bc3abb
Compare
|
Rebased again now that e2e tests should be fixed in trunk (and there were new conflicts) |
youknowriad
reviewed
Jan 3, 2024
| @@ -76,6 +76,7 @@ const BLOCK_EDITOR_SETTINGS = [ | |||
| '__unstableIsBlockBasedTheme', | |||
| '__experimentalArchiveTitleTypeLabel', | |||
| '__experimentalArchiveTitleNameLabel', | |||
| '__experimentalGetPostLinkProps', | |||
There was a problem hiding this comment.
Why is this "__experimental" ? It should be either "private" or stable.
There was a problem hiding this comment.
@youknowriad Do you know if there's prior art for making these private?
I saw in the block editor package there are some private settings, but not so much in the editor package.
There was a problem hiding this comment.
I'm not sure there's for the EditorProvider yet.
There was a problem hiding this comment.
I think there's something unusual going on here anyway, as it looks like getPostLinkProps is already passed in as an editor setting, so I'm not sure if __experimentalGetPostLinkProps is also needed.
There was a problem hiding this comment.
PR here - #57535
(I'll continue looking into whether it can be made private)
3 tasks
This was referenced Jan 9, 2024
|
@glendaviesnz Can I confirm whether you think any of the files here will need a backport to Core? Experiments page doesn't need to be brought over and the rest looks like JS changes only. |
|
@getdave the experiments file is the only php change so no backporting for this one. |
This was referenced Jan 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
In the
Synced patterns partial syncingexperiment 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 a focus mode view, similar to template editing.
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?
disabledusingsetBlockEditingModeunless the block connected attributes are set in which case the block is set tocontentOnly.Editbutton to pattern block toolbar to edit source pattern.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
Edit originalbutton in the block toolbar redirects to focus mode canvas. Make sure pattern can be edited and that back button on top menu bar works as expected.Edit originalbutton does not display if the user does not have edit permission for the given patternEdit originaland back buttons works as expectedcontentOnly. Also check that theContentpanel appears in right inspector panel when block or nested content selected.Screenshots or screencast
patterns-again.mp4