Fix not expanding pattern in page editor #53169
Merged
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
kevin940726
added
[Type] Bug
|
Size Change: -240 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
aaronrobertshaw
approved these changes
Aug 1, 2023
There was a problem hiding this comment.
Thanks @kevin940726 👍
This is a better approach than #53166.
✅ Core & theme pattern blocks are replaced by their inner blocks for pages in Site Editor
✅ Patterns referenced within a template part are also expanded correctly
✅ Couldn't spot any regressions around the disabled block editing
Given the short time left to sneak this fix into the 6.3 RC, I think we should move ahead in landing this PR.
LGTM 🚢
andrewserong
added
the
Backport to WP 6.7 Beta/RC
glendaviesnz
approved these changes
Aug 1, 2023
andrewserong
reviewed
Aug 1, 2023
There was a problem hiding this comment.
I like the approach in this PR, too, and I think it makes good sense for expanding the block.
One challenge I ran into in testing following the instructions in #52983 is that if the pattern you're referencing contains Post Title or Featured Image blocks within a Query Loop, then they get treated as though they're part of the editable content blocks. Here's an example:
2023-08-01.10.54.29.mp4
The template's markup:
<!-- wp:template-part {"slug":"header","tagName":"header"} /-->
<!-- wp:pattern {"slug":"core/query-standard-posts"} /-->
<!-- wp:post-content /-->
<!-- wp:template-part {"slug":"footer","tagName":"footer"} /-->
Ideally, I suppose we'd check the editable content blocks to make sure they're not a child of a query? I don't think this is necessarily a blocker for this PR, though, as the fix would likely happen elsewhere 🤔
For the issue I ran into, I could fix it by updating the following line to be In my testing environment |
andrewserong
approved these changes
Aug 1, 2023
There was a problem hiding this comment.
This looks good to me, thanks for the investigation here everyone! The only issue I ran into was some flickering of the Cover block when set to a dark background image within a synced block that is set within a page's template. I think this is likely a bug with the cover block's useCoverIsDark hook and how it updates attributes when the isDark value changes. My recommendation would be for us to merge this PR in as-is, and continue looking into Cover block behaviour in subsequent fixes / explorations.
LGTM! ✨
|
I've tested the tweak proposed by @andrewserong and it works well in fixing the issue noted in his review. While testing, I did encounter an issue with a synced Cover pattern flickering under some odd circumstances. This appears to be an issue with the detection of if the Cover block is "dark" or not and should be considered a separate issue. I'm happy to merge this one the e2es pass |
Mamaduka
reviewed
Aug 1, 2023
| */ | ||
| import { unlock } from '../lock-unlock'; | ||
|
|
||
| const PatternEdit = ( { attributes, clientId, rootClientId } ) => { |
There was a problem hiding this comment.
The rootClientId will be undefined since the block edit component doesn't receive it as a prop.
gutenberg/packages/block-editor/src/components/block-list/block.js
Lines 124 to 142 in 561d244
Mamaduka
reviewed
Aug 1, 2023
Comment on lines
+49
to
+57
| // Temporarily set the root block to default mode to allow replacing the pattern. | ||
| // This could happen when the page is disabling edits of non-content blocks. | ||
| __unstableMarkNextChangeAsNotPersistent(); | ||
| setBlockEditingMode( rootClientId, 'default' ); | ||
| __unstableMarkNextChangeAsNotPersistent(); | ||
| replaceBlocks( clientId, clonedBlocks ); | ||
| // Restore the root block's original mode. | ||
| __unstableMarkNextChangeAsNotPersistent(); | ||
| setBlockEditingMode( rootClientId, rootEditingMode ); |
There was a problem hiding this comment.
Maybe we should batch these updates. What do you think?
tellthemachines
pushed a commit
that referenced
this pull request
Aug 1, 2023
--------- Co-authored-by: Aaron Robertshaw <[email protected]>
|
I just cherry-picked this PR to the update/second-round-6-3-rc3 branch to get it included in the next release: 0bcda10 |
tellthemachines
removed
the
Backport to WP 6.7 Beta/RC
tellthemachines
added a commit
that referenced
this pull request
Aug 1, 2023
* Top toolbar: Fix issues with save button overlap, and plugin buttons (#53101) * Shorten the width of the top toolbar overlay and make doc title smaller. * Add a scrim and a margin to handle plugin buttons better. * Remove block tools back compat component schedule for deprecated in 6.3 (#53115) * Removes usage of BlockToolsBackCompat * Remove unwanted BlockTools from Nav sidebar * Footnotes/RichText: fix getRichTextValues for deeply nested blocks (#53034) * Defer to preceding handlers in command palette keyboard shortcut (#53001) * Image block: fix image size at wide and full width (#53184) * Fix regression with Edit site Navigate regions (#52940) * Make the navigabel region wrap the inert sidebar. * Adjust animation. * Fix not expanding pattern in page editor (#53169) --------- Co-authored-by: Aaron Robertshaw <[email protected]> * Footnotes: fix published preview (#53072) * Footnotes: fix published preview * remove var dump * Fix php lint * PHP lint * Address feedback * Add e2e test * Footnotes: disable for synced patterns and prevent duplication for pages in site editor (#53003) * Initial commit: - Prevent footnote creation withing core/block - Only insert a footnote if one isn't found in the entity block list * Try grabbing controlled blocks from parent post content block * Cache `selectedClientId` Get hasParentCoreBlocks using separate useSelect call. * Rename hasParentCoreBlocks to isBlockWithinPattern Add comments * Removing while loop since we're already fetching the post content parent in the `getBlockParentsByBlockName` call above * Reinstating while loop because it can deal with nested blocks --------- Co-authored-by: Andrew Serong <[email protected]> * Footnotes: add missing _ in revision field filter (#53135) * Footnotes: add missing _ in revision field filter * Use correct hook name * Revert prefixing callback names * don't display BlockContextualToolbar at all in contentonly locking (#53110) * Render the footer conditionally in the global styles sidebar component so that any side effects from the footer wrapper are not rendered, e.g., styles and what not (#53204) Ensure that the precise bottom margin persists if the footer isn't rendered * Pattern: Add getBlockRootClientId call (#53206) --------- Co-authored-by: Joen A <[email protected]> Co-authored-by: Dave Smith <[email protected]> Co-authored-by: Ella <[email protected]> Co-authored-by: Mitchell Austin <[email protected]> Co-authored-by: Aki Hamano <[email protected]> Co-authored-by: Andrea Fercia <[email protected]> Co-authored-by: Kai Hao <[email protected]> Co-authored-by: Aaron Robertshaw <[email protected]> Co-authored-by: Ramon <[email protected]> Co-authored-by: Andrew Serong <[email protected]> Co-authored-by: Andrei Draganescu <[email protected]>
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 and why?
See #52983.
How?
The page editor disables edits, which causes
replaceBlocksin the pattern block to fail silently. This PR temporarily unsets the editing mode before performing the action and then restores it after it's done. I'm not sure if this is the best solution though. This is more like a debug PR than a fix. Feel free to push other solutions directly in this PR!Testing Instructions
Follow the instructions in #52983.
Screenshots or screencast
Kapture.2023-07-31.at.17.35.16.mp4