Correcting the splitting logic in setNodes #4168
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Those conditions are: - When the anchor does not lie at the start of a node. - When the focus does not lie at the end of a node.
Previously, in the case the previous node gets merged in last iteration, prev pointer could be pointing to the wrong node. That posed problems, especially when normalizing empty text nodes. So, now in every iteration, we get a copy of updated node structure, and take value of prev from that.
- In this, since anchor and focus bleeds on both sides, splits happen. - Empty text nodes are introduced on either side. - New properties are set in the new node selection. - Normalization happens and takes care of those empty text nodes.
🦋 Changeset detectedLatest commit: 60b3537 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks @ridhambhat, great PR! |
Merged
beorn
pushed a commit
to beorn/slate
that referenced
this pull request
Apr 13, 2021
* Defined conditions to always split nodes Those conditions are: - When the anchor does not lie at the start of a node. - When the focus does not lie at the end of a node. * prev variable now points to correct updated node. Previously, in the case the previous node gets merged in last iteration, prev pointer could be pointing to the wrong node. That posed problems, especially when normalizing empty text nodes. So, now in every iteration, we get a copy of updated node structure, and take value of prev from that. * Test to check splitting and normalization logic. - In this, since anchor and focus bleeds on both sides, splits happen. - Empty text nodes are introduced on either side. - New properties are set in the new node selection. - Normalization happens and takes care of those empty text nodes. * Create two-lies-appear.md * Update two-lies-appear.md Co-authored-by: Ian Storm Taylor <ian@ianstormtaylor.com>
This was referenced Apr 14, 2021
4 tasks
4 tasks
z2devil
pushed a commit
to z2devil/slate
that referenced
this pull request
Dec 6, 2024
* Defined conditions to always split nodes Those conditions are: - When the anchor does not lie at the start of a node. - When the focus does not lie at the end of a node. * prev variable now points to correct updated node. Previously, in the case the previous node gets merged in last iteration, prev pointer could be pointing to the wrong node. That posed problems, especially when normalizing empty text nodes. So, now in every iteration, we get a copy of updated node structure, and take value of prev from that. * Test to check splitting and normalization logic. - In this, since anchor and focus bleeds on both sides, splits happen. - Empty text nodes are introduced on either side. - New properties are set in the new node selection. - Normalization happens and takes care of those empty text nodes. * Create two-lies-appear.md * Update two-lies-appear.md Co-authored-by: Ian Storm Taylor <ian@ianstormtaylor.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request aims to solves a couple of things:
Issue
Fixes: #4038
Example
Old behavior
New behavior
Context
For the splitting logic, if always is not set for the mentioned conditions, then for the case where anchor is at the end of one node and focus is at the start of another node, the selection bleeds through and properties is applied on nodes where it is not intended. It is safer to split nodes and have empty nodes which will be normalized later.
For the normalization, in particular conditions when a text node was sandwiched between two empty text nodes, then due to the way previous variable was calculated in logic before, it pointed to the wrong node, not taking in account that it could have been merged/deleted. To correct this behavior, now prev is taken from an updated copy of the base node at each iteration.
Checks
yarn test.yarn lint. (Fix errors withyarn fix.)yarn start.)yarn changeset add.)