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

Position breaks when the reference has an SVG element directly inside #653

Closed
diegohaz opened this issue Jul 22, 2018 · 2 comments
Closed
Labels
bug Something is not working. PRIORITY: low TARGETS: core Utility functions, lifecycle, core library stuff.

Comments

@diegohaz
Copy link

CodePen demo

https://codepen.io/anon/pen/djNawe

(remove the SVG element to see the difference)
(if you wrap the SVG in a div, it also works)

Steps to reproduce the problem

  1. Put the popover inside its reference;
  2. Make the reference absolutely positioned and give it some top and left values;
  3. Put an SVG element inside the reference.

What is the expected behavior?

Removing the SVG element makes it work as expected:
image

What went wrong?

When it has an SVG element, it breaks:

screen shot 2018-07-21 at 21 07 14

Any other comments?

I'm trying to integrate Popper.js on my component library (https://reakit.io).

@piglovesyou
Copy link

piglovesyou commented Jul 28, 2018

I took a look. isOffsetContainer checks if #ref is an offset container by doing getOffsetParent(refEl.firstElementChild), but svgEl.offsetParent does not exist and it goes wrong.

This issue would be solved when we change getOffsetParent(refEl.firstElementChild) to something like getOffsetParent(getFirstValidOffsetContainee(refEl)), although I'm not sure if all children are invalid (= no .offsetParent). Text Node also doesn't have .offsetParent by the way.

The issued DOM tree FYI:

  • #ref
    • svg
    • (text node)
    • #popper

I think getOffsetParent(getFirstValidOffsetContainee(refEl)) would be OK as long as we don't support a case of #ref > svg#popper.

piglovesyou added a commit to piglovesyou/popper.js that referenced this issue Jul 28, 2018
…correctly

Some element (eg. svg) does not have `.offsetParent` so we can try the
nextElementSibling of it
@piglovesyou
Copy link

No, getOffsetParent itself has a functionality to skip and try the nextElementSibling. I'll try this.

@FezVrasta FezVrasta added bug Something is not working. TARGETS: core Utility functions, lifecycle, core library stuff. PRIORITY: low DIFFICULTY: low labels Jul 28, 2018
piglovesyou added a commit to piglovesyou/popper.js that referenced this issue Jul 28, 2018
…correctly

Some element (eg. svg) does not have `.offsetParent` so we can try the
nextElementSibling of it
piglovesyou added a commit to piglovesyou/popper.js that referenced this issue Jul 28, 2018
…correctly

Some element (eg. svg) does not have `.offsetParent` so we can try the
nextElementSibling of it
piglovesyou added a commit to piglovesyou/popper.js that referenced this issue Jul 28, 2018
…correctly

Some element (eg. svg) does not have `.offsetParent` so we can try the
nextElementSibling of it
FezVrasta pushed a commit that referenced this issue Aug 28, 2018
fixes #653 

Some element (eg. svg) does not have `.offsetParent` so we can try the
nextElementSibling of it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working. PRIORITY: low TARGETS: core Utility functions, lifecycle, core library stuff.
Projects
None yet
Development

No branches or pull requests

3 participants