#56 closed enhancement (fixed)
Add support for callbacks
| Reported by: | drew | Owned by: | madair |
|---|---|---|---|
| Priority: | major | Milestone: | 1.0.2 |
| Component: | core | Version: | 1.0.0 |
| Keywords: | ajax loader | Cc: | drew.wells00@… |
Description
Currently users must poll a 'readyToUse' flag in order to use a dynamically added definition. By adding support for callback, they can be assured their projection has loaded already. I have added a simple enhancement to the constructor API to support this.
This will, however, not capture the case of failing to load a definition (which is another enhancement needed).
The API looks like this new Proj4js.Proj( projCode, callback );
The callback is pushed into a queue, when defsLoaded is run it activates these callbacks. A more advanced AJAX promise object would be ideal, but may be overkill.
Attachments (3)
Change History (14)
comment:1 by , 13 years ago
comment:2 by , 13 years ago
Drew this patch looks good and I don't have a timeline for 1.0.2 but this patch is worth a new release. I can do a new release when I get back from vacation in July.
comment:3 by , 13 years ago
Thanks for getting back to me.
I've fixed a bug in this patch, I'll get this submitted on Monday (I've forgotten how I made the diff). We had some wrapping logic for the Proj constructor that is no longer needed with my new patch.
comment:4 by , 13 years ago
As promised, attached updated version of queueing. There is still an issue that if multiple calls are made for a Projection before any finish, that this projection will be called multiple times. Unfortunately, a clean solution to this problem would take a serious refactor of the library. I have a hack in my client code to workaround this problem.
comment:5 by , 13 years ago
| Status: | new → assigned |
|---|
I'm having trouble with this diff file because it seems to be generated against a local svn copy (e.g. there is no diff line in the Proj constructor that includes the callback argument) and I'm afraid of missing something. Can you re-generate the patch against the trunk version of proj4js.js or attach the whole file and I can diff against trunk from that?
comment:6 by , 13 years ago
Sorry I was probably using the official release instead of trunk. Here's the entire file we're using. Let me know if I need to merge trunk and test again.
by , 13 years ago
| Attachment: | callbacks.patch added |
|---|
comment:7 by , 13 years ago
the patch file shows what would be committed, seems to work for me now. Can you see if I missed anything?
comment:8 by , 13 years ago
I noticed there's a bug with the queue. Each time a Proj is created it should instantiate it's own queue. this.queue =[] will do that, so these lines should be removed: 515-522.
comment:9 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:10 by , 13 years ago
slight change to the patch, only call the callbacks after projection code init
comment:11 by , 13 years ago
Not sure why I tested the queue twice probably chasing a bug I didn't fully understand. All of our unit tests passed, this changes looks good on my side.
Thanks for getting this in so fast!
Is there a date for milestone 1.0.2. Perhaps I could lend some assistance to adding this feature. I believe I have written a better version of this diff that is working well for us internally.