Grid inspector line numbers disappear when changing grid-row-end value

RESOLVED FIXED in Firefox 61

Status

defect
P2
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: nchevobbe, Assigned: ewright)

Tracking

(Blocks 1 bug)

Trunk
Firefox 61
Dependency tree / graph

Firefox Tracking Flags

(firefox60 wontfix, firefox61 fixed)

Details

Attachments

(3 attachments)

**Steps to reproduce**

  - Go to https://inspector-grid-empty-line-number.glitch.me/
  - Open the inspector
  - Select the "main" node
  - Turn on the grid inspector
  - Make sure you have the "display line numbers" checked
  - Select the "b" node in "main"
  - Focus the "-1" in the grid-row value
  - Hit ↓ to decrement it multiple times

**Expected results**

The lines update

**Actual results**

The line numbers disappear in the grid inspector, and eventually, at "-4", the whole grid inspector disappears
Blocks: dt-grid
Component: Developer Tools: Layout Frame Inspector → Developer Tools: Inspector
Assignee: nobody → ewright
To test: 
- go to https://gridbyexample.com/examples/code/example21.html (or any other example)
- Turn on the grid inspector
- Make sure you have the "display line numbers" checked
- select an element in the grid
- change the values of that element's 'grid-column' and 'grid-row' attributes to various combinations.
- observe the number flags.

with this patch:
- the highest number in the stack is always visible
- the whole grid inspector does not disappear


Should flag "1" always be visible in the positive numbers? I've attached some results from this patch and circled two cases where it is behaving as expected, but it looks a little odd.
Priority: -- → P2
Attachment #8959579 - Flags: review?(gl) → review?(pbrosset)
I see the bug described in comment 0 fixed now with the patch, which is great. Also the code changes look good to me at first sight.
But I want to discuss the point raised by Erica in comment 2.

The stacking of line numbers was added in bug 1380544. My understanding is that back then, we made it work fine for cases where there is no gap between tracks. Indeed, when lines really do stack because they have a breadth of 0, and there is no gap between them, then stacking the line number bubbles looks great. It gives a little visual cue that there are more than 1 line here.

Now, when there *is* a gap, I don't think the stacking works anymore. And as Erica mentioned, it's odd not seeing all the numbers, especially number 1.
In fact, if you push it to an extreme use case and have a gap that's larger than, say, 30px, then all you see is empty-looking bubbles, which is weird. It feels like a bug. Also because the empty bubbles are positioned with a slight offset.

My feeling is that as soon as the gap is more than 0 (or maybe a couple px), then we should draw all bubbles with numbers as usual, and skip the stacking behavior.

In the meantime, I'll R+ the patch because the code looks good and it does fix comment 0, but I'd like us to work on this. So, either in the same bug, or by filing another one.
Comment on attachment 8959579 [details]
Bug 1439512 - Correctly find stacked lines to ensure grid and numbers do no disappear when changing grid-row values.

https://reviewboard.mozilla.org/r/228370/#review236972
Attachment #8959579 - Flags: review?(pbrosset) → review+
Posted image large-gap.png
Here's what happens with a large grid gap.
Erica, do you mind filing another bug to capture this? Also, do you agree with my solution in my last comment?
Flags: needinfo?(ewright)
pbro: Yes, I think your suggestion is good. Though, perhaps the flags could stack even if the grid-gap is defined as a few pixels - if the amount is < the width of the flag? Since in that case the flags overlapping would make sense. Though, in that case, perhaps the number remains printed even though the flags are stacked? Since you'd see at least a portion of the number. And yes, I can open a bug for this.
Flags: needinfo?(ewright)
(In reply to Erica Wright [:ewright] from comment #7)
> pbro: Yes, I think your suggestion is good. Though, perhaps the flags could
> stack even if the grid-gap is defined as a few pixels - if the amount is <
> the width of the flag? Since in that case the flags overlapping would make
> sense. Though, in that case, perhaps the number remains printed even though
> the flags are stacked? Since you'd see at least a portion of the number.
Great, that's exactly what we were just discussing with Jen on a call just now. That makes sense to me.
> And yes, I can open a bug for this.
Awesome, thanks.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/945da7d0b6cb
Correctly find stacked lines to ensure grid and numbers do no disappear when changing grid-row values. r=pbro
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/945da7d0b6cb
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.