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.

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

Init plugin #2540

Closed
pjasiun opened this issue May 18, 2018 · 1 comment · Fixed by ckeditor/ckeditor5-autosave#2
Closed

Init plugin #2540

pjasiun opened this issue May 18, 2018 · 1 comment · Fixed by ckeditor/ckeditor5-autosave#2
Assignees
Labels
package:autosave type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@pjasiun
Copy link

pjasiun commented May 18, 2018

For easier integration, we should provide the Autosave plugin.

  • It should not fire any event. Instead, there should be provider property with a single save method which returns promise when save is done.
  • It should throttle on change event (500 ms).
  • It should add pending action as soon as a change happens. After 500 ms autosave plugin should get data and check if something really changed. If data is the same (for instance only users marker changed), pending action should be removed. If something changed, data should be passed to the save provider. provider.save( data ) should return promise when data are saved. Pending action should be removed then if there were no other changes in the meantime.
  • It should show a warning when on page unload if there are pending actions.

After that, it should be enough to check pending actions to learn if the editor can be closed.

@pjasiun
Copy link
Author

pjasiun commented May 18, 2018

We had a talk with @scofalik and @ma2ciek and realized that when the users' selection plugin will be enabled the pending action will be added for 500 ms every time any user changes the selection. After that timeout, getData will be called and compared and we will remove that pending action.

First, we need to check if it will be a problem in real cases. Editor blur will not cause selection change, so the user should be able to finish editing if no one will not move selection for 500 ms. It might be good enough.

If this will be a problem we have some solution.

Solution 1 - data view

Keep data view the same way we have editing view. I will allow up to observe if data changed and make getData work faster since the conversion won't be needed each time. However, it might be not an easy change.

Solution 2 - markers option

Since it is usually a marker what changes model the way that data do not change, we may have an option in addMarker and updateMarker to tell that this marker does not affect data.

Solution 3 - autosave.flush

We make have a method which will flush throttling and remove pending action if it is not needed. Such method should be called before finishing editing in order to make sure that there are no outdated pending actions.

I do not like this solution and all variants described below. It introduces an additional method to remove pending action which should not be added in the first place.

Solution 3.1 - pendingActions.refresh

We could make it part of the pending actions plugin interface. refresh method would fire an event to let all plugins check if their pending actions are still needed and remove them if not.

Solution 3.1 - a refresh on pending actions getters

We could also make it automatically. Refresh event could be fired whenever one wants to get the list of pending actions. But it is a bad smell to change the collection on collection getter.

Solution 4 (a.k.a. Workaround 1) - hardcoded if

For now, it might be the simples solution to check if what changed is users markers or something else. I know it is ugly, but it might be enough for a long time. I believe that other changes, even if not causes data change, will not occur often enough to make any problems with autosave.

@ma2ciek ma2ciek self-assigned this May 18, 2018
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-autosave Oct 9, 2019
@mlewand mlewand added this to the iteration 18 milestone Oct 9, 2019
@mlewand mlewand added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:autosave labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:autosave type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants