Commit dceac043 authored by Julian Descottes's avatar Julian Descottes
Browse files

Bug 1759332 - [devtools] Remove options.hideIfOffscreen for highlighters infobar elements r=Honza 

This feature is currently not tested.

Summary of the issue: we have up to 2 tooltips for grid elements: cell and area.
To avoid overlapping, today they are restricted with two options: `position` (top for one, bottom for the other) and `hideIfOffscreen`.
`position` already ensures that, when there is enough room, the tooltips will not overlap.
However, when they don't have enough room to be displayed, they will start "sliding" to remain in the viewport.
This means that under extreme conditions, you could again have an overlap between the two tooltips.

To avoid this, the `hideIfOffscreen` option was added. Instead of sliding, the tooltip are purely and simply hidden.
While I understand the idea, I think this creates more problems than it solves. For the sake of avoiding rare overlaps, we just don't display the tooltips in many situations.

Ideally we would have a better logic to place the 2 tooltips in compatible positions, or maybe we should even combine them.
But this is an easy way of fixing the issue here.

Differential Revision: https://phabricator.services.mozilla.com/D142203
parent ef456bd5

devtools/server/actors/highlighters/css-grid.js

+0 −2
Original line number Diff line number Diff line
@@ -1888,7 +1888,6 @@ class CssGridHighlighter extends AutoRefreshHighlighter { 
    const container = this.getElement("area-infobar-container");

    moveInfobar(container, bounds, this.win, {

      position: "bottom",

      hideIfOffscreen: true,

    });

  }
@@ -1919,7 +1918,6 @@ class CssGridHighlighter extends AutoRefreshHighlighter { 
    const container = this.getElement("cell-infobar-container");

    moveInfobar(container, bounds, this.win, {

      position: "top",

      hideIfOffscreen: true,

    });

  }

devtools/server/actors/highlighters/utils/markup.js

+1 −7
Original line number Diff line number Diff line
@@ -767,9 +767,6 @@ function waitForContentLoaded(iframeOrWindow) { 
 * @param  {String} options.position

 *         Force the infobar to be displayed either on "top" or "bottom". Any other value

 *         will be ingnored.

 * @param  {Boolean} options.hideIfOffscreen

 *         If set to `true`, hides the infobar if it's offscreen, instead of automatically

 *         reposition it.

 */

function moveInfobar(container, bounds, win, options = {}) {

  const zoom = getCurrentZoom(win);
@@ -842,10 +839,7 @@ function moveInfobar(container, bounds, win, options = {}) { 
    top -= pageYOffset;

  }



  if (isOverlapTheNode && options.hideIfOffscreen) {

    container.setAttribute("hidden", "true");

    return;

  } else if (isOverlapTheNode) {

  if (isOverlapTheNode) {

    left = Math.min(Math.max(leftBoundary, left - pageXOffset), rightBoundary);



    position = "fixed";