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

Greater save granularity #233

Closed
Gankra opened this issue Jun 4, 2013 · 5 comments
Closed

Greater save granularity #233

Gankra opened this issue Jun 4, 2013 · 5 comments

Comments

@Gankra
Copy link
Collaborator

Gankra commented Jun 4, 2013

Since you capture and block the CTRL/CMD+S shortcut, the only way to hook into this is to piggyback on the .on("save") event. Since a theoretical hook-in might want to save to the server itself, It would therefore be desirable to be able to gain greater control over how the save functionality behaves. For instance, one may want to autosave to localStorage every 100ms, but that might be excessive for saving to the server. However, the system should always save right away when a manual save has been triggered. Currently, there is no way to differentiate between these invocations, though.

I see three possible routes for this:

  • Give the on("save") callback an optional argument (auto) so that it can differentiate between when the user manually tried to trigger a save and when the system was autosaving.
  • Add a config option (debounceSave: [boolean]), which reinterprets the autoSave attribute to mean "trigger an autosave if the user has stopped typing after [autoSave]ms". Essentially equivalent to $(editor).on("keyup", _.debounce(save, autoSave)); See: http://underscorejs.org/#debounce
  • Add a config option (propagateSave: [boolean]), which allows the CTRL/CMD+S event to propagate normally.

These aren't mutually exclusive, of course.

Personally, I'm not entirely clear why the system keeps autosaving at all when the text has clearly not been modified. I suppose this is simply an easy way to avoid having to consider every way the textfield could have been modified?

@Gankra
Copy link
Collaborator Author

Gankra commented Jun 4, 2013

Differentiating auto/manual save can also be relevant because while one wouldn't really want to give user feedback on every single autosave, they would want to give feedback on a manual save.

@OscarGodson
Copy link
Owner

Give the on("save") callback an optional argument (auto) so that it can differentiate between when the user manually tried to trigger a save and when the system was autosaving.

I really like this idea a lot. It's true that you may want to autosave to localStorage, but only save to the server in some instances.

Personally, I'm not entirely clear why the system keeps autosaving at all when the text has clearly not been modified. I suppose this is simply an easy way to avoid having to consider every way the textfield could have been modified?

It shouldn't save if there has not been any changes, but it does. I can't remember off the top of my head why I'm resaving even if the content is the same, but it could just be a mistake.

@Gankra
Copy link
Collaborator Author

Gankra commented Jun 4, 2013

Alright, I'll go with the auto flag. Then the question is, what's an "auto" save? I'm noticing there's several places where the system saves just because its updated something. I'm inclined to say all of those are "auto". Basically the only calls that aren't "auto" are CTRL/CMD+S.

That sound right to you? Also, should the auto attribute be fed directly into the emit, or should something be done to avoid clobbering the default value (self.getFiles(...)). Basically, are you guaranteeing that level of API stability yet, or is that for 1.0?

@OscarGodson
Copy link
Owner

I'd say anything saved by the public API or the keyboard command. As an example, if a dev added a save button and made a call to .save() on button click that should be a manual save. The saving we do internally should not be. All of the internal saving we do for the textarea and preview should be using our internal draft file, so this means the only save youd have to check for is the one in the loop I believe.

The other idea I was just thinking is, should we change it so there's an autosave event and save event. What do you think about that?

I'd rather get a nice API before 1.0 then start to worry about backwards compatibility. As long as we do a major bump (0.x.0) and make a clear note of the change and how to upgrade, I'm fine with whatever usually.

@Gankra
Copy link
Collaborator Author

Gankra commented Jun 4, 2013

Having a straight split between save and autosave is probably easier, yeah.

Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 4, 2013
Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 4, 2013
Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 4, 2013
Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 4, 2013
Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 4, 2013
Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 5, 2013
Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 5, 2013
Gankra pushed a commit to Gankra/EpicEditor that referenced this issue Jun 5, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants