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.

Sign up for GitHub

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

Jump to bottom

Improve History API #11226

Closed
scofalik opened this issue Feb 7, 2022 · 3 comments · Fixed by #11634
Closed

Improve History API #11226

scofalik opened this issue Feb 7, 2022 · 3 comments · Fixed by #11634
Assignees
@ma2ciek
Labels
package:engine squad:collaboration Issue to be handled by the Collaboration team. type:improvement This issue reports a possible enhancement of an existing feature.
Milestone
iteration 53 (v34...

Comments

@scofalik
Copy link
Contributor

scofalik commented Feb 7, 2022

📝 Provide a description of the improvement

History is a simple class that is mostly an array with a few additional functionalities. Recently, History was used in real time collaboration and revision history. Some API was lacking, and it would be nice to introduce it:

  • make _operations public+readonly.
  • #clear() - removes all data from the history.
  • #last - returns the most recent operation from the history.
  • #length - returns the total amount of operations in the history.

Other than that it would be good to think about making getOperations() faster (now it looks through all operations). It is not a problem right now -- I will make a separate issue if it proves problematic. But it would not harm to think on this matter for some several minutes.
@scofalik scofalik added type:improvement This issue reports a possible enhancement of an existing feature. package:engine squad:collaboration Issue to be handled by the Collaboration team. labels Feb 7, 2022
@scofalik
Copy link
Contributor Author

Maybe History could also include offset value? It seems that this makes sense. We could try and see how this change would influence the existing code.

@scofalik
Copy link
Contributor Author

Rename getOperations() to getOperationsRange()? Minor breaking change, method name would make more sense now and it would not be weird to have both operations property and getOperations() method.

@CKEditorBot CKEditorBot added status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. and removed status:planned Set automatically when an issue lands in the "Sprint backlog" column. We will be working on it soon. labels Mar 14, 2022
@ma2ciek ma2ciek mentioned this issue Apr 14, 2022
Merged
@scofalik
Copy link
Contributor Author

Summary of changes:

  • Before changes, the Document#version property referred to the base version of the last document operation present in the History, thus the property has been moved to the History#version and Document#history became a getter to provide backward compatibility and easier access.
  • The History#addOperation() method now automatically increments the History#version property. You should not need to set the version manually, but if it is necessary in your case, it is possible to change it and then call History#addOperation(). Note, that changing the version property may easily lead to breaking the document.
  • The History#reset() method has been introduced to allow resetting History to the initial state.
  • The History#getOperations(from, to) method was redesigned to be faster - now it will not have to iterate over all operations so the speed of this method for small ranges should be improved.
  • The History#lastOperation has been introduced to quickly access the last operation added to the history.

scofalik added a commit that referenced this issue Apr 26, 2022
@scofalik
Merge pull request #11634 from ckeditor/cke/11226
8e96364 
Feature (engine): Improved the `History` API. Changes summary is in the related issue. Closes #11226.

Fix (restricted-editing): Standard editing mode postfixers will no longer create marker operations with invalid base versions. Closes #11644.

MINOR BREAKING CHANGE: The `Document#version` became a getter and changing it is now impossible. `document.applyOperation()` increments the `Document#version`, so changing it directly should not be necessary. However, for advanced use cases, it can be changed using the new `History#version` property.
@CKEditorBot CKEditorBot removed the status:in-progress Set automatically when an issue lands in the "In progress" column. We are working on it. label Apr 26, 2022
@CKEditorBot CKEditorBot added this to the iteration 53 milestone Apr 26, 2022
@scofalik scofalik mentioned this issue May 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ma2ciek ma2ciek

Labels
package:engine squad:collaboration Issue to be handled by the Collaboration team. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Milestone
iteration 53 (v34.1.0)
Development

Successfully merging a pull request may close this issue.

Improve History API ckeditor/ckeditor5
3 participants
@scofalik @ma2ciek @CKEditorBot