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

Debounced validation functionality #92

Merged
merged 12 commits into from
Jan 6, 2016
Merged

Conversation

offirgolan
Copy link
Collaborator

In regards to #57 & #58

var Validations = buildValidations({
  foo: validator('custom-ajax', {
    debounce: 500
  })
});

@stefanpenner @rwjblue let me know what you guys think of this.


assert.equal(object.get('validations.isValid'), false, 'isValid was expected to be FALSE');
// TODO: I feel like initially a debounced validation should not be debounced.
assert.equal(object.get('validations.isValidating'), true, 'isValidating was expected to be TRUE');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right when we create the object, the lastName debounced promised is created which sets the isValidating to true

@stefanpenner
Copy link
Collaborator

does anything cancel debounces, if the object is destroyed?

@offirgolan
Copy link
Collaborator Author

Ah. No there is not cleanup. Let me do that 😄

@offirgolan
Copy link
Collaborator Author

@stefanpenner all done. Let me know if you approve 👯

@stefanpenner
Copy link
Collaborator

@offirgolan ping me around noonish tomorrow, i gotta clear my queue of other stuff before i do a thorough review.

@offirgolan
Copy link
Collaborator Author

Will do!

let cache = getDebouncedValidationsCacheFor(model);
// Return a promise and pass the resolve method to the debounce handler
value = new Promise(resolve => {
cache[attribute] = run.debounce(validator, getValidationResult, validator, options, model, attribute, resolve, debounce, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We likely need to deal with the > 1 debounced per attribute, and also > 1 debounced per attribute per type (2 ajax validators for 1 attribute example).

@offirgolan
Copy link
Collaborator Author

@stefanpenner seems like travis keeps failing because it cant find Map since I started using new Map() for my cache... do you have a workaround on this? Or should I just scrap using Map?

@offirgolan
Copy link
Collaborator Author

I decided to revert back to just using pojos as the caches. I really dont think there is much of a need to create another class for such a simple use case. In the future, if it becomes needed, ill make the refactor and hopefully by then es6 Map will be more widely supported (phantomjs included 😞 ).

As for the willDestroy, again, I dont think it's very complicated to need even more deconstruction... if you're still adamant on these let me know and we can chat 😄

offirgolan added a commit that referenced this pull request Jan 6, 2016
@offirgolan offirgolan merged commit 7433fdb into master Jan 6, 2016
@offirgolan offirgolan deleted the debounced-validations branch January 6, 2016 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants