Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Improve call frequency #16

Merged
merged 16 commits into from
Aug 16, 2018
Merged

Improve call frequency #16

merged 16 commits into from
Aug 16, 2018

Conversation

ma2ciek
Copy link
Contributor

@ma2ciek ma2ciek commented Aug 13, 2018

Suggested merge commit message (convention)

Other: Improved call frequency. Closes ckeditor/ckeditor5#2539. Closes ckeditor/ckeditor5#2541. Closes ckeditor/ckeditor5#2547. Closes ckeditor/ckeditor5#1158.


Additional information

  • This PR Introduces Autosave#state, which can have the following values:

    • synchronized - when all changes are saved
    • waiting - when the plugin is waiting for other changes before calling adapter#save() and config.autosave.save()
    • saving - when the provided save method is called and the plugin waits for the response from callbacks
  • It switches from throttle to lodash's debounce

  • It makes the timeout configurable

@pjasiun pjasiun self-requested a review August 13, 2018 12:40
src/autosave.js Outdated

// A minimum amount of time that needs to pass after the last action.
// After that time the provided save callbacks are being called.
const waitingTime = config.waitingTime || 2000;
Copy link

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

After #12 (comment) default timeout should work in most cases and is should not need to be configurable.

Copy link

Choose a reason for hiding this comment

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

Also, I am not sure if 2000 is not too much. Maybe 1000?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After #12 (comment) default timeout should work in most cases and it should not need to be configurable.

Right, I overlooked it.

Also, I am not sure if 2000 is not too much. Maybe 1000?

The last call will be on the Editor#destroy or Window#beforeunload, so IMO we can use a higher value here.

src/autosave.js Outdated
}

// Do nothing if the plugin is in `external-saving` state.
Copy link

@pjasiun pjasiun Aug 13, 2018

Choose a reason for hiding this comment

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

A comment is needed that is will be handled based on the document version.

@pjasiun pjasiun merged commit 820e060 into master Aug 16, 2018
@pjasiun pjasiun deleted the t/12 branch August 16, 2018 12:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants