1501997 - [markup-view] Set line-height to 14px for more predictable vertical rhythm and alignment

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Florens Verschelde :fvsch]

Currently the lines in the markup view have use a line-height defined like this:

.tag-line {
  min-height: 1.4em;
  line-height: 1.4em;
}

Since the font-size is fixed to 11px, this results in a computed line-height and min-height of 15.4px.

In my experience, fractional line-heights can result in imprecise rendering on 1x displays, due to pixel-fitting. A line can be visually rendered at 15px or 16px, the distance between two text baselines can be alternatively 15px or 16px, etc.

In https://github.com/devtools-html/ux/issues/3 I described how we could strive to match the Photon grid, which uses 4px steps. So a content line could be 16px or 20px (depending on the need), toolbars and headers 24px or 28px, etc.

Since we're already close to 16px, I suggest going for that. This can be implemented with more precise fractions, assuming a 11px font-size but leaving room for font-size changes in the future (if any):

.tag-line {
  min-height: 1.4545em;
  line-height: 1.4545em; /* or 1.4545 (inherited as a ratio rather than as a computed pixel value) */
}

This would enable us to try and achieve more precise and less jumpy micro-layouts for editor textareas, and more precise vertical alignment for event/grid/flex pill buttons.

Re. content density, this means that when we were showing 26 lines in a viewport, we would now be showing 25 lines instead.

Comment 1

[Florens Verschelde :fvsch]

Patrick, do you think it's worthwhile pursuing this?
IMO it unlocks more "UI precision" improvements to the markup view.

Comment 2

[Patrick Brosset <:pbro>]

Yes I think it is.

Comment 3

[Florens Verschelde :fvsch]

Attachment: markup-view-line-height-16px-vs-14px.png

So I’ve looked a making the markup view's line height more precise, and the good news is that it's doable and it allows us to more precisely align the markup badges ("event", "grid", "flex", and "...").

On the design front, I think we have two options:

1. Left: increase the current line-height (15.4px) a tiny bit, to 16px.
2. Right: decrease the line-height from 15.4px to 14px, and use 1px padding to achieve a 16px row height. This makes rows that wrap to several lines (tags with many attributes, long text nodes) *smaller* than currently.

So the design question is: which do we like best, left or right?

For context:
- CSS property:value lines in Rules use 14px line-height
- Console rows use 14px line-height
- Debugger source code uses "normal" line-height, resulting in 13.5px line-height on some platforms/fonts (we've talked about setting it to 14px)
- font-size in all these contexts and in the markup view is 11px

So it could be interesting to go with the 14px option, for consistency's sake.

Comment 4

[Florens Verschelde :fvsch]

Attachment: Bug 1501997 - Set markup-view line-height to 14px; r=pbro

Set markup view lines to use a precise 14px line-height, 16px total height.
Align markup badges more precisely with vertical-align:top and margin-top,
because vertical-align:middle is imprecise and has unwanted side effects.

Comment 5

[Florens Verschelde :fvsch]

I have a tentative patch on Phabricator for the 14px option, but it can easily amended for 16px.

I've also aligned the drag-target and drop-target markers (they're 2px tall, so a top:-1px centers them), and the markup badges using vertical-align:top and margin-top (because vertical-align:middle is often just wrong, and has side effects such as increasing the line's height beyond the specified line-height).

Comment 6

[Patrick Brosset <:pbro>]

Comment on attachment 9020540 [details]
markup-view-line-height-16px-vs-14px.png

I think I prefer a line-height of 14px with some padding to make each node be on a 16px line.
This way, each node is nicely separated from the rest, and nodes that wrap are a bit more compact.
Also because of the consistency reason you mentioned.

Comment 7

[Florens Verschelde :fvsch]

Attachment: markup-line-height-current-vs-14px.png

On Linux at 1x pixel density: current Nightly (left) versus 14px line-height (right).

For the record, the markup badge is 12px tall and the "expand" (ellipsis) badge is 10px tall. We can try making them 14px and 12px if the small height is a concern, but then they look a bit bulky IMO.

Comment 8

[Victoria Wang [:victoria]]

Comment on attachment 9020540 [details]
markup-view-line-height-16px-vs-14px.png

Sorry for the delay on this! I agree, the 14px does have a nice grouping effect on the nodes, and the consistency with other panels will be great.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a8540eb0e90
Set markup-view line-height to 14px; r=pbro

Comment 10

[Florens Verschelde :fvsch]

Late try, all green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fe2e7b6df8eabb403298ea6292a8a24af62427c

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/7a8540eb0e90