seiyria / bootstrap-slider

A slider control for Bootstrap 3 & 4.
http://seiyria.github.io/bootstrap-slider/
Other
3k stars 1.14k forks source link

preventOverflow flag to offset tooltips to stay within viewport #953

Open edemaine opened 3 years ago

edemaine commented 3 years ago

This PR fixes #649 and in some sense fixes #952.

I initially tried the second or third approach outlined in #952, but realized that it would be difficult to actually use Popper.js. createPopper requires a DOM element to anchor the tooltip to, and in some uses of bootstrap-slider, there isn't a tick associated with the position being tooltipped. We could create an artificial tooltip position DOM element to give to createPopper, but I instead tried a fourth approach:

  1. Implement the logic of preventOverflow manually.

This achieves the goal of no dependencies, adding a fairly small amount of code for nice functionality, hidden within a new preventOverflow option. I didn't change the defaults because it would break a lot of tests and backward compatibility, but happy to change.

I did not implement flip yet, mainly because I care about it less. I could work on it if desired.

I've gotten this working well in my project (via npm link), which uses left-to-right horizontal sliders and always-shown tooltips. Here are some screenshots

context screenshot
left extreme image
right extreme (including scrollbar) image
in the middle image

I should mention that the logic I implemented is much simpler than Popper's preventOverflow.js which seems to be mainly dealing with the case of clipping to the nearest display: fixed ancestor. I opted to just clip to window. There may be some subtleties I'm missing.

As this is my first contribution, I could use some guidance on how to test more thoroughly/broadly, and these PR requests:

seiyria commented 3 years ago

Thank you for this! It's the first contribution we've gotten in a while so please be patient. @rovolution - what do you think? You spent more time on this so I want to get your opinion as well if possible.

edemaine commented 3 years ago

Thanks for all the quick responses! Also, if you'd rather run with this yourself, tweaking it how you'd like/rather it be, feel free to do so (e.g. pushing to this branch).

rovolution commented 3 years ago

is there anyway to possibly add an automated test for this? i know sometimes it can be tricky with these types of changes

FezVrasta commented 3 years ago

Popper supports virtual reference elements, it would allow you to position a tooltip even if there's not a html element to attach to.