[hint addon] Offset the hint to position it correctly when hintOptions.container is given

[Nouzbe]

See #5854.

I tested it on the complete demo but did not commit changes to the demo file since I believe that having a container is not the most common usage. Let met know if you would prefer that this demo uses a container.

[marijnh]

.x and .y don't exist on IE and Edge, so you'll want to use .left and .top instead.

[marijnh]

Did you also test on a scrolled document? It seems that subtracting the top of document.body where nothing was subtracted before would break something.

[Nouzbe]

You are right, the previous solution did not work with IE/Edge, nor on a scrolled document (whether container was given or not).

Here is a new attempt where all of this should be fixed:

• Used .left and .top in place of .x and .y
• Didn't change the existing behavior when hintOptions.container is undefined.
• Used the position offset between the offsetParent and body in order to not inadvertently offset the scroll, as was previously the case.

[codeStryke]

Hi, I tried this out locally and feel like this line should be container.getBoundingClientRect(); to get the hint to show up at the exact same position as before. I maybe wrong, but it didn't work for me as is.

[marijnh]

I guess that depends on whether container is positioned or not—I guess we'd want the offsetParent of a child of the container.

[Nouzbe]

As you pointed out, the previous did not work well if the container is positioned.

Here is a new attempt, that should work in this case too.

[marijnh]

That should work better, but now you're introducing another layout cycle (changing the DOM first, then measuring it). I think using getComputedStyle to figure out whether the container is positioned would be a more efficient approach.

[Nouzbe]

Here's a new possible option. It works well too as far as I tested and does not change the points at which the script interacts with the DOM.

Also, I realized that other style updates (aiming at repositioning the hint if it overflows) reassigned left and top and we need to apply the same offset there too. My bad for not seeing it earlier. Better late than never though.

[marijnh]

Another nitpick: includes doesn't exist on all browsers that we support. Use indexOf instead, or a regexp.

Also, a space after if would make the code look more like the rest of the codebase.

[Nouzbe]

Sorry about all these roundtrips, and thanks for your patience @marijnh 🙏

Here's a new one, including your last remarks.

[marijnh]

Thanks for bearing with me!

[codeStryke]

@Nouzbe thanks for doing this. I was facing the same problem and now I can use this.
@marijnh thanks for the reviewing the changes. Is it released or any idea what release it will be in?