FIX: Remove loading spinner after image export

[VladimirMikulic]

In the old UI, the spinner would continue spinning after the images have been
exported. Now it's fixed.

Resolves #470

@sashadev-sky could you review this, please?

[welcome]

Thanks for opening this pull request! Dangerbot will test out your code and reply in a bit with some pointers and requests.
There may be some errors, but don't worry! We're here to help! 👍🎉😄

[chen-robert]

Is there a way to test this without waiting for the download to complete? It's been taking a very long time for me (over 15 minutes) for some reason.

[chen-robert]

You could probably move the data.jpg !== null condition inside the data.status === 'complete' to make the code cleaner.

Something like

if (data.status === 'complete') {
        L.IconUtil.toggleXlink(link, 'get_app', 'spinner');
        L.IconUtil.toggleTitle(link, 'Export Images', 'Loading...');
        L.DomUtil.removeClass(link.firstChild, 'loader');

        if (data.jpg !== null) {
    

[sashadev-sky]

@chen-robert @VladimirMikulic SORRY should have mentioned this. The exporter never actually completes locally, it needs to be up on github pages. For my testing, I use https://sasha.mapknitter.org/examples/select.html. I added @VladimirMikulic work there and it works, you can check it out!

The exporter is really the only thing you would need you own page for, but if you plan on helping out with it you might want to get one setup! You will need a custom domain which I think @jywarren can provide you guys with, and I'll walk you through how to do it. Let me know!

[sashadev-sky]

@VladimirMikulic This is really great! My one concern here is that a lot of people using this library use it without our toolbar, and bind the action methods to their own custom UI. So, we would want to keep all logic related to the toolbar outside of the export functionality, in ExportAction.js. Also just promotes modularity.

Maybe by doing something like a Promise with startExport and moving those lines into a then?

Also @chen-robert your point is valid, but @VladimirMikulic didn't write the conditional, so refactoring them is technically outside the scope of this. But if he/ you wants to fix them -- all for it!

[VladimirMikulic]

@sashadev-sky I wrapped startExport function inside of a promise and tested it.
It works as expected.

[VladimirMikulic]

@sashadev-sky I would be thankful if you could review this.

[sashadev-sky]

this is really impressive!! Thank you! It does work.

Our library actually doesn't support ES6 😣Meaning we don't have a module bundler like webpack to compile the code back to ES5 - and it might not run in some browsers

We could polyfill the Promise but then were still left with the arrow functions.
options would be to

1. set the library up with the module bundler of your choice (this would be considered another hard task)
2. add a polyfill for the Promise and replace the arrow functions with ES5 syntax (not another task)
3. accomplish this Promise idea using an ES5 compliant way (not sure what this would be exactly)

[VladimirMikulic]

@sashadev-sky polyfill added!

[sashadev-sky]

@VladimirMikulic awesome thank you! So you're still using an arrow function here in the Promise so that needs to be updated. And then one more thing: run $ grunt concat:dist to build the dist file with your changes and commit that. Typically I develop with $ grunt running in the background in another terminal tab and it automatically builds changes (although occasionally it does not for some reason)

[VladimirMikulic]

@sashadev-sky I missed it, sorry, but here is the improved version.

[SidharthBansal]

@chen-robert @jywarren can you please review?

[VladimirMikulic]

@sashadev-sky ?

[jywarren]

This is great, can you share a gif of this working, or a link to it running in a GitHub pages branch? Thanks!

[VladimirMikulic]

@jywarren @sashadev-sky here is the showcase video: https://streamable.com/ye1oz

[jywarren]

This is wonderful! Thanks so much, great work!

[welcome]

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will likely be published to https://mapknitter.org in the next few days.
In the meantime, can you tell us your Twitter handle so we can thank you properly?
Now that you've completed this, you can help someone else take their first step!
See: Public Lab's coding community!

[VladimirMikulic]

@jywarren could you also approve the task on GCI, so I can move on?
Thanks.

[SidharthBansal]

Pls give me link. I will approve it

…

On Sat, 14 Dec 2019, 9:47 pm Vladimir Mikulic, ***@***.***> wrote: @jywarren <https://github.com/jywarren> could you also approve the task on GCI, so I can move on? Thanks. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#477?email_source=notifications&email_token=AFAAEQZ3VXFGW2LKYSIDZRTQYUBJFA5CNFSM4JYBCPCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG4F5OY#issuecomment-565731003>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFAAEQ44CGMOF6V6QE6J6ZDQYUBJFANCNFSM4JYBCPCA> .

[VladimirMikulic]

Thank you @SidharthBansal.