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

Angular module #46

Closed
afknapping opened this issue Feb 12, 2014 · 28 comments
Closed

Angular module #46

afknapping opened this issue Feb 12, 2014 · 28 comments
Labels

Comments

@afknapping
Copy link

I use angularLocalStorage because it makes synching scope data to the LocalStorage a oneliner:

storage.bind($scope,'data');

Would be awesome to have this for LocalForage, too.

@tofumatt
Copy link
Member

That looks like a synchronous API... I know EmberJS has support for promises in it's data storage drivers, what about here?

@afknapping
Copy link
Author

Do you mean if angularLocalStorage has it? Cannot find anything about that in the docs, so guess no. Would of course be nice to have to give user feedback....

@ocombe
Copy link
Contributor

ocombe commented Feb 14, 2014

I'm writing an angularJS service based on localForage, I'll let you know once it's out !

@tofumatt
Copy link
Member

@ocombe That sounds great! If you'd be willing to submit it as a pull request here that'd be amazing; I'm writing an EmberJS one in my spare time and would like to include the drivers for frameworks in the src/adapters/ folder

@jimthedev
Copy link

@ocombe, Excited to see that angular service.

@ocombe
Copy link
Contributor

ocombe commented Feb 17, 2014

It's done, I'm testing it at the moment on my project at work.
I wanted to give it the same functionalities that https://github.com/grevory/angular-local-storage but localforage doesn't give you any way to list all current keys, you can just request one if you know its name, so it will miss a few features (clearAll will wipe all storage data for this domain, not just the one related to your project, and you won't be able to get an array of the current keys)

@ocombe
Copy link
Contributor

ocombe commented Feb 17, 2014

And I should take a look at https://github.com/agrublev/angularLocalStorage to add this bind feature, it seems very nice !

@peterbe
Copy link
Contributor

peterbe commented Feb 18, 2014

Here's how I solved it:
https://github.com/peterbe/buggy/blob/master/client/static/js/angularForage.js
That could perhaps we made into a proper module so that you don't have to manually mention the $scope. To me it's important to be able to choose between angularForage and localForage because in some callbacks I don't want it to digest the scope until I decide to do so myself.

@ocombe
Copy link
Contributor

ocombe commented Feb 20, 2014

My module is available here : https://github.com/ocombe/angular-localForage
I'll make a pull request to add it to this repository once you guys have tested it and found it good enough !

@ocombe
Copy link
Contributor

ocombe commented Mar 14, 2014

Have you guys tried my angular module ? Do you think that it's stable/good enough to be merged with this repository ?

@magalhas
Copy link
Contributor

I'm not an Angular user though by looking at your source code I think you should try to use UMD.

@ocombe
Copy link
Contributor

ocombe commented Mar 14, 2014

Angular has it's own module system and is not compatible with UMD

@magalhas
Copy link
Contributor

I had no clue. Sorry about that :)

@ocombe
Copy link
Contributor

ocombe commented Mar 14, 2014

no problem :)

@tofumatt
Copy link
Member

I'm not a huge angular user, so I feel like a bad person to judge it. Maybe I can check with some folks and if not evaluate it myself? But I am not opposed to seeing more adapters in here.

@peterbe
Copy link
Contributor

peterbe commented Mar 14, 2014

I think @ocombe's repo is looking great and I personally believe it works as a great resource for those of us who like to combine localforage in angularjs code.
But I do not think it belongs here in this project. Why add angular support and not React or Backbone or TheNextBigThing.
Keep it small and to the point or else, in a years time, someone will look at this and think it's bloated and re-write their own clone.

@tofumatt
Copy link
Member

We have Backbone support, and I'm happy to include adapter support right from our repo. It's not added to the library size, and if there's community support for adapters they can stay here.

@jimthedev
Copy link

Great stuff. I agree that adapters probably don't belong in core. With that
said, I'd love to see this rolled into a yeoman generator.

On Mar 14, 2014, at 5:52 PM, Peter Bengtsson notifications@github.com
wrote:

I think @ocombe https://github.com/ocombe's repo is looking great and I
personally believe it works as a great resource for those of us who like to
combine localforage in angularjs code.
But I do not think it belongs here in this project. Why add angular support
and not React or Backbone or TheNextBigThing.
Keep it small and to the point or else, in a years time, someone will look
at this and think it's bloated and re-write their own clone.

Reply to this email directly or view it on
GitHubhttps://github.com//issues/46#issuecomment-37705597
.

@austinpray
Copy link

I'd love to see this rolled into a yeoman generator

Why would you do this over managing the dependency via bower? Or am I misunderstanding?

@jimthedev
Copy link

I was specifically referring to angular+localforage as an app skeleton.
Yeoman generators use bower to install their dependencies so it'd be pretty
slick. Throw in bootstrap 3 or ratchet 2 and you've got an app skeleton.

On Mar 14, 2014, at 7:34 PM, Austin Pray notifications@github.com wrote:

I'd love to see this rolled into a yeoman generator

Why would you do this over managing the dependency via bower? Or am I
misunderstanding?

Reply to this email directly or view it on
GitHubhttps://github.com//issues/46#issuecomment-37710229
.

@ocombe
Copy link
Contributor

ocombe commented Mar 15, 2014

I listed localforage as a bower dependency but we would need release numbers to make it safe.
For example the "driver as a function" commit from 2 days ago was a breaking change, but I couldn't specify that my plugin was working with version 0.0.1 and not 0.0.2 in the bower.json file.
If someone had installed it in the mean time, it would have been broken :(

@ocombe
Copy link
Contributor

ocombe commented Mar 15, 2014

Anyway, I have nothing against keeping it as a separate repository, but we should at least list it in the doc for people who might be looking for it :)

@tofumatt
Copy link
Member

I added a version 0.1.0 tag earlier today, and will be mindful of API breakage in the future.

In terms of yeoman generators, that's outside the scope of this discussion. I do want angular adapters, but @potch and I were even discussing if that requires a separate repo to separate them into different bower components.

But the point is that I'll look at the module and see if it can be blessed as out driver.

Matthew Riley MacPherson (Sent from mobile)

On Mar 14, 2014, at 23:04, Olivier Combe notifications@github.com wrote:

I listed localforage as a bower dependency but we would need release numbers to make it safe.
For example the "driver as a function" commit from 2 days ago was a breaking change, but I couldn't specify that my plugin was working with version 0.0.1 and not 0.0.2 in the bower.json file.
If someone had installed it in the mean time, it would have been broken :(


Reply to this email directly or view it on GitHub.

@tofumatt
Copy link
Member

Regarding linking to it were it a separate repo: absolutely!

Matthew Riley MacPherson (Sent from mobile)

On Mar 14, 2014, at 23:05, Olivier Combe notifications@github.com wrote:

Anyway, I have nothing against keeping it as a separate repository, but we should at least list it in the doc for people who might be looking for it :)


Reply to this email directly or view it on GitHub.

@magalhas
Copy link
Contributor

Well this ressembles my suggestion of having Backbone adapter on a different repo as well. Lets be coherent and whatever it's decided lets do it for all the adapters.

@jimthedev
Copy link

@tofumatt @magalhas @ocombe I agree that a separate repo for adapters seems reasonable. If I'm using this for a mobile app, I really want to keep it as small as possible. Also, I don't want to have to clean up the repo after using bower to install, just to get back some of that space used by other adapters that I'm not interested in. Seems reasonable to have a section in readme.md where adapter creators can list their adaptors. Perhaps something like how Dokku documents their plugins: https://github.com/progrium/dokku/wiki/Plugins

@austinpray
Copy link

+1 for separating out adapters. Would be easy to manage everything with bower.

@tofumatt
Copy link
Member

Fixed with #107. Thanks @ocombe!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants