Page MenuHomePhabricator

Tooltips do not appear in when editor is in fullscreen mode
Closed, ResolvedPublic

Description

Title says it all.

Or could this be the intended behavior?

Event Timeline

Yomi raised the priority of this task from to Needs Triage.
Yomi updated the task description. (Show Details)
Yomi added a project: Maniphest.
Yomi added a subscriber: Yomi.
epriestley triaged this task as Wishlist priority.Apr 1 2014, 7:13 PM
epriestley added a subscriber: epriestley.

This is not intended, but kind of a pain to fix, and argubably not having tooltips is less distracting?

The easiest fix for this is probably for tips to check if they'll display off screen in their intended direction, and to flip directions if they will (i.e., "N" -> "S", or "E" -> "W"). I think that will never give us worse results, and won't require goofy special-casing around these tips.

First, find a comment box. An easy way to find one is to go to Maniphest, then Create New Task, then create a test task, then scroll to the bottom of the page.

In the comment box, if you hover over the "B", "I", "T", etc., icons above a comment box, you get tooltip popups saying "Bold", "Italic", etc. These appear above (vs beside/below) the icons, since they're internally specified to have "N" (north) alignment.

Screen_Shot_2014-04-08_at_8.17.21_PM.png (50×281 px, 6 KB)

However, if you click the double-arrow icon on the right-hand side to expand the comment area to go to "Fullscreen Mode", the tooltips are no longer visible because they're off the top of the screen. This is the button in question:

Screen_Shot_2014-04-08_at_8.18.23_PM.png (66×105 px, 3 KB)

To fix this in the general case, I think we should do this:

  • Before displaying a tooltip, check if it will be off screen when displayed.
  • If it will be, flip its alignment (so "N" (north) becomes "S" (south)), etc.

The goal is for the tips to appear in full screen mode, but show up below the buttons instead of above them.

(We could do other things, like special case the alignment for the tips in this case, but I think this general rule will improve all current/future cases of this problem and can't come up with any cases it makes worse.)


The code for rendering tooltips is in phabricator/webroot/rsrc/js/core/ToolTip.js, and the part that positions the tooltips is near line 50:

...
// Append the tip to the document, but offscreen, so we can measure it.
node.style.left = '-10000px';
document.body.appendChild(node);

var p = JX.$V(root);
var d = JX.Vector.getDim(root);
var n = JX.Vector.getDim(node);

// Move the tip so it's nicely aligned.

switch (align) {
  case 'N':
    node.style.left = parseInt(p.x - ((n.x - d.x) / 2), 10) + 'px';
...

I think you should be able to put some code in the middle (near the // Move the tip.. comment). For context:

  • root is the DOM node which we're showing a tip for (like the "B" or "I" button).
  • node is the tip itself (the black box with text in it).
  • p, d and n are JX.Vectors, which have a .x and .y property:
    • p is the position of root. For example, p.x has its horizontal position in pixels.
    • d is the size of root (the thing we're showing a tip for).
    • n is the size of node (the tip).
  • align is the layout direction, which you ultimately want to make a decision about flipping (e.g., from "N" to "S").

You'll also probably need to make a call to JX.Vector.getViewport(). That will give you a vector with x and y that correspond to the scroll position in the document. This will help you figure out if the tip will be offscreen or not.

To get started, if you just add some code like p.x += 50 and reload the page, that should affect the tooltips.


Once you have a patch, run this script before sending a diff:

phabricator/ $ ./bin/celerity map

You should run that script before sending code for review if your patch includes static resource changes (CSS, JS, or images). You can find more information here:

This might be vaguely useful:

Small correction -- JX.Vector.getViewport() may be useful, but JX.Vector.getScroll() is the method I was actually talking about. The "Viewport" is the width and height of the browser window, while the "Scroll" is the scroll position within the document.

On the z-index thing: I can't find any record of explicitly having tried to hide the tips in fullscreen mode or other nonsense like that -- let's just move the z-index for tips to 51.