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

Fix losing overrides after detaching patterns #58164

Merged
merged 4 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Pattern blocks allows conversion back to blocks when the reusable block has unsaved edits 1`] = `
"<!-- wp:paragraph -->
<p>1</p>
<!-- /wp:paragraph -->"
`;

exports[`Pattern blocks can be created from multiselection and converted back to regular blocks 1`] = `
"<!-- wp:paragraph -->
<p>Hello there!</p>
Expand Down
28 changes: 0 additions & 28 deletions packages/e2e-tests/specs/editor/various/pattern-blocks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,34 +252,6 @@ describe( 'Pattern blocks', () => {
);
} );

// Check for regressions of https://github.com/WordPress/gutenberg/issues/26421.
// Skip reason: This is broken at the time with Pattern Overrides.
// See https://github.com/WordPress/gutenberg/issues/58122
it.skip( 'allows conversion back to blocks when the reusable block has unsaved edits', async () => {
await createReusableBlock( '1', 'Edited block' );

// Make an edit to the reusable block and assert that there's only a
// paragraph in a reusable block.
await canvas().waitForSelector( 'p[aria-label="Block: Paragraph"]' );
await canvas().click( 'p[aria-label="Block: Paragraph"]' );
await page.keyboard.type( '2' );
const selector =
'//div[@aria-label="Block: Pattern"]//p[@aria-label="Block: Paragraph"][.="12"]';
const reusableBlockWithParagraph = await page.$x( selector );
expect( reusableBlockWithParagraph ).toBeTruthy();

// Convert back to regular blocks.
await clickBlockToolbarButton( 'Select parent block: Edited block' );
await clickBlockToolbarButton( 'Options' );
await clickMenuItem( 'Detach' );
await page.waitForXPath( selector, {
hidden: true,
} );

// Check that there's only a paragraph.
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

// Test for regressions of https://github.com/WordPress/gutenberg/issues/27243.
it( 'should allow a block with styles to be converted to a reusable block', async () => {
// Insert a quote and reload the page.
Expand Down
42 changes: 27 additions & 15 deletions packages/patterns/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/

import { parse } from '@wordpress/blocks';
import { cloneBlock } from '@wordpress/blocks';
import { store as coreStore } from '@wordpress/core-data';
import { store as blockEditorStore } from '@wordpress/block-editor';

Expand Down Expand Up @@ -90,25 +90,37 @@ export const createPatternFromFile =
export const convertSyncedPatternToStatic =
( clientId ) =>
( { registry } ) => {
const oldBlock = registry
const patternBlock = registry
.select( blockEditorStore )
.getBlock( clientId );
const pattern = registry
.select( 'core' )
.getEditedEntityRecord(
'postType',
'wp_block',
oldBlock.attributes.ref
);

const newBlocks = parse(
typeof pattern.content === 'function'
? pattern.content( pattern )
: pattern.content
);
function cloneBlocksAndRemoveBindings( blocks ) {
return blocks.map( ( block ) => {
let metadata = block.attributes.metadata;
if ( metadata ) {
metadata = { ...metadata };
delete metadata.id;
delete metadata.bindings;
}
return cloneBlock(
block,
{
metadata:
metadata && Object.keys( metadata ).length > 0
? metadata
: undefined,
},
cloneBlocksAndRemoveBindings( block.innerBlocks )
);
} );
}

registry
.dispatch( blockEditorStore )
.replaceBlocks( oldBlock.clientId, newBlocks );
.replaceBlocks(
patternBlock.clientId,
cloneBlocksAndRemoveBindings( patternBlock.innerBlocks )
);
};

/**
Expand Down
47 changes: 47 additions & 0 deletions test/e2e/specs/editor/various/pattern-overrides.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,4 +216,51 @@ test.describe( 'Pattern Overrides', () => {
] );
} );
} );

test( 'retains override values when converting a pattern block to regular blocks', async ( {
page,
admin,
requestUtils,
editor,
} ) => {
const paragraphId = 'paragraph-id';
const { id } = await requestUtils.createBlock( {
title: 'Pattern',
content: `<!-- wp:paragraph {"metadata":{"id":"${ paragraphId }","bindings":{"content":{"source":{"name":"pattern_attributes"}}}}} -->
<p>Editable</p>
<!-- /wp:paragraph -->`,
status: 'publish',
} );

await admin.createNewPost();

await editor.insertBlock( {
name: 'core/block',
attributes: { ref: id },
} );

// Make an edit to the pattern.
await editor.canvas
.getByRole( 'document', { name: 'Block: Paragraph' } )
.focus();
await page.keyboard.type( 'edited ' );

// Convert back to regular blocks.
await editor.selectBlocks(
editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } )
);
await editor.showBlockToolbar();
await editor.clickBlockOptionsMenuItem( 'Detach' );

// Check that the overrides remain.
await expect.poll( editor.getBlocks ).toMatchObject( [
{
name: 'core/paragraph',
attributes: {
content: 'edited Editable',
metadata: undefined,
},
},
] );
} );
} );
69 changes: 69 additions & 0 deletions test/e2e/specs/editor/various/patterns.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,4 +252,73 @@ test.describe( 'Synced pattern', () => {
},
] );
} );

// Check for regressions of https://github.com/WordPress/gutenberg/issues/26421.
test( 'allows conversion back to blocks when the reusable block has unsaved edits', async ( {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original test tested when there are unsaved edits to the reusable block entity, which seems to be lost now in this new test, but I think is still a valuable test.

The new test is also valuable, but maybe it's worth also keeping the original and restoring it at some point rather than deleting it (it would have to be changed to use the 'Edit original' button to update the entity).

Given this new test is for overrides, is it worth moving it to the pattern-overrides file? Suggestion for a new test title:

Suggested change
test( 'allows conversion back to blocks when the reusable block has unsaved edits', async ( {
test( 'retains override values when converting a pattern block to regular blocks', async ( {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, right! Makes sense!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in fe5aa44.

page,
requestUtils,
editor,
} ) => {
const { id } = await requestUtils.createBlock( {
title: 'Synced pattern',
content:
'<!-- wp:paragraph -->\n<p>Before Edit</p>\n<!-- /wp:paragraph -->',
status: 'publish',
} );

await editor.insertBlock( {
name: 'core/block',
attributes: { ref: id },
} );

await editor.selectBlocks(
editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } )
);
await page
.getByRole( 'toolbar', { name: 'Block tools' } )
.getByRole( 'link', { name: 'Edit original' } )
.click();

const editorTopBar = page.getByRole( 'region', {
name: 'Editor top bar',
} );

// Navigate to the pattern focus mode.
await expect(
editorTopBar.getByRole( 'heading', {
name: 'Synced pattern',
level: 1,
} )
).toBeVisible();

// Make an edit to the source pattern.
await editor.canvas
.getByRole( 'document', { name: 'Block: Paragraph' } )
.fill( 'After Edit' );

// Go back to the post.
await editorTopBar.getByRole( 'button', { name: 'Back' } ).click();

const expectedParagraphBlock = {
name: 'core/paragraph',
attributes: { content: 'After Edit' },
};

await expect.poll( editor.getBlocks ).toMatchObject( [
{
name: 'core/block',
attributes: { ref: id },
innerBlocks: [ expectedParagraphBlock ],
},
] );

await editor.selectBlocks(
editor.canvas.getByRole( 'document', { name: 'Block: Pattern' } )
);
await editor.clickBlockOptionsMenuItem( 'Detach' );

await expect
.poll( editor.getBlocks )
.toMatchObject( [ expectedParagraphBlock ] );
} );
} );
Loading