Compensate for body tag with position absolute, relative or static

[jasco]

In cases where the body tag has a position style of absolute, relative or static adjusts the positioning to compensate for position offsets and borders so powerTip remains properly placed.

Also opts for making all placement based on top rather than bottom attribute. The placement can be adjusted to use bottom if there is some reason to prefer it. The tests have also been adjusted.

This extra flexibility was needed because the control is used in pages where I do not control the html.

Comments or suggestions for improvement welcomed.

[stevenbenner]

This looks real good, and would be a huge improvement. I need to do a detailed code review. I do have a concern from the first-pass:

• Tips that were using bottom for CSS position now use top.

  The reason that they were using bottom was because it is more reliable. If we use bottom for the northern tooltips then if the content in the tooltip changes shape the tooltip will remain in the correct position. (See: #31 for this original disucssion)

Would it be possible to refactor this to keep using the bottom CSS rule on north-* tooltips?

[jasco]

@stevenbenner The positioning has been updated to use bottom for north placements. The branch has also been rebased to current tip of master.

[jonwolfe]

@stevenbenner thanks for the great library. We've incorporated PowerTip into a new project, and are experiencing this issue.

As we decide whether to implement a workaround or do a temporary monkeypatch of the current PowerTip release, it would be great to know if you plan on accepting this PR.

Thanks again.

[stevenbenner]

@jonwolfe Yes, I do plan on accepting this pull request. However it won't be in the next version (1.3.0). It's so close to release, and this change has more risk than I'm comfortable with for 1.3.0. Hopefully it will get in for version 1.3.1.

[jasco]

As an aside, perhaps higher risk commits fit better in a major revision release rather than a minor one?

In any event, it turns out that some webpages position the html tag rather then the body tag. I have a reworked patch that checks both body and html. Would you prefer that I submit a separate pull request or update this one?

[stevenbenner]

@jasco How big is the change? If it's a significant rewrite of this patch then go ahead and open a new pull request. Might be educational for me to see the changes. If it's just tweaking a couple lines then go ahead and update this pull request.

[jasco]

The technique is the same but the compensation calculation code was significantly restructured. In order to check body and html tags in turn the CSS position check and offset calculation were decomposed into functions. Upon further consideration I think it makes sense to update this patch since the updated implementation really does supersede this one because the checks are ordered. I will keep the commit history so if you can see both versions.

Since I am not sure the issue was clearly articulated before, I'll summarize. The issue is that absolute positioning is relative to the closest, non-static ancestor. Only when all ancestors are static is it relative to the document origin. The compensation adjusts the coordinates relative to document origin to the coordinates relative to the anchoring element.

Edit: fixed misuse of terms

[stevenbenner]

I've had a chance to test this out and do a code review, and I did find one bug.

Normal

bodyoffset-rel

Env: Firefox 51.0.1

Notice that there is a gap between the tooltip arrow and the element the tooltip opened for on the relative positioned test. I think this is because you're adding the $.postion() value to the returned offsets. I don't see why that is needed, is there a reason you're using elPositionPx?

[jasco]

Good catch, thank you. Not sure why I did not notice the offset. It seems that when calculated as I intended that the el*WithMargin and the elPositionPx terms all cancel out. The simplified calculation seems to fix the anomaly.

[jasco]

I added the position settings to edge-cases.

It works fine for body but not for the html tag. The jquery offset returns left: 0, top: 0 even when the html tag is positioned. I'm not sure the most robust way to find the position yet. getBoundingClientRect has some potential but I do not yet have the right computation for right and bottom offsets.

I'll push the tests. Maybe you have a suggestion?

[stevenbenner]

Thanks for the updates! The latest version fixed that bug, though I did discover another bug in testing.

Smart placement broken in relative positioning

It looks like the smart placement feature isn't working properly for tooltips that use bottom or right css positioning when the page is using relative positioning.

Repro steps:

1. Open tests-edge.html
2. Set the position to "BODY relative"
3. Scroll down to the "Huge Text with Smart Placement" test
4. Mouse over the "North" button

You'll see that even if there is plenty of room the tooltip will not show in the north position.

I believe this is because the collision detection from the getViewportCollisions() function is broken. This happens when the positionCompensation.top value is large. After reviewing the code I think that the positionCompensation adjustments are being applied in the wrong place (see my comment in the code review).

Side thought

After reviewing the code I've started to think that maybe this new logic belongs in the CSSCoordinates() object. It might be convenient to change the top, left, right, and bottom properties to be methods/getters that automatically apply the positionCompensation adjustments when something tries to read the property. That way you wouldn't have to apply the compensation to every place in code that touches the CSS positioning values.

This is just a thought I had, not a change request. So feel free to ignore.

[jasco]

(Rebased to current master. It appeared there was no conflict between this patch and the diff to master.)

Thanks for your review and feedback on this issue. I be out the rest of the week but I'll look at your comments when I get back.

[jasco]

The compensation calculations and collision detection have been completely revamped. I also took your suggestion to move the compensation computation into CSSCoordinates. The collision detection has been revised to use the same origin for all values include bottom and right. Also, the calculations now accounts for non-zero border widths on the non-static ancestor.

Also, the branch has been rebased to the head of master again.

I hope/think it is right now. I finally feel like I completely understand how measurements work in all of the cases I considered. It has been a rather painful process but I learned something along the way. I would note that, while I don't know of any issues, I have not reviewed for browser compatibility or behavior in legacy doctypes.

Thanks for your feedback so far.