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

Check if loading already exists on page #97

Open
JPustkuchen opened this issue Apr 1, 2016 · 1 comment
Open

Check if loading already exists on page #97

JPustkuchen opened this issue Apr 1, 2016 · 1 comment

Comments

@JPustkuchen
Copy link

Hej and thank you very very much for this great plugin. We love it!
Now here's a tiny little feature request that would make the plugin even better, as we think.

Context:
We're setting the loading element in DOM already to have a loading animation as early as possible. This works and looks really good.

Problem:
When animsition calls addLoading it doesn't check if an element with the loading class already exists on the page. So it's being added twice, which looks bad and doesn't make a lot of sense

Proposed solution:
in addLoading check if an element with the options.loadingClass already exists on the page and only add it, if it doesn't.
We've patched the plugin this way and it works wonderful, so we'd guess this might also be helpful for other users because the loading element doesn't make sense to be duplicated on the same page, as we think.

So here's the simple and tested code:

addLoading: function(options){
      if($('.' + options.loadingClass).length <= 0){
        $(options.loadingParentElement).append('<div class="' + options.loadingClass + '">' + options.loadingInner + '</div>');
      }
    },
@vzr314
Copy link

vzr314 commented Apr 12, 2016

Proposed solution:
in addLoading check if an element with the options.loadingClass already exists on the page and only add it, if it doesn't

I observed this tinny bug may times for myself, but thanks to your little fix loaders are now started properly. Tested on several instances with various web apps and it works everywhere.

AFAIC this should be committed to master branche.

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

No branches or pull requests

2 participants