Fixing vertical (and horizontal) rhythm in debugger right sidebar

[jaril]

Taking the lead from #34 and #6631, I went ahead and extended the same design to the rest of the right sidebar panes (watch expression, breakpoints, scopes, XHR breakpoints). Where there was no detail I interpolated from the discussions and used my judgement.

Note that this does not include the Call Stack pane as there's already a PR working on it.

Overview

Pane Breakdown

Guidelines

General:

• Components should be snapped to a 4px grid vertically and horizontally (per Photon spec)
• Pane headers are font-size 12px, code breakpoints (monospace) are 10px, the rest are 11px
• Rows are standardized with 20px height

Vertical:

• There should be a 4px top and bottom padding for any list or input box
  Click here for example

  
• Where there’s no list/input box for the pane, a 4px bottom padding should be added to the option element
  Click here for example

  
• 4px bottom paddings should be inclusive of a 1px border (3px padding + 1px border)
  Click here for example

  
• When a 1px border is needed between two elements, they should be added to the first element and not the second element
  Click here for example

  
• Elements with a border should have a height (border included) that is aligned to the 4px grid
  Click here for example

  

Horizontal:

• All elements (apart from pane headers) should be indented from the left by 20px
  Click here for example

  
• The indented element should include both the text label and any checkbox/arrow
  Click here for example

  
• The text label should start 16px after the start of checkbox/arrow to its left
  Click here for example

  
• Indented children elements should include its parent's indentation and a new 16px indent
  Click here for example

  

Current panes

With measurements and vertical separation

Click here to show picture

Proposed panes

With measurements and vertical separation

Click here to show picture

Without markup

Click here to show picture

[jaril]

Opening the issue here since it's UX related - once the ball gets rolling we can move it to debugger.html if that's better.

Thoughts? @darkwing @mcroud @fvsch @violasong

[darkwing]

@fvsch @violasong @mcroud Could we fast track evaluation and discussion on this? Getting hard numbers ASAP would be incredibly helpful as we look at to add Event Listener and DOM Listener panels in Q1

[DPX-designer]

Superb! This looks really really good. So I'm just going to go through your items here and relay my stream of thought! I'm happy with the 4px padding for top and bottom of panels being a standard.

Pane header text as 12 and pane body as 11 went down well with the Callstack panel so I'm very happy with that. Could you just elaborate on "code breakpoints are 10px", is this because monospace renders larger anyway?

Rows are standardized with 20px height

I recall a discussion or proposal about having the option of two accepted row sizes? Not sure if anyone can elaborate on my fuzzy memory. Regardless 20px worked well for Callstack pane so happy here.

All elements (apart from pane headers) should be indented from the left by 20px

It might be me interpreting this wrong and that you're including the twisty arrow here, but the pane headers are also 20px from the left, the below image is from the yet to be finalised pane's documentation:

So the pane header starts (going from left to right) with 6px padding, then an 8px twisty arrow, then another 6px padding.

I like standardising the 16px horizontal inset for checkbox and child rows.

For the right edge padding of pane headers I believe the first pane doc draft proposed 20px, so on the below image for example we would have 20px between the "+" and the right edge. Looking at a pane with closable rows with ❎ buttons perhaps 20 is too much, we have 12px proprosed here, what does everyone else think?

The 'twisty' arrow should be the new one below: Pane-arrow.svg.zip An 8 x 8 pixel icon containing an 8 x 5 pixel SVG.

Aside from a little clarification there's little I could say I have an issue with here, cheers and good job!

[jaril]

Pane header text as 12 and pane body as 11 went down well with the Callstack panel so I'm very happy with that. Could you just elaborate on "code breakpoints are 10px", is this because monospace renders larger anyway?

Exactly - the monospace font used for code (Menlo) rendered larger, where Menlo 10px is equivalent to the default font (San Francisco) 11px.

I recall a discussion or proposal about having the option of two accepted row sizes? Not sure if anyone can elaborate on my fuzzy memory. Regardless 20px worked well for Callstack pane so happy here.

The one I've found was in #6631, where 24px was as an option if taller rows were wanted. It was also discussed in #34, though I agree with the line of thought that for dev tools, compact information is preferable.

It might be me interpreting this wrong and that you're including the twisty arrow here, but the pane headers are also 20px from the left, the below image is from the yet to be finalised pane's documentation:

I just meant that any non-header rows should be indented 20px from the left. As for pane headers, I like your mockup and agree the pane header text should be 20px from the left. Although with my current measurements I think it's actually 19px from the left (4px padding+15px arrow)

[fvsch]

Superb! This looks really really good.

Agreed.

For the expand arrow (well, triangle), you can find it here too: https://searchfox.org/mozilla-central/source/devtools/client/themes/images/arrow.svg

I recall a discussion or proposal about having the option of two accepted row sizes?

We mentioned that both 20px and 24px could work, and we needed to try them out. I’m pretty happy with how 20px renders here. With the added 4px top/bottom padding for each row group, it doesn't look cramped. I’d say 20px is a keeper for this sidebar.

When a 1px border is needed between two elements, they should be added to the first element and not the second element

I think both elements should be 20px without the border. The border can be implemented as a separate 1px, or as a border-bottom on the first element. If it's implemented as a border-bottom on the first element, that element will be 21px (20px excluding the border).

My understanding is that we're not trying to adhere to a perfect vertical rhythm where every text line sits on a global vertical grid (that was a design trend a while ago, but it's very impractical to do and creates bad side effects, for very little gain). Mostly, we're trying to make sure that elements have similar heights and don't appear too cramped or too tall.

The text label should start 16px after the start of checkbox/arrow to its left

I’m not sure the checkbox will be 12px wide on all platforms. They tend to be bigger on Linux, for instance, so we probably can't have this requirement.

For the expand arrow icons in the "Scopes" section, I’m not sure we want to implement them like this, with the triangle's left side always touching the left "guide". That means that we would have to rotate those icons and translate them at the same time, which is a bit hard to implement consistently and may not feel great in practice. Currently we're rotating them only (around their center point).

[fvsch]

For typography, it would be great if we could use the main existing font-family/size combinations, rather than adding new ones. That would be:

1. Headers and tabs: sans-serif at 12px (line-height often unspecified, but 16px is good here)
2. Other non-code text labels: sans-serif at 11px (line-height often unspecified, but 14px is good here)
3. Code: monospace at 11px (line-height often at 'auto', resulting in 13 or 13.5px; 14px in the Console; planning to move to 15px for the Inspector, Console and most source code views)

(See #2 for generic Typography discussion.)

It would be nice to also decide on what content should be sans-serif and what content should be monospace. It seems that the Debugger's sidebar is mostly sans-serif, even for JavaScript object representations and values, with one exceptions: code breakpoints. That feels strange. Maybe we should use sans-serif for everything there (alterntively, use monospace for more things). It might be that we only have monospace for code breakpoints because of the syntax coloring implementation, which we could override.

@darkwing Do you know why we have monospace for the code breakpoints exclusively? Are we using CodeMirror to do syntax highlighting there maybe?

I'll also argue that JavaScript object representations (e.g. the expanded arr(10) in Watch Expressions and window in Scopes) should be more compact, and use a tighter line-height (probably 14px, maybe 15px). That would be more consistent with how we use object representations in other tools (Console, Network details).

[jaril]

My understanding is that we're not trying to adhere to a perfect vertical rhythm where every text line sits on a global vertical grid (that was a design trend a while ago, but it's very impractical to do and creates bad side effects, for very little gain).

I was specifically trying to address the inconsistent vertical rhythm by following a consistent rule, hence the strict adherence to the 4px grid. It's possible I just haven't thought about this enough, but the only difference I can think of is that it enclosing rows would have -1px to the padding and +1px to the border.

For elements that require a bottom border I prefer the 20px total (including 1px border) approach instead of 21px (including 1px border), specifically because most (if not all) rows use box-sizing: border-box which includes padding and border in the height property. The first approach keeps the height properties between rows (border or no border) predictable, which is why I prefer it.

Can you elaborate on how it could be impractical and create bad side effects?

For the expand arrow icons in the "Scopes" section, I’m not sure we want to implement them like this, with the triangle's left side always touching the left "guide".

Yes the rotate behavior does get tricky, and it's likely better keep it to leave it the way it is. As it is, the original arrow points down (is "expanded") and touches the left guide and then when it is "collapsed", the left side of the arrow will be a few pixels away from touching the left guide.

It would be nice to also decide on what content should be sans-serif and what content should be monospace. It seems that the Debugger's sidebar is mostly sans-serif, even for JavaScript object representations and values, with one exceptions: code breakpoints. That feels strange.

I personally prefer having the breakpoints as monospace because reading code that is not in a monospace font is jarring. Though extending that thought, what else counts as code? How about watch expressions or object reps with properties?

@darkwing could use your input here.

