search

Esri / offline-editor-js

ArcGIS JavaScript library for handling offline editing and tiling.
http://esri.github.io/offline-editor-js/demo/
Apache License 2.0
159 stars 142 forks source link

v3.0.6 3/3/2016 #450

Closed robkspeer closed 8 years ago

robkspeer commented 8 years ago

Version 3.0.6 - March 3, 2016

No breaking changes

Bug Fixes

robkspeer commented 8 years ago

Agreed, the only time the else would come in is in an error. And, if the database can't be opened, we're not going to get very far adding anything to it anyway!

I do think a default of layer._nextTempId = -1 needs applied somewhere, just as protection from anything funny happening.

That could happen in the else, or assigning it before calling _editStore.getNextLowestTempId() as a default. I just didn't know how the timing of the callback would effect as the code moves along? I think putting a deferred in there and waiting for it to resolve might be overkill.

Where would you like to see the default of -1 be assigned? I'll go along with the coding pattern you prefer and rebuild with the proper uglify.options.preserveComments: /^!/ parameter for this PR.

andygup commented 8 years ago

I do think a default of layer._nextTempId = -1 needs applied somewhere, just as protection from anything funny happening.

I believe that you built that into the database query and it will return -1 by default, right? Reference: https://github.com/Esri/offline-editor-js/pull/450/files#diff-642819f962c2a421135dc243e333877aR574

I just didn't know how the timing of the callback would effect as the code moves along?

Should be okay, since it's already wrapped in a deferred. Reference https://github.com/Esri/offline-editor-js/blob/master/lib/edit/OfflineEditBasic.js#L130

andygup commented 8 years ago

@robkspeer How's it looking on the pull request?

robkspeer commented 8 years ago

Sorry Andy, this got away from me.

For #451, test/SpecRunner.offlineTilesAdvanced.TokenBased.html shows that the new optional dbConfig.offlineIdManager parameter can be supplied, or if that line is commented out, the localStorage name will default to "offline_id_manager."

commit 574b8b9eb838651c3042005461b16ec8232ca27c should wrap this up.

andygup commented 8 years ago

Awesome job, thanks for the pull request!

Just a few steps remaining. I can wrap up pushing the gh-pages update, creating a release and pushing to npm. It's truly no problem on my end, it will only take a 2 - 3 minutes.