Skip to content
This repository was archived by the owner on Feb 6, 2023. It is now read-only.

Fix issue using insertFragmentIntoContentState function with only a content block in fragment #1869

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

marconi1992
Copy link

Summary
This PR fix the issue reported in #1868, the function updateExistingBlock into src/model/transaction/insertFragmentIntoContentState.js wasn't keeping the original block type.

Test Plan
I updated the test, so if the block type was unstyled and you paste a heading html element the new block should be heading instead unstyled.

@yangshun
Copy link
Contributor

Thanks for the PR @marconi1992! Please use a more meaningful PR title and a commit message that is clearer about what the PR is trying to achieve.

The tests are failing on master now probably because of an update to Node/npm. I believe I fixed them in #1866 but you'll have to rebase first to get the fix.

@marconi1992 marconi1992 changed the title Fix issue #1868 Fix issue using insertFragmentIntoContentState function Sep 12, 2018
@marconi1992 marconi1992 changed the title Fix issue using insertFragmentIntoContentState function Fix issue using insertFragmentIntoContentState function with only a content block Sep 12, 2018
@marconi1992 marconi1992 changed the title Fix issue using insertFragmentIntoContentState function with only a content block Fix issue using insertFragmentIntoContentState function with only a content block in fragment Sep 12, 2018
@marconi1992
Copy link
Author

Thanks, @yangshun, the PR is updated and the related issue #1868

@colinjeanne
Copy link
Contributor

Also note that this is a breaking change to the API. Please see #1511 for a case in which copying the fragment's block type is actually quite bad.

@marconi1992
Copy link
Author

marconi1992 commented Sep 23, 2018

@colinjeanne I read the comment your left in #1511, it will happen when we're pasting in a block that has already text, we should keep the type when there's no content there, You can compare the behavior against CK Editor. https://ckeditor.com/ckeditor-4/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants