Fix positioning error

[iansan5653]

Fixes a bug where the menu position would be calculated incorrectly because dom-input-range gives absolute exact positions and we were still offsetting by the position/scroll of the input.

This approach calculates where the menu is and where it isn't, then adjusts the top and left values based on the difference between the two. This relative approach is a little more complicated than just setting top and left directly but avoids have to make any special consideration for fixed/relative/absolute/top-layer positioning. It's also still very fast as all the computations involved are fairly cheap.

Also adds a floating menu to the demo so we can see this in action.

[keithamus]

Does this account for scroll containers? It's dropping the scrollx code which tells me it might not work properly if scrolled.

[iansan5653]

Sorry, didn't intend to mark this ready for review yet. I need to figure out a way to test this locally in dotcom.

Does this account for scroll containers? It's dropping the scrollx code which tells me it might not work properly if # scrolled.

Theoretically scrolling should work as expected: DomInputRange is already accounting for scroll. When getBoundingClientRect is called on the range, the result is exactly where we want the menu to be relative to the document, so it's just a matter of making sure that's where the menu ends up.

[iansan5653]

Okay, now we're ready for review. Tested in dotcom by manually copying in the changes and it's working great. I updated the approach to only have to set top and left once (fewer repaints!) and extracted a positionMenu function.

[keithamus]

👍 LGTM

[silverwind]

Also noticed that the positioning in @github/text-expander-element@2.7.0 is rather broken compared to 2.6.1, e.g. too far in both the x and y axis. I hope this fixes that.

[iansan5653]

Also noticed that the positioning in @github/text-expander-element@2.7.0 is rather broken compared to 2.6.1, e.g. too far in both the x and y axis. I hope this fixes that.

Yes, this should fix that! Will merge today and release as 2.7.1; please file a bug if you continue to see problems after that release

[silverwind]

Thanks, I can confirm 2.7.1 fixed that issue for me.