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

Local changes #21

Merged
merged 6 commits into from
Jul 24, 2015
Merged

Local changes #21

merged 6 commits into from
Jul 24, 2015

Conversation

HipsterBrown
Copy link
Contributor

This PR contains the tests and initial implementation of .hasLocalChanges. By listening for internal change, push,and clear events, updates/destroys a localStorage reference array of ids when needed.

Issues

Fixes #19
Fixes #6

@gr2m
Copy link
Member

gr2m commented Jul 23, 2015

looking good 👍

Can you add tests / implementation to pass an id / object as filter?

hoodie.store.hasLocalChanges('foo')
hoodie.store.hasLocalChanges({id: 'foo'})

@HipsterBrown HipsterBrown force-pushed the local-changes branch 2 times, most recently from f3f01f8 to 98ae5f2 Compare July 23, 2015 16:36
store.add([localObj1, localObj2])

.then(function () {
// 'change' event fires after Promist is returned so delay is needed before checking for changes
Copy link
Member

Choose a reason for hiding this comment

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

This could potentially confuse developers :/ Any way we can avoid that?

Think for example something like hoodie.update(obj).then(render) and render would be a sync method that would render the object detail view. If we'd have an store.hasLocalChanges(obj) in the render method, it would return false, which is not the expected behavior right after a hoodie.update call ... hmmmm :/

@gr2m gr2m merged commit 71b06d6 into master Jul 24, 2015
@HipsterBrown HipsterBrown deleted the local-changes branch July 24, 2015 20: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.

None yet

2 participants