julien-maurel / jQuery-Storage-API

This plugin is no longer maintained! A new one, without dependencies with jQuery, is available : https://github.com/julien-maurel/js-storage
308 stars 83 forks source link

Add isEmpty, keys methods. Added Jasmine tests for new and old methods. #2

Closed swanjr closed 11 years ago

swanjr commented 11 years ago

Hello,

I found your library very helpful but needed to make a few modifications for easier use.
1) Added isEmpty method returns true if the storage is empty. 2) Added keys to returns the keys currently in the storage. 3) Added Jasmine tests for existing and new methods. 4) Added boolean to deleteAll. If you pass in true now it will now delete all the variables in storage even if they are outside of the namespace. 5) Reimplemented cookie_storage so it works more like the build in window.localStorage object. This allows all the methods to work correctly with the cookie storage now, such as deleteAll. You can also now use a for loop and Object.keys() with cookie_storage and it will function correctly. 6) Removed the get, set, delete documentation examples for passing in multiple variable arguments outside of an array. For example, get("item1", "item2", "item3"). I was not able to get this syntax working even before I started making modifications. The multiple argument in an array syntax works correctly, however.

I hope you find these changes helpful. Let me know if you have any questions.

Thanks. Josh

swanjr commented 11 years ago

Couple of other notes:

Josh

julien-maurel commented 11 years ago

Hi, Thanks for your contribution. I added some comments on your request. For the moment, I don't merge it. But I will do it when I have time. If you change something else after my comments, I continue to look it :)

PS : But I pushed your fix on set function because that was quick. Thanks

swanjr commented 11 years ago

Thanks for the feedback. I'll take a look at your suggestions when I get a moment. As for the multiple argument syntax, ex. $.localStorage.set('a','b','c',1). I still can't get them working in Firefox where I am testing. In what browser is it working for you?

julien-maurel commented 11 years ago

Strange. I tested on Chrome and Firefox (20.0.1), and it's good on both (I use the last version (1.3.1) (issue before on set function))

> $.localStorage.set('a','b','c',1)
Object {b: Object}
> $.localStorage.get('a','b','c')
1
swanjr commented 11 years ago

Ok, I'll double check it on my version of Firefox again, which is 21.0.

Also, I was looking at removing the deleteAll(boolean) from the namespace storage like you suggested and just use $.localStorage.deleteAll() instead. However, if I use $.localStorage to delete everything inside and outside of a namespace storage it deletes the namespace storage object itself. This breaks the namespace storage internally and it no longer works. That is why the new deleteAll(true) reinitializes the namespace object to an empty object.

julien-maurel commented 11 years ago

I began to merge it. About Object.xxx, I removed it (not only keys), because all is not compatible with ie<9.

For isEmpty & keys, I will just add code to support chain parameters, and I will add test to Jasmine.

About deleteAll, I will do something a little different. like this : deleteAll(reinit)

You will notify when I will push version 1.4 ;) Thanks for your support!

swanjr commented 11 years ago

Ok, sounds good. I had started removing the Object.keys references but was to busy to finish. I look forward to the next release.

Thanks.

julien-maurel commented 11 years ago

1.4.0 is out :) Thanks