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

fix for depending on css context as same than javascript context #234

Closed
wants to merge 3 commits into from

Conversation

MetalArend
Copy link

Added a new option "document". The current version 2.0.1 has a problem, where loading the spin.js javascript code from another frame, added the css rules to that other frame. Basically it came down to wrongly relying on the "document" where the javascript is running as the default setting.

I added this for use of spin.js in a chrome extension, but I'm sure this bug also happens when using iframes, multiple frames, or other stuff like that.

@MetalArend
Copy link
Author

I realise I changed the position of a few things, as it was a pain to get around some stuff. Hope it is all clear (also tried to keep your coding style), but if there are some things I did completely wrong, let me know.

@dinhvh
Copy link

dinhvh commented May 28, 2014

I need to give it a try.

@@ -18,17 +18,19 @@

var prefixes = ['webkit', 'Moz', 'ms', 'O'] /* Vendor prefixes */
, animations = {} /* Animation rules keyed by their name */
, useCssAnimations /* Whether to use CSS animations or setTimeout */
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable was only used in the spin function, when declaring a new Spinner. Depending on the document you load it in, this could change, so you can better check it when initialising the instance, not when loading the code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. This can only change if the browser changes, which is not possible.

@dinhvh
Copy link

dinhvh commented May 29, 2014

Tested! It just works as expected!
@MetalArend Could you create the minified version too?
@fgnass Can you look into merging it?

@MetalArend
Copy link
Author

You should look at the update. I changed the animations name cache. I had a bug, where the spinner would stop working when the front document was reloaded, without reloading the background document. The update looks at the animations names for the spinner, and removes them from the global Spinner names array, so they will be readded when a new Spinner is created.

@theodorejb
Copy link
Collaborator

spin.js v3.0 no longer injects a CSS sheet into the document head, so this should no longer be an issue.

@theodorejb theodorejb closed this Nov 10, 2017
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

Successfully merging this pull request may close these issues.

None yet

4 participants