Skip to content

Commit

Permalink
Prevent RESET_BLOCKS from affecting reusable blocks
Browse files Browse the repository at this point in the history
Changes the editor reducer so that RESET_BLOCKS will only remove blocks
that are actually in the post, i.e. blocks that are a descendent of the
`''` `blocks.order` key.

This fixes a bug where editing a post in Text Mode would break any
reusable blocks in the post.
  • Loading branch information
noisysocks committed Nov 12, 2018
1 parent c746581 commit 0c92c67
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 2 deletions.
58 changes: 56 additions & 2 deletions packages/editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,31 @@ function getFlattenedBlocks( blocks ) {
return flattenedBlocks;
}

/**
* Given a block order map object, returns *all* of the block client IDs that are
* a descendant of the given root client ID.
*
* Calling this with `rootClientId` set to `''` results in a list of client IDs
* that are in the post. That is, it excludes blocks like fetched reusable
* blocks which are stored into state but not visible.
*
* @param {Object} blocksOrder Object that maps block client IDs to a list of
* nested block client IDs.
* @param {?string} rootClientId The root client ID to search. Defaults to ''.
*
* @return {Array} List of descendant client IDs.
*/
function getNestedBlockClientIds( blocksOrder, rootClientId = '' ) {
const attachedClientIds = [];
if ( blocksOrder[ rootClientId ] ) {
blocksOrder[ rootClientId ].forEach( ( clientId ) => {
attachedClientIds.push( clientId );
attachedClientIds.push( ...getNestedBlockClientIds( blocksOrder, clientId ) );
} );
}
return attachedClientIds;
}

/**
* Returns an object against which it is safe to perform mutating operations,
* given the original object and its current working copy.
Expand Down Expand Up @@ -211,6 +236,35 @@ const withInnerBlocksRemoveCascade = ( reducer ) => ( state, action ) => {
return reducer( state, action );
};

/**
* Higher-order reducer which targets the combined blocks reducer and handles
* the `RESET_BLOCKS` action. When dispatched, this action will replace all
* blocks that exist in the post, leaving blocks that exist only in state (e.g.
* reusable blocks) alone.
*
* @param {Function} reducer Original reducer function.
*
* @return {Function} Enhanced reducer function.
*/
const withBlockReset = ( reducer ) => ( state, action ) => {
if ( state && action.type === 'RESET_BLOCKS' ) {
const visibleClientIds = getNestedBlockClientIds( state.order );
return {
...state,
byClientId: {
...omit( state.byClientId, visibleClientIds ),
...getFlattenedBlocks( action.blocks ),
},
order: {
...omit( state.order, visibleClientIds ),
...mapBlockOrder( action.blocks ),
},
};
}

return reducer( state, action );
};

/**
* Undoable reducer returning the editor post state, including blocks parsed
* from current HTML markup.
Expand Down Expand Up @@ -280,6 +334,8 @@ export const editor = flow( [
blocks: flow( [
combineReducers,

withBlockReset,

// Track whether changes exist, resetting at each post save. Relies on
// editor initialization firing post reset as an effect.
withChangeDetection( {
Expand All @@ -289,7 +345,6 @@ export const editor = flow( [
] )( {
byClientId( state = {}, action ) {
switch ( action.type ) {
case 'RESET_BLOCKS':
case 'SETUP_EDITOR_STATE':
return getFlattenedBlocks( action.blocks );

Expand Down Expand Up @@ -392,7 +447,6 @@ export const editor = flow( [

order( state = {}, action ) {
switch ( action.type ) {
case 'RESET_BLOCKS':
case 'SETUP_EDITOR_STATE':
return mapBlockOrder( action.blocks );

Expand Down
52 changes: 52 additions & 0 deletions packages/editor/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,58 @@ describe( 'state', () => {
} );

describe( 'blocks', () => {
it( 'should not reset any blocks that are not in the post', () => {
const actions = [
{
type: 'RESET_BLOCKS',
blocks: [
{
clientId: 'block1',
innerBlocks: [
{ clientId: 'block11', innerBlocks: [] },
{ clientId: 'block12', innerBlocks: [] },
],
},
],
},
{
type: 'RECEIVE_BLOCKS',
blocks: [
{
clientId: 'block2',
innerBlocks: [
{ clientId: 'block21', innerBlocks: [] },
{ clientId: 'block22', innerBlocks: [] },
],
},
],
},
];
const original = deepFreeze( actions.reduce( editor, undefined ) );

const state = editor( original, {
type: 'RESET_BLOCKS',
blocks: [
{
clientId: 'block3',
innerBlocks: [
{ clientId: 'block31', innerBlocks: [] },
{ clientId: 'block32', innerBlocks: [] },
],
},
],
} );

expect( state.present.blocks.byClientId ).toEqual( {
block2: { clientId: 'block2' },
block21: { clientId: 'block21' },
block22: { clientId: 'block22' },
block3: { clientId: 'block3' },
block31: { clientId: 'block31' },
block32: { clientId: 'block32' },
} );
} );

describe( 'byClientId', () => {
it( 'should return with attribute block updates', () => {
const original = deepFreeze( editor( undefined, {
Expand Down

0 comments on commit 0c92c67

Please sign in to comment.