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(start()): locate parent before every 'start()' #56

Closed
wants to merge 2 commits into from

Conversation

networkerror
Copy link

When the (ng|ui)-view is attached to the body
And the route is changed
Then the body may be re-written out from under the loading bar
And the $parent pointer will be left in a bad state

By finding the $parent every time the loading bar is started,
it ensures that the loading bar's <div> elements aren't attached
to a stale parent.

I discovered this bug on a project I'm working on. I tested this
fix, and it seemed to do the trick. The loading bar still vanishes
part-way through a route change, but it begins working again
the next time start() is called. (Prior to this fix, it did not.)

More details: We're using ui-router and attaching our app to the
body element. <body ui-view>...</body>

When the ng-view is attached to the body
And the route is changed
The the body may be re-written out from under the loading bar
And the $parent object will be invalid

By finding the $parent every time the loading bar is started,
it ensures that the <div> elements aren't attached to a stale
parent.
@networkerror
Copy link
Author

If this needs more work before it can be merged, please let me know. :)

@chieffancypants
Copy link
Owner

Sorry for the delay and thanks for the help!

So this is related to a change I thought I had already made (4937d6c) but apparently never merged. It turns out it's a bad idea to allow someone to specify where in the DOM the loading bar gets inserted because in addition to what you noticed, it can also cause bugs like #12 and even another project of mine had similar issues chieffancypants/angular-hotkeys#8.

This seems like a simple enough work around for the time being though. Thanks for the PR!

@chieffancypants
Copy link
Owner

Landed in 124d631

@networkerror
Copy link
Author

Thanks, Chief! :D

On Fri, May 2, 2014 at 9:20 AM, Wes Cruver notifications@github.com wrote:

Sorry for the delay and thanks for the help!

So this is related to a change I thought I had already made (4937d6chttps://github.com/chieffancypants/angular-loading-bar/commit/4937d6c135)
but apparently never merged. It turns out it's a bad idea to allow someone
to specify where in the DOM the loading bar gets inserted because in
addition to what you noticed, it can also cause bugs like #12https://github.com/chieffancypants/angular-loading-bar/issues/12and even another project of mine had similar issues
chieffancypants/angular-hotkeys#8chieffancypants/angular-hotkeys#8
.

This seems like a simple enough work around for the time being though.
Thanks for the PR!


Reply to this email directly or view it on GitHubhttps://github.com//pull/56#issuecomment-42049600
.

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.

2 participants