Adds an option to disable moving tethered element to the body #96
Conversation
pzuraq
force-pushed
the
add-option-to-disable-move-element
branch
from
f42b79b
to
6979d94
Compare
|
I'm not really sure I understand how tether can work without moving elements into the body. The basic premise is that it positions it's element absolutely, usually relative to the body. There are specific cases where it can position relative to a parent element instead, but you are not forcing it to use that mode (https://github.com/pzuraq/tether/blob/add-option-to-disable-move-element/src/js/tether.js#L715-L726). If you apply this change, and any of the tethered elements parents are not |
|
@zackbloom Thanks for the quick response! Completely understand, and at first it does seem a little counterintuitive but ultimately we are trying to position relative to the body, just a couple elements deeper. The HTML structure looks something like this: <body>
<div id="animation-container">
<div class="tethered-element"></div>
</div>
<div>
<!-- Rest of the application -->
</div>
</body>#animation-container {
position: absolute;
top: 0;
left: 0;
}If the tether is attached to Being able to attach and remove tether's to specific elements allows us to encapsulate our animation logic in other web components. Ember in particular makes use of the |
|
I understand, but this code doesn't enforce the invariant that no parent of the element is positioned. I would suggest that rather than using an option, you instead iterate through the elements parents, and if they are all |
|
@zackbloom That sounds like an excellent idea! Will close this PR and submit a new one with that behavior instead. |
|
There's another case where having an option to define a different parent element would be extremely helpful and that's testing Ember applications. In testing the Ember app typically renders the whole application into an element somewhere below the body (usually sth. like In that case it would be really nice if we could limit tether to the testing container. Our current quick fix is to simply not use tether in testing mode but that doesn't really seem so nice. |
|
@marcoow I'm getting around this at the moment by setting the positioning of the testing container to static instead of relative. There don't appear to be any differences in performance, though I suppose absolutely positioned elements which are not |
|
The problem I'm having isn't really that positioning is wrong or so but the fact that tether moves the positioned element under the |
|
@marcoow Yes, and if you set the testing container to It's somewhat hack-y, and may cause other side-effects due to a lack of relative positioning for the rest of the container, but the tether element isn't moved which means that it still acts as normal in the body. |
|
I'm not actually talking about the positional parent element but about the actual parent element in the DOM. Tether actually moves the element in the DOM which results in problem for me. |
|
@marcoow Yes, I know. I am talking about the PR I just made that prevents tether from doing that if the positional parent is the body. |
|
ah, sorry - didn't get that ;) |
|
No worries! Two different types of positioning, hard to differentiate them semantically, totally understand :) |
|
@pzuraq I maybe understanding this wrong but after looking through the code / trying it out it seems like all elements between the body and the tethered element must be statically positioned in the current implementation as opposed to exit as soon as you hit one statically positioned element. Is this correct? |
|
@varblob yep, that is correct. The Hubspot team didn't want to invalidate their rule, which is that the tethered element is positioned with respect to the body. Personally, having worked with this for a while, I still think that it would make sense to add the option to allow users to manually override this behavior. We're encountering a lot of edge cases regarding z-index and testing containers (which are positioned relatively) which are breaking our apps, and we've had to cobble together some ugly hacks to make everything play nice. @zackbloom I'd like you to reconsider adding this option. Users will still have to opt-out of tether's default behavior, and the majority of users will not. For those of us who have good reason to opt-out, we know (and accept) the potentially disastrous consequences. It would make life in the Ember world a lot easier (and I imagine the same goes for React and other frameworks). |
|
For the library I'm developing we create two roots, one for the application and one for tethered elements: <body>
<div class="tether-container">
<div class="animation-container">
<div class="tethered-element"></div>
</div>
</div>
<div class="ember-application">
...
</div>
</body>The tether container and animation container elements are web-components that manage the actual animation of tether, using the standard Ember animation library Liquid Fire. Note that we are guaranteed that positioning will be relative to the document, as the tether and animation containers are not offset from the document in any way. There are two major issues that have popped up because we can't put tethered elements inside of absolute, relative, or fixed position elements:
.tether-container * {
z-index: 9999;
}
Like I said we were able to get around both of these issues, and the current functionality works for the most part, but I believe these are just the first of many potential edge cases that will popup because we cannot manually control the DOM behavior of tethered elements |
|
This is what I currently use to workaround the difficulties of testing tether with ember: yapplabs/ember-tether#33 |
|
@martndemus That works for testing purposes, but for the z-index issue it doesn't do anything unfortunately. This is really one of the fundamental differences between |
Updated version of #84, with version bump to signal new feature.