Link preview: do not add elements inside headers #592
Conversation
There is no need to add the tooltip next to the anchor element where the tooltip is going to be displayed. The position is pre-calculated and it does not interfere where in the DOM is added. To simplify, I'm adding the tooltips always before closing the body of the page.
| @@ -189,7 +189,7 @@ function setupTooltip(el, doctoolname, doctoolversion, selector) { | |||
| ); | |||
| newTooltip.setAttribute(TOOLTIP_DATA_HREF, anchorElement.href); | |||
| newTooltip.classList.add("tooltip"); | |||
| anchorElement.insertAdjacentElement("afterend", newTooltip); | |||
| document.body.insertAdjacentElement("beforeend", newTooltip); | |||
There was a problem hiding this comment.
I am wondering what effect this might have. I assumed looking at this code that the element was inserted adjacently for positioning purposes. For example, if the tooltip is positioned relatively, measuring from the document body would be different than the measurement from a sibling element to the tooltip trigger element.
This might be noticeable if the link was inside an element that had a large margin-left for example, as the document body would not have this.
But as far as I can tell this doesn't look like it should be the case?
There was a problem hiding this comment.
Positioning or similar shouldn't be an issue because it's based on the original anchor element: https://github.com/readthedocs/addons/blob/humitos%2Flp-headers/src/linkpreviews.js#L125-L155
As far as I can tell, all the tooltip previews I've tested are well positioned 👍🏼
Closes #584