Get scroll parent #65
Conversation
src/scroll-timeline-base.js
Outdated
|
|
||
| case 'absolute': | ||
| case 'fixed': | ||
| return findClosestAncestor(element, isAbsoluteOrFixedElementContainer); |
There was a problem hiding this comment.
This skips step 2:
If the position property is absolute, the containing block is formed by the edge of the padding box of the nearest ancestor element that has a position value other than static (fixed, absolute, relative, or sticky).
tldr; if position absolute, you need to consider the above condition or any fixed container condition.
src/scroll-timeline-base.js
Outdated
|
|
||
| default: | ||
| return getScrollParent(node.parentNode); | ||
| return getScrollParent(containingBlock); |
There was a problem hiding this comment.
While technically correct, I do prefer looping over recursion as it's slightly more efficient.
src/scroll-timeline-base.js
Outdated
| return getScrollParent(containingBlock); | ||
| while (true) { | ||
| const containingBlock = getContainingBlock(node); | ||
| if (!containingBlock) |
There was a problem hiding this comment.
nit: I think it's slightly cleaner to move this to the while condition, e.g.:
while (node = getContainingBlock(node)) {
const style = getComputedStyle(node);
switch(style['overflow-x']) {
case 'auto':
case 'scroll':
case 'hidden':
return node;
}
}
return document.scrollingElement;There was a problem hiding this comment.
done. Also added missing special case rule for the body element.
src/scroll-timeline-base.js
Outdated
| // of the first visible child body is applied instead. | ||
| if (node.tagName != "BODY" || | ||
| getComputedStyle(document.scrollingElement).overflow != "visible") | ||
| return node; |
There was a problem hiding this comment.
nit: I think it would be clearer if you flipped the condition and used this case for the exception (the body element in these special circumstances) to return document.scrollingElement and then left the return below as returning the node.
src/scroll-timeline-base.js
Outdated
| // The UA must apply the overflow from the root element to the viewport; | ||
| // however, if the overflow is visible in both axis, then the overflow | ||
| // of the first visible child body is applied instead. | ||
| if (node.tagName != "BODY" || |
There was a problem hiding this comment.
As we talked about in person, wouldn't checking node == document.body correctly only apply this behavior to the first child?
Fix calculation the nearest ancestor scroll container.