Skip to content

Merging blocks: clone blocks to safely insert selection tracking character#16419

Merged
ellatrix merged 2 commits intomasterfrom
try/merge-blocks-clone
Jul 5, 2019
Merged

Merging blocks: clone blocks to safely insert selection tracking character#16419
ellatrix merged 2 commits intomasterfrom
try/merge-blocks-clone

Conversation

@ellatrix
Copy link
Copy Markdown
Member

@ellatrix ellatrix commented Jul 4, 2019

Description

  • The selection tracking character shouldn't be inserted when the merge effect returns early (e.g. no block transformation).
  • Only insert the selection tracking character when we are sure that there is text selection.

Might fix wordpress-mobile/WordPress-iOS#11950?

How has this been tested?

Insert a list and a heading. Try to merge the heading with list. Select something else. Ensure that no space is added before the heading text. In master, some whitespace is added.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@ellatrix ellatrix requested review from SergioEstevao, Tug and koke July 4, 2019 15:49
@ellatrix ellatrix added [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended labels Jul 4, 2019
@ellatrix ellatrix added this to the Gutenberg 6.1 milestone Jul 4, 2019
@ellatrix ellatrix changed the title Merging blocks: clone blocks to safely insert selection character Merging blocks: clone blocks to safely insert selection tracking character Jul 4, 2019
Copy link
Copy Markdown
Contributor

@koke koke left a comment

Choose a reason for hiding this comment

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

I wasn't able to reproduce the original description on mobile or the crash in wordpress-mobile/WordPress-iOS#11950, but the approach of cloning the blocks seems like it could help

@ellatrix
Copy link
Copy Markdown
Member Author

ellatrix commented Jul 5, 2019

Thanks @koke. This seems generally a good improvement and hopefully fixes the issue. Let me know if the Sentry reports stop or not. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RCTFatalException: Unhandled JS Exception: undefined is not an object (evaluating 'y.attributes[V].multiline')

2 participants