MaxArt2501 / share-this

Medium-like text selection sharing without dependencies
https://maxart2501.github.io/share-this/
MIT License
808 stars 53 forks source link

Popover goes over scrollable content #26

Closed RigaNik closed 6 years ago

RigaNik commented 6 years ago

Hello Max,

Reposition method works as expected, it follows the text as it scrolls, but there is still one small problem here. If you scroll up while a text is selected and the popup is active, you will notice that popup goes over the "parent"(div that holds the text) and doesn't follow the text under it.

I made small fiddle example of the problem and you can check it at the following link: https://jsfiddle.net/Nikola22/oL5ha3tw/

MaxArt2501 commented 6 years ago

The provided fiddle doesn't work, but I think I know what you mean. The problem is that detecting if a selection is actually visible or not is a task too complex for the purpose of the library, so it's a task left to the developer. In fact, there are many ways a portion of the DOM can become invisible, and not just by scrolling.

Scrolling is probably the most common case, and could be resolved using IntersectionObservers. But it's not an API already available everywhere, so that would mean it would imply a polyfill. And even if it was available in all major browsers, it wouldn't help in more complex use cases (e.g. nested scrollable containers).

My suggestion is to keep a reference to the popover's element defining a onOpen callback, then toggling it visibility after performing a check during the scroll event.