1396673 - Proper stacking of positive and negative numbers

Status: NEW

(DevTools :: Inspector, defect, P3)

Description

[Micah Tigley [:micah]]

Attachment: negative-stack-issue.png

Stacking negative line numbers looks off in some cases. Please see attachment.

Comment 1

[Micah Tigley [:micah]]

Attachment: Bug 1396673 - Negative line numbers are stacking incorrectly.

Review commit: https://reviewboard.mozilla.org/r/178070/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/178070/

Comment 2

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 8906338 [details]
Bug 1396673 - Negative line numbers are stacking incorrectly.

https://reviewboard.mozilla.org/r/178070/#review186992

::: devtools/server/actors/highlighters/css-grid.js:1250
(Diff revision 1)
>          const { breadth }  = gridLine;
>  
>          if (breadth === 0) {
>            stackedLines.push(negativeLineNumber);
>  
> -          if (stackedLines.length > 0) {
> +          if (stackedLines.length > 0 && stackedIndex === 0) {

I notice now that this approach basically considers just one line per dimension where the number are stacked, but I believe that this is not necessary the case.

See for example this grid, provided by Jen:

https://s.codepen.io/jensimmons/debug/pWvNXa

(test also with different zoom level and viewport size).
There are empty boxes, and also misplaced (because they're considered the "back" of the stack).

That of course, is applying on both positive and negative numbers. We should probably rename this bug something like "stack properly positive and negative numbers", and it should have an higher priority too.

::: devtools/server/actors/highlighters/css-grid.js:1648
(Diff revision 1)
>  
>      [x, y] = apply(this.currentMatrix, [x, y]);
>  
>      if (stackedLineIndex) {
> -      // Offset the stacked line number by half of the box's width/height
> -      const xOffset = boxWidth / 4;
> +      // Offset the stacked line number by a quarter of the box's width/height
> +      const boxOffset = boxHeight / 4;

You should keep both `boxWidth` and `boxHeight`, and therefore `xOffset` and `yOffset`, since one of them could be different from the other. They're equals *only* if the content is not bigger than the minium size.

Comment 3

[Micah Tigley [:micah]]

Attachment: Screenshot from 2017-09-24 20-46-50.png

Drawing the number for every stacked line reveals some issues with how negative line numbers are displayed. Notice how "the lowest number of the stack" is brought to the front, however in Jen's example ends up incorrectly showing the sequence of negative line numbers instead.