Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

fix: make overlay display over backdrop, by appending overlay to body #165

Closed
wants to merge 2 commits into from

Conversation

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@timblack1
Copy link
Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@valdrinkoshi
Copy link
Member

Hi @timblack1, the current proposal has several drawbacks, e.g. it breaks style encapsulation. Also, the fact that you reparent it will cause detach and attach to be triggered, potentially entering in an infinite loop.

@timblack1
Copy link
Author

Does https://github.com/PolymerElements/iron-overlay-behavior/blob/master/iron-overlay-backdrop.html#L99 break style encapsulation too? That is the line which necessitates a fix like the one proposed here.

@valdrinkoshi
Copy link
Member

Not really, as it is done if you explicitly call the method and if the element doesn't have a parent.
In essence it is suboptimal to attach, detach & attach elsewhere. This would cause query selectors to stop working, it would be a very breaking change.

I'm working on overcoming these stacking context related issues in the stacking-context branch. Happy to discuss the approach in a separate issue!

@timblack1
Copy link
Author

I don't see an infinite loop in my manual testing of this proposed change. I look forward to seeing a good resolution of the problem.

@valdrinkoshi
Copy link
Member

The PR with the proposal is this one #155
Closing this PR.

@timblack1 timblack1 deleted the patch-1 branch May 6, 2016 15:38
@alimd
Copy link

alimd commented Aug 9, 2016

its works but its not a good way for solving problem, its like a hack dom, and maybe make problem with data binding, etc ...

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

Successfully merging this pull request may close these issues.

6 participants