I'll also argue that JavaScript object representations (e.g. the expanded arr(10) in Watch Expressions and window in Scopes) should be more compact, and use a tighter line-height (probably 14px, maybe 15px). That would be more consistent with how we use object representations in other tools (Console, Network details).

That would be in line with what they were before, which was 15px height. I find the properties easier to read at the 20px height, though if consistency with other tools is of higher priority we should stick with the original 15px height. Or 16px if we follow the 4px grid.

[darkwing]

Let's keep fonts as is for now -- it's an unrelated, simple change

[DPX-designer]

I thought I would drop in with my recent mockups to align with what we've decided upon so far!

Below is an image of the Event Breakpoints panel (let me know if you spot something that doesn't align with the above so far 😄 )

it's likely better keep it to leave it the way it is. As it is, the original arrow points down (is "expanded") and touches the left guide and then when it is "collapsed", the left side of the arrow will be a few pixels away from touching the left guide.

I agree with both of you, in the image above the 20px measure besides "Drag & Drop" doesn't touch the arrow, but the expanded arrow in the "Keyboard" row does touch the 20px measure.

I’m not sure the checkbox will be 12px wide on all platforms. They tend to be bigger on Linux, for instance, so we probably can't have this requirement.

In my mockup I started the event category names 4px from wherever the checkbox ended. Do we think this is the best course of action given the possibility of different checkbox sizes?

I think the only remaining discussion point is whether there is a sort of fixed baseline of 20px for rows which may include a 1px border, or whether rows are 20px and may have a 1px border between them (not apart of the 20px row height). Does that sound right?

[fvsch]

Do we think this is the best course of action given the possibility of different checkbox sizes?

We should check the checkbox size on Windows (Windows 10 only maybe). If it's in the 12-14px range, we should be good. I'm okay with having a potential 1-2px imprecision range here, between Windows and macOS. On Linux, the checkboxes may be bigger, but I suggest that we don't optimize for that.

I think the only remaining discussion point is whether there is a sort of fixed baseline of 20px for rows which may include a 1px border, or whether rows are 20px and may have a 1px border between them (not apart of the 20px row height).

Strongly in favor of rows being 20px excluding borders (if any). This is how we manage toolbars, for instance. (When we don't, in a couple places, it's a known bug with work-in-progress patches.)

On a 20px grid, if you have a 14px object that is vertically centered, you can have 3 pixels of padding on the top and the bottom. If you eat one pixel of the bottom padding to draw a border, you get a misaligned object: 3px whitespace on top and 2px whitespace on bottom. This 3/2 difference is humanly visible and feels off, IMO. (On a bigger scale it can be less of an issue.)

Content rows should not be implemented with height: 20px, because that does not respond well to text that wraps on 2 or more lines, font-size changes, etc. The CSS implementation should probably look more like this:

.some-content-row {
  padding: 3px 8px;
  padding-inline-start: 20px;
  /* font-size and line-height values will be var(--theme-*) variables,
     so that we use consistent values all over devtools and make it possible
     to change them later on or offer theming features */
  font-size: 11px;
  line-height: 1.2727;
}

Then you get a 20px row for one line of text, 34px for two lines, and so on. If you add a border, the element itself will be 21px tall, but its content will be (and look) 20px.

Final note: if at one point we use a different font-size and/or line-height, e.g. bumping our font-sizes from 11px to 12px or line-heights from auto/13px/14px to 15px/16px in a few places, we may end up with rows that are 21px or 22px instead of the initial 20px. I think it's a worthy trade-off, and we should not obsess about matching the 20px/24px values.

For instance in the Console we currently have 20px-high rows, but we're likely to bump the line-height to 15px and end up with 21px rows. In the Inspector, we may use 15px rows in the markup view and Rules. In the Debugger, we may end up using 15px rows for source files.

[jaril]

On a 20px grid, if you have a 14px object that is vertically centered, you can have 3 pixels of padding on the top and the bottom. If you eat one pixel of the bottom padding to draw a border, you get a misaligned object: 3px whitespace on top and 2px whitespace on bottom. This 3/2 difference is humanly visible and feels off, IMO. (On a bigger scale it can be less of an issue.)

Then you get a 20px row for one line of text, 34px for two lines, and so on. If you add a border, the element itself will be 21px tall, but its content will be (and look) 20px.

This makes sense, though it's not a problem with the right sidebar since none of the content rows wrap their text. I do see how flexibility with font-size/line-height and symmetrical visual spacing is handy across devtools, so not being restricted to the 4px multiple for sizing is a good thing.

I think the only remaining discussion point is whether there is a sort of fixed baseline of 20px for rows which may include a 1px border, or whether rows are 20px and may have a 1px border between them (not apart of the 20px row height). Does that sound right?

I think so, and having 1px border between them sounds good!

[jaril]

Last remaining point is:

I'll also argue that JavaScript object representations (e.g. the expanded arr(10) in Watch Expressions and window in Scopes) should be more compact, and use a tighter line-height (probably 14px, maybe 15px

I'm biased towards having more spacing and having it be 20px high, since eye strain while inspecting object representations is one of my least favorite things. Although I also see the case for keeping it the way it is (15px) so that you can fit more lines in the same space, and hopefully find what you're looking for quicker.

@darkwing thoughts on 20px vs 15px for JS object reps?

Once we tie this part up, I can come up with a final spec (changing this and the border) and move it to debugger.html to start implementing. If I'm missing anything else let me know!

[DPX-designer]

Great, I think we will settle with 1px borders being seperate from rows then 👍

A quick and dirty visual comparison:

[darkwing]

I defer to @mcroud on the 15px vs. 20px thing. :)

[fvsch]

I vote 15px for consistency with how we display those objects in the Console. (Currently with a 14px line-height, moving to 15px in the near future.)

[DPX-designer]

I'm 100% for making things easier to scan for users, but I'm also not aware of any reported issues with the current 15px implementation which would make the argument to change it a lot easier (I'm not the best searcher in Bugzilla but wasn't able to find anything - not that it NEEDS to be reported to be an issue mind.)

We do provide the means to allow users to zoom in and increase the text size so I feel in this instance, given that this is also how it has looked in the console object inspector panel, I would vote to remain at 15px - at least for now.

[jaril]

Here's the newest mockup tacking on the two changes discussed:

1. 1px borders should be outside of the 20px row
2. keep object representations at 15px per row

Just needs the thumbs up from @fvsch and @mcroud, then we can go ahead and file this along as an issue over at devtools-html/debugger.html. If something's lacking, let me know.

Preview

Overview of new panes

[fvsch]

I'm not sure what you are representing with the dotted borders.

If you wanted to add dotted borders in the rendered UI, I think this makes things a bit too visually busy. If it's just a representation of spacing (representation which should not be rendered UI), they're mostly okay, except we're probably not going to leave 1px blanks between rows when we don't have a visible border, so those 1px spaces are probably not necessary. (We can move to implementation with this caveat.)

Otherwise things look great. My only nitpick would be that the "Pause on exceptions" section could use a 2px or 4px vertical padding (making its total height 24 or 28px), right now it looks a bit cramped.

[jaril]

The dotted borders are just a design guideline for visually identifying separate vertical rows to make it easier to match pixel values to what it is they're measuring. The measured values are still correct for implementation, as the dots just signifies the bottom inner border of the previous row.

For example, an element with 10px height is visualized in the mockup as a 9px of content + 1px of border on bottom. Ignoring the 1px border on the bottom still gives you the 10px as identified in the measurements.

On second thought I should have taken them out of the side-by-side preview. Here it is without the dots:

[IMG] Side by side without dotted lines

Agree on Pause on Exceptions/Pause on any URL being too cramped. Here are mockups for 2px/4px. I'm leaning towards 4px for consistency - let me know what you think.

[fvsch]

Agree on Pause on Exceptions/Pause on any URL being too cramped. Here are mockups for 2px/4px. I'm leaning towards 4px for consistency - let me know what you think.

The consistent 4px is great, let's go with that.

Thanks again for all your work, and the level of detail of the blue/red lines. :heart:

[jaril]

Final spec (with the new 4px padding around options checkboxes)

Preview

Overview of new panes

[violasong]

Outstanding work! Thanks so much @jaril. I can't wait to see this implemented!

[violasong]

And of course, big thanks to Florens and Matt for y'all's work on this too :)

[DPX-designer]

Wonderful! what a collaborative result 🎉 Final spec looks great! Thanks @jaril and @fvsch 👏

[digitarald]

@violasong would it make sense to add those results to the devtools photon docs?

[violasong]

@digitarald Yes, good point - will add to backlog.

[violasong]

(I think this issue can be closed - let me know if otherwise.)

[fvsch]

Yep, good to close.

There are some good mockups in this issue that we could reuse -- or get inspiration from -- of we want to document some patterns, but that's true of a few already closed issues. If we get around to making more generic design docs, we can revisit the closed issues to retrieve some material.

[violasong]

That sounds good to me - thanks Florens!