Reject parent with overflow set to visible #41
Conversation
| el.clientWidth < el.scrollWidth; | ||
| hasVisibleOverflow = | ||
| w.getComputedStyle(el, null).overflow === 'visible'; | ||
| } while (!isBody && !(hasScrollableSpace && !hasVisibleOverflow)); |
There was a problem hiding this comment.
Bare with me and this not so clear statement. Script will keep looking for a parent to scroll unless the element is the body element or has space to scroll but overflow is not set to visible. I hope that simplifies the reading.
| || el.clientWidth < el.scrollWidth)); | ||
|
|
||
| // set condition variables | ||
| isBody = el === d.body; |
There was a problem hiding this comment.
could we bail out early if any of these conditions are true? The reason is because the clientHeight/scrollHeight properties force layout.
There was a problem hiding this comment.
We can bail if the closest parent of the element is the body. That said I don't know in how many cases this will actually happen so I'm not seeing a real benefit but if you think is worth it I'll add at top @iamdustan
|
This looks legit to me, @jeremenichelli. Do we want to land this along with any other open PRs and cut another release? |
|
@iamdustan this is en edge case so I prefer to wait a few days and see what happens with #42 so we can ship both. Plus, if I have time, I might do the periodically revision I do and see if there's room for improvements in code or build steps. |
@iamdustan this fixes issue #36 review when you have time to see if I am missing something.
Description: in cases where the overflow style property is set to visible and children are absolute positioned the script needs to scroll the body instead of the scrollable parent.
@KitaitiMakoto with this fix, the example you provided in the first comment of the issue is mitigated; after merged test it on your project locally to see if it works there too.