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

optimizations.moveElement: false still moves DOM node #54

Closed
jgraichen opened this issue Jul 6, 2014 · 18 comments
Closed

optimizations.moveElement: false still moves DOM node #54

jgraichen opened this issue Jul 6, 2014 · 18 comments

Comments

@jgraichen
Copy link

Even with optimizations.moveElement set to false the DOM node is moved. That makes it extremely hard to use Tether with e.g. React that depends on a non-changing DOM.

In the example on tether.io the attachment is moved to the body too. The documentation indicates that it should not be moved.

Do I miss anything?

@jbrantly
Copy link

jbrantly commented Jul 8, 2014

I'm running into the same exact issue.

@svenssonaxel
Copy link

Same issue here. As far I can see, the string "moveElement" doesn't even appear in the source code.

@zackbloom
Copy link
Contributor

@svenssonaxel
Copy link

@zackbloom My bad, don't know how I missed it. Can you confirm the issue, though?

@davidosomething
Copy link

i think i fixed it in #65 -- you'll have to build to try it, but it works for me.

@jd327
Copy link

jd327 commented Dec 13, 2014

+1, doesn't work.

@dan-dr
Copy link

dan-dr commented Dec 21, 2014

same here

can be fixed by moving replacing lines 910 with 909 thus making the move inside the closing curly braces

@xdumaine
Copy link

+1 This is a big problem.

@xdumaine
Copy link

@zackbloom Can you merge #65 to get this fixed?

@zackbloom
Copy link
Contributor

I just want to clarify what moveElement is for. Tether does absolute positioning of elements. Elements are always positioned relative to one of:

  • The page (body)
  • The viewport
  • The element's offset parent (roughly the first non position: static parent)

When moveElement is off, that third option is disabled. The other two however are enabled, and at present, the element is always moved to be a direct child of the body in both cases. That move allows position absolute to work as we expect.

Perhaps there should be another option which disables all moving, but that will require some changes to the positioning logic, as if the element has parents that are not position: static, their positions will have to be taken into account. Additionally, if they move, the elements position will potentially be incorrect.

I apologize for the confusion, and am sympathetic to the issue, but moveElement works as it should (although it probably is misnamed). Unfortunately exact positioning of an element which could appear anywhere in the DOM is just not always possible in the way you hope (for example if a parent is overflow: hidden). I encourage you to make your elements direct children of the body, and set moveElement to false, if you do not wish them to be moved.

@xdumaine
Copy link

The real confusion here is that what you're saying completely contradicts
the docs:

We are moving where the DOM node is, so if you have CSS which styles
elements within the offset parent, you may see some rendering changes. Also
note that this optimization works best if the scroll parent is the offset
parent. In other words, the scroll parent should be made position relative,
fixed or absolute to enable this optimization.

If you do see stylistic changes occur when the element is moved, you
might want to disable this optimization. You can do that by setting
optimizations.moveElement to false.
On Thu, Jan 22, 2015 at 10:58 PM Zack Bloom notifications@github.com
wrote:

Closed #54 #54.


Reply to this email directly or view it on GitHub
#54 (comment).

@davidosomething
Copy link

Oh, well nuts.

Anyway I've actually been running in production with my edit in #65 live on Time.com for the past 6 months and it works the way I need it to, lol

The use case is when I render a flash object that is tethered, removing it from the DOM actually destroys the flash object and appending it causes a new one to be created.

@xdumaine
Copy link

My formatting got screwed up by email, but you get my point? The docs indicate that disabling moveElement means the node won't be moved in the DOM.

@zackbloom
Copy link
Contributor

That will work as long as you meet some criteria like not having positioned
parents, not having a parent that is overflow: hidden, etc.
Unfortunately we can't assume those constraints will be met by every usage.

On Thu, Jan 22, 2015 at 11:04 PM, davidosomething notifications@github.com
wrote:

Oh, well nuts.

Anyway I've actually been running in production with my edit in #65
#65 live on Time.com for the past 6
months and it works the way I need it to, lol

The use case is when I render a flash object that is tethered, removing it
from the DOM actually destroys the flash object and appending it causes a
new one to be created.


Reply to this email directly or view it on GitHub
#54 (comment).

@adamschwartz
Copy link
Contributor

@davidosomething first off, I want to say thanks for using Tether on Time.com. That’s awesome.

Unfortunately, Zack’s last point is correct, that not moving the element won’t work as a general solution for everyone.

I’d be in favor of renaming moveElement to something which better fits the description of what it does, and creating a new property (perhaps called moveElement, or perhaps something else to avoid confusion with its previous use) which does what you’ve put forth in #65, but with a caveat and warning in the docs that Using this option can cause Tether to not position properly but this option can be useful for situations in which <embed>, <object>, or <iframe> elements are descendants of the tethered element.

@davidosomething
Copy link

+1 on that since they serve different use cases, and I got lucky the lines to detach and reappend the the DOM nodes were in @Move()

Can you provide a working example of where the current moveElement optimization is needed (e.g. a demo of the failing case and then a working case with the optimization)? I think it would help to have the visual explanation since we all misunderstood its use.

@mik01aj
Copy link

mik01aj commented Mar 24, 2015

+1

@mik01aj
Copy link

mik01aj commented Aug 20, 2015

Guys using React, here's my solution: https://gist.github.com/mik01aj/5b98bad5b5ba04a1b93f

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

No branches or pull requests

10 participants