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

Shared Blocks: Fix editing paragraph blocks #7077

Merged
merged 1 commit into from
Jun 4, 2018

Conversation

youknowriad
Copy link
Contributor

closes #7075

Right now editing shared blocks is broken because when we render the Autocomplete component (used in paragraph blocks), we're triggering a "fetch shared blocks" request. This requests is causing the uid of the block attached to the shared block to be regenerated which means the block will rerender (considered as a different block) and the loop continues.

The root cause of the issue is not fixed in this PR, this is a hacky fix will small implications than can land in 3.0 but as a follow-up I'm planning to fix the root issue which is the fact that we have a cycle dependency in our shared blocks state.

sharedBlock.id => block.uid relation
and
block.uid => sharedBlock.id

This cycle is forcing us to regenerate the block attached to the shared block when we fetch them.

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Jun 1, 2018
@youknowriad youknowriad added this to the 3.0 milestone Jun 1, 2018
@youknowriad youknowriad self-assigned this Jun 1, 2018
Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix + proper fix. LGTM.

@mcsf
Copy link
Contributor

mcsf commented Jun 1, 2018

Regarding the failing tests: the fact that we now always fetch shared blocks means that RECEIVE_BLOCKS will be dispatched via RECEIVE_SHARED_BLOCKS. As a result, core/editor::editor.past receives a history snapshot and the Undo button becomes enabled because hasEditorUndo( state ) === state.editor.past.length > 0 === true.

This isn't an issue introduced in this PR (nor in the previous PR working on the autocompleter, IMO), but it exposes the root problem: that adding block knowledge (awareness of available shared blocks) to our application (even when said blocks are not impacting the post) is treated as an editor change.

Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Nice hack 👍

@youknowriad youknowriad force-pushed the fix/editing-shared-paragraph-bloocks branch from 122bc28 to ac9e0a9 Compare June 4, 2018 09:22
@youknowriad
Copy link
Contributor Author

I updated the hack to avoid the initial "history". It's still hacky though. Let's get this in and see how we can fi the shared blocks root state issue later.

@youknowriad youknowriad merged commit 80864d4 into master Jun 4, 2018
@youknowriad youknowriad deleted the fix/editing-shared-paragraph-bloocks branch June 4, 2018 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shared Blocks: Can't edit Paragraphs
3 participants