-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use JS lib handle toolbar positioning #36
Comments
Two thoughts so far:
Maybe worth looking into some libs like tether, or just tether itself, and if we can't use them directly, copy out chunks of logic. |
Found popper.js which might be the way to go. Apparently it's a "positioning engine", but sounds like it doesn't make many assumptions and can be configured nicely. Only thing to check would be if it supports (or could support via plugin) using a Range as a reference, rather than an element. |
The "range as reference" is a feature request that already has a ticket open for it. |
Great! Thanks @FezVrasta - I'll comment on that issue and see if I can help out at all |
As per my comment on that issue, looks like all you'd have to do is have e.g. something like: function makeReferenceFromRange(range) {
let getRect = () => range.getBoundingClientRect();
return {
getBoundingClientRect: getRect,
get clientWidth: () => getRect().width,
get clientHeight: () => getRect().height
};
} Though not sure if for a range the |
Forked the codepen in that comment and put together a rough example of what it would look like for our use case. Looks promising! |
👍 Let's do a spike on this for simpla-text, and if it works we can generalise to a behavior for reuse throughout simpla, going to be a reasonably common pattern |
Or, of course, the other option is to bail on scroll and resize. That would be equal robust and possibly much more performant, but worse UX. For me would depend entirely on perf - don't like the idea of handling scroll events with js, no matter how you spin it that's a heavy weight for a minor piece of functionality. That pen looks okay, but only tested on my 2015 MacBook without throttling. |
I tested Popper extensively to make sure it provides great performance. Your fiddle with a CPU throttling down to 20x works smoothly. |
Yep, I did another few tests after making that comment and it's pretty impressive, I generally associate scroll interaction of any sort with woeful perf (for the outcome), but can't argue with real world results. @bedeoverend I'm happy to go ahead with testing this in real elements |
So I did a quick spike of this locally, but it seems popper assumes a reference elements are fixed...? Not sure, but have raised an issue on popper. See this pen for minimum repro of the behaviour. A workaround is requesting an update from popper in a |
Sure sounds good. That's implementation details anyway. |
There's a whole lot of edge cases that the simple abspos CSS positioning we're currently using for
simpla-text-toolbar
would fail - eg: if the range is in a scollable container (but the toolbar is relative to the body/document).Two solutions to this:
Exit the toolbar on scroll, similar to how
simpla-img
container worksUse JS to position (and update the position of) the toolbar, maybe taking advantage of a library like Tether.js (though don't think tether supports ranges out of the box)
Whatever we decide, should be consistent with
simpla-img-editor
as well (ie: either bail on any sort of pos change, or handle positioning in a more robust, but less performant manner).The text was updated successfully, but these errors were encountered: