Skip to content

Allow empty parentSelector #244

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

Merged
merged 5 commits into from
Mar 16, 2016

Conversation

stefanotorresi
Copy link
Contributor

This patch builds on #218, which supersedes, because it wasn't handling all the cases.

This fixes #239, angular-ui/ui-router#2260 and #179 by appending the bar to the body tag when the specified selector (if any) is not available. The check happens when start() is invoked, so as soon as the selector is ready it will be used, without involving $timeout or such things.

@furyscript
Copy link

When this fix is merged into master?? Actualy this commit can fix this: angular-ui/ui-router#2260 and #239 ?

@jklaus
Copy link
Contributor

jklaus commented Feb 8, 2016

I have resolved the issue caused by the parentSelector implementation in pull 272. Feel free to pull from me.

@chieffancypants
Copy link
Owner

merged in #272

@stefanotorresi
Copy link
Contributor Author

Honestly, I don't think #272 actually resolves the issue this PR is about.
This PR replaces find usage with querySelector, which makes the selector more usable, and it uses body as a fallback selector.
Besides that has no tests so there is no way to know... :p

@chieffancypants
Copy link
Owner

@stefanotorresi querySelector doesn't have great support in older IEs, which I still have an obligation to support.

@stefanotorresi
Copy link
Contributor Author

Correct me if I'm wrong, but querySelector browser compatibility goes as far as Angular's.

@jklaus
Copy link
Contributor

jklaus commented Feb 24, 2016

@chieffancypants, out of curiosity how far back are you having to support? My company just recently moved from IE8 to current (adding Chrome & FF) ... finally.

@chieffancypants
Copy link
Owner

@stefanotorresi Yes, technically, but with work, you can get the 1.2 branch to work with IE8, which is what I've been doing. Perhaps we could rewrite the PR to use querySelector if available, that way assholes like myself supporting shitty browsers can still use find, but you cool kids can use all your fancy tricks.

@jklaus: We're still IE8. This should change very soon though, since MS has dropped support for it. My customers are all large enterprises, so they inevitably refuse to upgrade even when there is significant pressure to do so.

@stefanotorresi
Copy link
Contributor Author

@chieffancypants all right then, I'll update the PR right away to make it more old-browser friendly :)

@faceleg
Copy link
Collaborator

faceleg commented Mar 14, 2016

@chieffancypants approve?

@chieffancypants chieffancypants added this to the next version milestone Mar 16, 2016
chieffancypants added a commit that referenced this pull request Mar 16, 2016
@chieffancypants chieffancypants merged commit 6b4c469 into chieffancypants:master Mar 16, 2016
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.

Loading bar don't find parentSelector
5 participants