-
Notifications
You must be signed in to change notification settings - Fork 40
T/389 ModelRange#getTransformedByMove and other #392
Conversation
…d at the same position. (#340)
…e of transforming targetPosition.
…that are contained by other returned ranges.
… when it is collapsed (#340).
Follow up #393 |
@@ -307,16 +311,15 @@ export default class Document { | |||
* * 'removeRootAttribute' when attribute for root is removed, | |||
* * 'changeRootAttribute' when attribute for root changes. | |||
* | |||
* Change event is fired after the change is done. This means that any ranges or positions passed in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it was not true - data.range
is indeed after the change, but data.position
is before the change. I added additional note in params comments.
Important reminder – please remember to rebase your branch onto the latest master before putting it on review. |
} | ||
|
||
return transformed; | ||
} | ||
|
||
/** | ||
* Returns all deltas from history, starting from given history point (if passed). | ||
* | ||
* @param {Number} from History point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing @returns
. Can't it be a generator, btw?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Is there really a difference? I always rebase with master ( I also find git history much more messy when doing stuff your way. |
@scofalik: I was asking exactly for rebasing :D. We only need to merge once an issue is already on master. Anyway, it's not a big issue – it just may delay or confuse the reviewer. We had some issues because of that in the past. Especially, when more things depend on each other, being really up to date with your changes is important to ensure that tests pass. |
Sure, sure. I understand the point of being up-to-date with master :), I just thought you are talking about different command. |
This PR combines changes from ckeditor/ckeditor5#3692 which are required by undo manager. The biggest change is introducing
engine.treeModel.Range#getTransformedByMove
method.