Use popper.js for toolbar positioning logic

[bedeoverend]

@seaneking could you take a look and see if this works for you?

[bedeoverend]

Only bit of weirdness is it doesn't position itself quite right when the text is at the edge of the screen:

compared with before:

Most likely an issue with popper.js

Given this, perhaps we should just leave this PR and do it properly when we develop Anchor? Then can just pillage the logic from this branch.

[madeleineostoja]

Oh yeah I'd say just leave this for a shareable behavior (Anchor), but if you want to dogfood this as an MVP until then, then sure happy to try it out. Does it have the viewport bounds behavior and all that?

[bedeoverend]

Keen to dogfood given it'll be essentially the same logic once Anchor is a thing. And yeah keeping it in the viewport is just a part of popper.

So turns out the visual bug was because that env had body { position: relative; } but still the default margin of 8px so the toolbar was off by 8px. @seaneking think that's a reasonable enough caveat? i.e. that the toolbar will position itself relative to the body if the body has position: relative;

[madeleineostoja]

Not really, it shouldn't matter what position the body has - that's part of the problem we have now. That makes me wonder, if it's positioning itself relative to the body does that mean things like scrolling containers still break? In which case we'd in the exact same position as we are now.

[bedeoverend]

I don't think so. Pretty sure scrolling containers are fine, but I need to investigate this some more and flag it in the popper.js repo - it's just that we have a very specific scenario, I've been struggling to nail a proper minimum repro, and I've already sunk too much time trying to figure out what's going on.

I might put this on hold for now, and we can nail it down when building Anchor / simpla-article.

[madeleineostoja]

Since we're now dogfooding this on simpla-article, and I'm assuming a lot of the logic in this is out of date, I'd prefer to just make a shareable behavior we can iterate on between both article and text. Because the pos logic on article is already pretty buggy cross-browser and needs fixes which I'm sure text will need as well.

[FezVrasta]

You may find something useful in the preventOverflow modifier documentation:
https://popper.js.org/popper-documentation.html#modifiers..preventOverflow.boundariesElement

If you find out being an issue of Popper.js please open an issue on its repository so that I can fix it.

[bedeoverend]

@FezVrasta yeah tried that, don't think it quite addresses the issues we've been seeing. Will definitely make an issue once I have a consistent minimum repro, thanks!