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

Pasting text into columns isn't showing up on frontend #16645

Closed
mapk opened this issue Jul 17, 2019 · 4 comments · Fixed by #16717
Closed

Pasting text into columns isn't showing up on frontend #16645

mapk opened this issue Jul 17, 2019 · 4 comments · Fixed by #16717
Labels
[Block] Columns Affects the Columns Block [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@mapk
Copy link
Contributor

mapk commented Jul 17, 2019

Describe the bug
I'm trying to paste test into the Column block, but that text isn't showing on the frontend. It only shows up if I make a change to the color, size, or another attribute first, then it shows.

To reproduce
Steps to reproduce the behavior:

  1. Add Columns block.
  2. Copy and paste text from another website into each of the columns.
  3. Save the document and preview it.
  4. Observe that the text isn't showing on the frontend.

Expected behavior
I expect that the text shows without having to change any attributes of it. In the screencast below, I've already copied the text in the Columns block. I've made an attribute change to the text in the first columns making a line bold. But the other two columns I haven't touched. When I check the preview, the two columns of text don't show.

Screenshots

columns

Desktop (please complete the following information):

  • OSX
  • Firefox 68.0
  • Gutenberg 6.1
@mapk mapk added [Type] Bug An existing feature does not function as intended [Block] Columns Affects the Columns Block labels Jul 17, 2019
@draganescu
Copy link
Contributor

draganescu commented Jul 19, 2019

I think this is a problem with all Inner Blocks. So far found the behaviour in both columns and the group block and it doesn't matter in what kind of block we paste: heading, list or paragraph.

This also happens if you add a paragraph and paste a copied image in it: the block is converted to image but after save and refresh it's gone.

@draganescu draganescu added [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Priority] High Used to indicate top priority items that need quick attention labels Jul 19, 2019
@noisysocks
Copy link
Member

When the text is pasted, the REPLACE_BLOCKS action swaps out the inner block for a new block. My guess is that this doesn't update the store in a way that invalidates the cached getBlocks selector:

export const getBlocks = createSelector(
( state, rootClientId ) => {
return map(
getBlockOrder( state, rootClientId ),
( clientId ) => getBlock( state, clientId )
);
},
( state ) => [
state.blocks.byClientId,
state.blocks.order,
state.blocks.attributes,
]
);

@talldan
Copy link
Contributor

talldan commented Jul 23, 2019

I have a theory, but it needs more testing to verify it. The issue might be the caching of getBlock, which getBlocks calls through to.

export const getBlock = createSelector(
( state, clientId ) => {
const block = state.blocks.byClientId[ clientId ];
if ( ! block ) {
return null;
}
return {
...block,
attributes: getBlockAttributes( state, clientId ),
innerBlocks: getBlocks( state, clientId ),
};
},
( state, clientId ) => [
// Normally, we'd have both `getBlockAttributes` dependancies and
// `getBlocks` (children) dependancies here but for performance reasons
// we use a denormalized cache key computed in the reducer that takes both
// the attributes and inner blocks into account. The value of the cache key
// is being changed whenever one of these dependencies is out of date.
state.blocks.cache[ clientId ],
]
);

I notice that it only has state.blocks.cache[ clientId ], as the data that invalidates it.

This cache looks like it is updated when the replaceBlocks action is triggered, but only for the replaced block itself (the leaf inner block that the content has been pasted into) and the block that replaces it:

case 'REPLACE_BLOCKS_AUGMENTED_WITH_CHILDREN':
newState.cache = {
...omit( newState.cache, action.replacedClientIds ),
...fillKeysWithEmptyObject(
getBlocksWithParentsClientIds( keys( flattenBlocks( action.blocks ) ) ),
),
};
break;

The issue might be that when getBlock is called for the parent column block, its cache entry hasn't been invalidated, so it doesn't bother recursing down to fetch the replaced inner blocks. The selector still returns the column as having the old inner block

Contrast this to inserting a new block for an inner block and it looks like the rootClientId is added to the list of blocks that should be invalidated, which means it doesn't happen there:

case 'INSERT_BLOCKS': {
const updatedBlockUids = keys( flattenBlocks( action.blocks ) );
if ( action.rootClientId ) {
updatedBlockUids.push( action.rootClientId );
}
newState.cache = {
...newState.cache,
...fillKeysWithEmptyObject(
getBlocksWithParentsClientIds( updatedBlockUids ),
),
};
break;
}

@talldan
Copy link
Contributor

talldan commented Jul 23, 2019

Looks like it's related to the incorrect clientId being passed into getBlocksWithParentsClientIds. #16717 should fix it.

@talldan talldan added the [Type] Regression Related to a regression in the latest release label Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns Block [Feature] Nested / Inner Blocks Anything related to the experience of nested/inner blocks inside a larger container, like Group or P [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants