Closed Bug 1424880 Opened 6 years ago Closed 6 years ago

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

Categories

(DevTools :: Inspector, defect, P2)

defect

Tracking

(firefox60 fixed)

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: info, Assigned: pbro)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Component: Layout → Developer Tools
Product: Core → Firefox
Version: 59 Branch → unspecified
Component: Developer Tools → Developer Tools: Inspector
Summary: Area names are sometimes HUGE and obscure the web page. → CSS Grid Inspector area names are sometimes HUGE and obscure the web page.
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.
Blocks: dt-grid
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
I found another pretty crazy example here: https://codepen.io/ire/pen/Legmwo
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.
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.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
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.
Severity: enhancement → normal
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.
Attachment #8946167 - Flags: review?(gl) → review+
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2b3a5cc80128
Size the font for grid area names after the smallest dimension; r=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.
https://hg.mozilla.org/mozilla-central/rev/2b3a5cc80128
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
QA Whiteboard: [good first verify]
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
QA Whiteboard: [good first verify] → [good first verify] [testday-20180420]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.