Skip to content
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

F# tooltips position is off #71

Open
vlaci opened this issue Jun 3, 2015 · 10 comments
Open

F# tooltips position is off #71

vlaci opened this issue Jun 3, 2015 · 10 comments

Comments

@vlaci
Copy link

vlaci commented Jun 3, 2015

On my notebook at 1366x768 resolution tooltips of all example presentation are displayed at the bootom edge of the screen and sometimes even off the screen. The problem is present in both IE11 and FF38 at normal zoom level.
I fixed the issue for mysef by replacing the following call of getPos:

    var pos = findPos(owner ? owner : (evt.srcElement ? evt.srcElement : evt.target));
    var posx = pos[0];
    var posy = pos[1];

To:

    var posx = evt.clientX;
    var posy = evt.clientY;

I don't know how the findPos code should be better than just getting the coordinates directly from the event args.

@Andrea
Copy link

Andrea commented Nov 28, 2015

Having the same issue for a while now too , having this issue with chrome

@isaacabraham
Copy link

This is an issue for me in Firefox but seems to work ok in Chrome.

@forki
Copy link
Member

forki commented Nov 29, 2015

If someone can do a pull request that would be awesome

@rmunn
Copy link

rmunn commented Jun 1, 2016

I plan to look into this when I have time, sometime in the next couple of weeks. If someone else beats me to it, though, it looks like getBoundingClientRect() might be the right answer, possibly with window.scrollX and window.scrollY added. See https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect for more details.

@rmunn
Copy link

rmunn commented Jun 3, 2016

I've looked at this a little, and the current code is doing exactly what getBoundingClientRect() would do: the tooltips are being set to a position that should be immediately below the "parent" element. However, reveal.js is doing something funny with zoom levels and scaling that's causing the abolutely-positioned elements to not be placed where they normally "should" be placed based on their coordinates. So the cause of this bug is an unintended interaction between the reveal.js code and the tooltip placement.

I haven't yet found the details of the interaction, so I don't yet have a solution to propose.

@rmunn
Copy link

rmunn commented Jun 4, 2016

The reason reveal.js is throwing things off appears to be that it's using 3D transforms to style its slide transitions (at least, if you use the most common transitions) and to scale the content to fit the browser window. Try it: load an FsReveal slide and try using the Ctrl-plus and Ctrl-minus shortcuts to zoom the browser window. You'll find that the slide text stays pretty much the same size, because reveal.js is using 3D transforms to scale to whatever size your viewport is.

So why does that matter? Well, it appears that getBoundingClientRect() and other similar methods (like the calculations that findPos() is currently doing) return positions that are calculated after the 3D transform are applied -- in other words, its actual position on the screen. But when you assign an element's top and left positions in pixels, those values are taken as representing its position before the current 3D transform is applied.

Both of these are normally what you would want, but here they're working against what we're trying to do. I found a detailed discussion of this phenomenon here:

https://www.bountysource.com/issues/1510871-initializing-inside-a-transformed-css-block-invalidates-codemirror-s-positioning-assumptions

This is a discussion between several CodeMirror devs who are trying to figure out how to use CodeMirror inside a CSS-transformed element. (CodeMirror does a lot of positioning various elements relative to each other.) The fifth comment in that discussion looks particularly useful: Nathan Hammond discusses using a "canary" element (as in "canary in the coal mine") to calculate what the transform matrix must be, by setting its position attributes to various values and observing what results. His idea appears to be that they could then apply that transform matrix (or its inverse) to switch between what I'll call "original" coordinates and "transformed" coordinates.

In that discussion thread, they eventually give up on that idea because the performance hit of constantly recalculating the positions of elements from a transform matrix would be too great. However, in the FsReveal context, we can afford to take a greater performance hit since we don't need to feel instantly responsive to keystrokes -- and tooltip-position calculations only happen when the user mouses over the code anyway, which isn't very often. So we could potentially rewrite the findPos() function to calculate the transform matrix that reveal.js is using, and apply that transform matrix in reverse to the values we get from getBoundingClientRect() in order to figure out what values to assign to the tooltip's left and top attributes.

If you're reading this and that made no sense at all to you, don't worry about it -- I'm mostly writing this down as notes to myself so that I can remember what I was thinking when I come back to this later. (I'm running out of spare time to work on it today).

But if you're reading this and this comment made perfect sense to you, then you probably understand CSS and 3D transforms better than I do (I'm a complete novice at 3D math), so _please_ feel free to tackle the problem yourself -- because your solution will probably be better than what I come up with.

@rmunn
Copy link

rmunn commented Jun 4, 2016

Actually, I believe I've found an answer that should work for every zoom level and screen size -- though it needs testing. Replace your tips.js with the following:

var currentTip = null;
var currentTipElement = null;

function hideTip(evt, name, unique) {
    var el = document.getElementById(name);
    el.style.display = "none";
    currentTip = null;
}

function hideUsingEsc(e) {
    if (!e) { e = event; }
    hideTip(e, currentTipElement, currentTip);
}

function showTip(evt, name, unique, owner) {
    document.onkeydown = hideUsingEsc;
    if (currentTip == unique) return;
    currentTip = unique;
    currentTipElement = name;

    var target = owner ? owner : (evt.srcElement ? evt.srcElement : evt.target);
    var posx = target.offsetLeft;
    var posy = target.offsetTop + target.offsetHeight;

    var el = document.getElementById(name);
    var parent = target.offsetParent;
    el.style.position = "absolute";
    el.style.left = posx + "px";
    el.style.top = posy + "px";
    parent.appendChild(el);
    el.style.display = "block";
}

Notice how findPos() is gone. Instead, we can just calculate the X,Y offset of the span whose tooltip we're displaying. Then we find the .offsetParent of the span, which is the element relative to which the offset is calculated. Finally, we make the tooltip a child of that element so that we're calculating the tooltip's position in the exact same way as the span element's position is calculated.

Result: so far I've seen the tooltip show up precisely under its corresponding span, each time, every time.

HOWEVER: I still need to test one scenario. Looking at the CSS for reveal.js, it seems that if you select no transitions, 3D transforms are not used. When I tested the above Javascript against a "normal" F# page formatted with FSharp.Formatting, the parent.appendChild(el) line did not have the desired effect. When 3D transforms are not happening, position: absolute positions things relative to the document just like you would expect. So if someone has selected NO transforms for their presentation, I believe the code above would fail.

I will test this further, and then submit a proper PR.

In the meantime, anyone affected by this bug has a workaround: after you run build.sh, replace the tips.js file in your output/css/fsharp.formatting/styles folder with the code above and the tooltips should be properly positioned. Note that this will be overwritten the next time a build runs, so if you're still editing your presentation you'll have to apply this workaround multiple times.

If that's too much hassle for you, then just wait for the pull request. Once I fix this properly, it should Just Work™ without needing to edit files that build.sh is going to overwrite. PR should hopefully be coming in a couple of days.

rmunn added a commit to rmunn/FsReveal that referenced this issue Jun 4, 2016
The reveal.js animated slide transitions are done with 3D transforms,
which throws off the element-positioning logic from FSharp.Formatting
since it assumes a "normal" Web page. Under reveal.js, tooltips should
be positioned as a sibling of the element they belong with. Then their
positioning will be correct no matter what 3D transform is in effect.

Fixes fsprojects#71.
@rmunn
Copy link

rmunn commented Jun 4, 2016

I've tested this, and it works properly with NO transitions as well. So here's my PR.

@rmunn
Copy link

rmunn commented Jul 27, 2016

This should be fixed now that FsReveal 1.3.1 is out. If @vlaci can confirm that it works, this issue can be closed.

You can tell if you have FsReveal 1.3.1 by looking at your paket.lock file in the root of your repo. If you see FsReveal 1.3 or earlier, run paket update to pull in the latest version of FsReveal, after which point your tooltip positions should now be correct.

@pirrmann
Copy link

pirrmann commented Mar 5, 2017

I've also had this issue for a while and updating to 1.3.1 fixed it, I think this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants