1424880 - CSS Grid Inspector area names are sometimes HUGE and obscure the web page.

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Morgan]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.84 Safari/537.36

Steps to reproduce:

Create a grid using CSS grid, use `grid-template-areas`, the first and last columns should be small (e.g. 5px or 0). Add some HTML to be laid out—then use the grid inspector and display area names. 

I have created a reduced test-case as an example to demonstrate: https://codepen.io/morganfeeney/pen/ZarpPZ?editors=1100

There are more examples here: https://github.com/mozilla/css-grid-inspector/issues/31


Actual results:

The area names are far too big and obscure the web page, making it hard to work with as a design aid as... well the design is completely obscured in some cases.


Expected results:

I expect to see area names at the same size I would normally get using the Grid Inspector.

Comment 1

[Patrick Brosset <:pbro>]

Thanks for filing Morgan. It's true that the area names can sometimes be pretty big.
I think this will need more design work at this stage. Marking this as an enhancement.

Comment 2

[Patrick Brosset <:pbro>]

I found another pretty crazy example here: https://codepen.io/ire/pen/Legmwo

Comment 3

[Patrick Brosset <:pbro>]

I think one of the issues occurs with very wide but short areas. For instance, think of an area that's 1000px wide but only 10px high.
By default, we use a font-size of 30.
However, we later check the size of the column and row, and if the font-size is bigger than any of them, we then take the average of these 2 values.
So, 30 is larger than 10 (the row height), so we do 1000 + 10 / 2 = 505, which is a really big fontSize!

This is the code: https://searchfox.org/mozilla-central/rev/062e1cf551f5bf3f0af33671b818f75a55ac497b/devtools/server/actors/highlighters/css-grid.js#845-849

Instead, we should only size the font relative to the smaller of the 2 dimensions.

Comment 4

[Morgan]

Yes, it's a real shame as when you get the font at that size you can no longer use the named areas as part of your workflow.

Comment 5

[Patrick Brosset <:pbro>]

Attachment: Bug 1424880 - Size the font for grid area names after the smallest dimension;

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

Comment 6

[Patrick Brosset <:pbro>]

Here's an attempt at fixing this.
It makes the examples from comment 0 and comment 2 work a lot better, while still preserving the fix from bug 1379715.
And the code change is minimal. So, even if the code isn't a completely dynamic system that will adapt to anything, I think it's still will work with probably more than 95% of all cases.

Comment 7

[Patrick Brosset <:pbro>]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8aa08fdbcf0246f4b8b5ab17496b8706db97370a

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8946167 [details]
Bug 1424880 - Size the font for grid area names after the smallest dimension;

https://reviewboard.mozilla.org/r/216164/#review221916

::: devtools/server/actors/highlighters/css-grid.js:847
(Diff revision 1)
>          let column = fragment.cols.tracks[columnNumber - 1];
>  
>          // Check if the font size is exceeds the bounds of the containing grid cell.
>          if (fontSize > (column.breadth * displayPixelRatio) ||
>              fontSize > (row.breadth * displayPixelRatio)) {
> -          fontSize = (column.breadth + row.breadth) / 2;
> +          fontSize = Math.min([column.breadth, row.breadth]);

Add a comment here like:
Size the font for the grid area names after the smallest dimensions if the font size exceeds the bounds of the containing grid cell.

Comment 9

[Patrick Brosset <:pbro>]

Comment on attachment 8946167 [details]
Bug 1424880 - Size the font for grid area names after the smallest dimension;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/216164/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b3a5cc80128
Size the font for grid area names after the smallest dimension; r=gl

Comment 11

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8946167 [details]
Bug 1424880 - Size the font for grid area names after the smallest dimension;

https://reviewboard.mozilla.org/r/216164/#review221918

::: devtools/server/actors/highlighters/css-grid.js:844
(Diff revisions 1 - 2)
>      for (let rowNumber = rowStart; rowNumber < rowEnd; rowNumber++) {
>        for (let columnNumber = columnStart; columnNumber < columnEnd; columnNumber++) {
>          let row = fragment.rows.tracks[rowNumber - 1];
>          let column = fragment.cols.tracks[columnNumber - 1];
>  
> -        // Check if the font size is exceeds the bounds of the containing grid cell.
> +        // If the font size exceeds the bounds of the containing grid cell, size it its

Typo here - s/size it its/size its

::: devtools/server/actors/highlighters/css-grid.js:845
(Diff revisions 1 - 2)
>        for (let columnNumber = columnStart; columnNumber < columnEnd; columnNumber++) {
>          let row = fragment.rows.tracks[rowNumber - 1];
>          let column = fragment.cols.tracks[columnNumber - 1];
>  
> -        // Check if the font size is exceeds the bounds of the containing grid cell.
> +        // If the font size exceeds the bounds of the containing grid cell, size it its
> +        // row or column dimension, whichever is smallest.

s/dimension, /dimension to whichever is smallest.

Comment 12

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/2b3a5cc80128

Comment 13

[Nazir Ahmed Sabbir [:NaSb]]

Reproduced this bug on Nightly 59.0a1 (2017-12-12) (64-bit) (Build ID: 20171212100127) in Linux,64 bit

This Bug is now verified as fixed on Latest Developer Edition 60.0b14 (64-bit) 

Build ID: 20180419200216
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0