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

feature request: throttle setting for autosave #29

Closed
zeroasterisk opened this issue Jun 15, 2016 · 10 comments
Closed

feature request: throttle setting for autosave #29

zeroasterisk opened this issue Jun 15, 2016 · 10 comments
Assignees
Labels
Type: Feature New features and feature requests

Comments

@zeroasterisk
Copy link
Contributor

zeroasterisk commented Jun 15, 2016

When typing in a value, I don't want to autosave every character (when typing fast).

I'd like to wrap the autosave onSubmit with something like _.throttle(onSubmit, 2000)

@serkandurusoy
Copy link
Contributor

serkandurusoy commented Jun 15, 2016

onSubmit is just a function call and you can do your throttling within that function itself. in fact you can throttle, debounce, mix and match as you like.

In fact that's a pattern I use extensively with uniforms.

@radekmie
Copy link
Contributor

I agree with @serkandurusoy - you can do it by yourself, but I'll consider an autosaveDelay prop - that might be handy and more user-friendly. I'll close it for now and maybe reopen it in the future.

@radekmie radekmie added the Type: Feature New features and feature requests label Jun 15, 2016
@serkandurusoy
Copy link
Contributor

@radekmie @zeroasterisk I think we should revisit this issue.

We just hit a probem where, autosave combined with a debounce handler that wraps the forms submit handler creates a unique problem!

When we have an array of strings or objects and "add" a new record on an "update" form where we provide a model prop that contains the live document from the collection (reactive), the debounce can cause a race condition where a new input is added to the form, but an empty string/object is not added to the underlying collection.

This causes different problems:

  • Sometimes, one needs to click twice on the plus icon to add new lines!
  • Sometimes, the newly created input does not contain a new unique key and react complains!

So, probably, the proper way to solve this problem would include:

  • implement throttle and debounce functions for the onSubmit handler within the form component
  • listen for the model changes such that if a change involves the length of an array field increasing (not decreasing, since decreasing is an actual change, but increasing is adding an empty input that may or may not be used, otherwise cleaned), bypass the throttle/debounce

What do you guys think?

@radekmie
Copy link
Contributor

That's interesting, @serkandurusoy. I know it won't be doable within minutes, but could you create a repository (or at least a gist) with reproduction? That one for #22 was very useful.

In spite of all - I'll try to come up with something.

@radekmie radekmie reopened this Jun 17, 2016
@radekmie radekmie self-assigned this Jun 17, 2016
@serkandurusoy
Copy link
Contributor

@radekmie I'll try to do that and if I can't I'll ask @guciek27 for help on that.

In the meanatime, as an additional note to my solution suggestion:

I think we can track "new array inputs";

  • using explicit state which gets set by the "plus" button itself.
  • instead of trying to implicitly detect it in the change event (because tracking deeply nested arrays might be slow)

@radekmie
Copy link
Contributor

Any progress?

@radekmie
Copy link
Contributor

Hey, @guciek27, @serkandurusoy - any progress? I've tried to reproduce it but I didn't find out anything.

@serkandurusoy
Copy link
Contributor

@guciek27 I did not have time to work on simplifying your reproduction
repo, is it possible that you finisih it up so that radek can have a look?
thanks!

On Thu, Jun 30, 2016 at 7:07 PM, Radosław Miernik notifications@github.com
wrote:

Hey, @guciek27 https://github.com/guciek27, @serkandurusoy
https://github.com/serkandurusoy - any progress? I've tried to
reproduce it but I didn't find out anything.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#29 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AEbz3HxSssrcMOioxTri8grtzVeJmaxqks5qQ-ndgaJpZM4I2uG3
.

@zeroasterisk
Copy link
Contributor Author

update - I've passed a throttled handler function down from container --> page --> AutoForm until internally supported

  // AutoForm.onSubmit handler
  //   gets all form values as an object
  const handleSubmit = _.throttle((values, cb) => {
    ...
  }

though I still like the idea of it being internally throttled to some sane default, with a configuration option... just to simplify adoption.

AutoForm has internal throttling

@radekmie
Copy link
Contributor

@zeroasterisk
I've added an autosaveDelay property (with default 0, which means no delay at all, so it remains synchronous). It works more like _.debounce, no _.throttle because there's no need to save meanwhile writing a long text in an input.

@serkandurusoy, @guciek27
Your use case is something that we could call a live document and it's more complex. If you want, please create another issue - I've got some experience with such things, so we can discuss it more in-depth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

No branches or pull requests

3 participants