1519904 - Add borders to Console rows

Status: RESOLVED FIXED

(DevTools :: Console, enhancement, P2)

Description

[Florens Verschelde :fvsch]

We'd like to add a 1px border between each row in the Console, to help with legibility when logging long text-heavy elements such as JS objects and stack traces.

UX discussed in https://github.com/devtools-html/ux/issues/42

Comment 1

[Florens Verschelde :fvsch]

Taking this bug to do a test implementation, but I think it's not 100% sure we'll go with it.

Comment 3

[Florens Verschelde :fvsch]

Attachment: Bug 1519904 - Add borders to Console rows; r=nchevobbe

- Change Console line-height to 15px (from 14px)
- Add borders to console messages, with different subtly contraste red, yellow and blue shades for navigation, error and warnings.
- Refactor console-specific CSS variables for internal consistency, and so that they all start with --console-
- Remove the vertical jump between the validated input and its display as a message row
- Add a globe icon designed at a 12px size for crips rendering on low resolution displays

Comment 4

[Florens Verschelde :fvsch]

So my patch is a bit massive and is not strictly limited to adding borders and styling them. But in order to make sure that this style works visually and in practice, I wanted to get in other tweaks such as the 15px line-height and removing the jump between input (CodeMirror) and its display as a message. From there, it made sense to refactor the Console's CSS variables to make things less messy and manageable.

So I would rather keep the patch this big, if possible.

One thing that could be moved to a different patch and even a different bug is the 12px globe icon, but since it's unlikely to add any instability I'm not sure it's worth cutting it out.

Comment 5

[Florens Verschelde :fvsch]

Will split the bug in a few patches:

1. variable refactor
2. icon tweaks
3. border and stacking of messages
4. bringing the CodeMirror input in line (eliminating the vertical jump)

Maybe keep 3-4 in the same patch if it's small enough.

I'm planning to work on this in the 67 cycle.

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Hello Florens, is there anything we can do to help you here?

Comment 7

[Florens Verschelde :fvsch]

Not much, I focused on Debugger this cycle and wanted to finalize this in parallel but was short on time.
I'll try to prioritize this in the next 2 weeks.

Comment 11

[Florens Verschelde :fvsch]

Moved the 12px globe icon and related icon updates to bug 1549185.

Comment 12

[Florens Verschelde :fvsch]

Attachment: console-borders-dark.png

New patch in progress.
Result in dark theme.

Comment 13

[Florens Verschelde :fvsch]

Attachment: console-borders-light.png

And here's the result in light theme.

Victoria, I think we already discussed this style on GitHub but since that was months ago, do those two screenshots look good to you or would you like some changes?

Comment 14

[Florens Verschelde :fvsch]

Attachment: Bug 1519904 - Add borders between console messages; r=nchevobbe

Add borders to console messages, using the same colors and layout strategy (border-top: ...; border-bottom: ...; margin-top: -1px; + stacking with z-index) as in D16592, but this time limiting changes to webconsole.css and avoiding scope creep :P

One thing that I did include was renaming a lot of CSS variables. Since I was adding a bunch of CSS variables for border colors and some background colors, it just made sense to use variables for navigation marker's text color, and harmonize a few other things. I avoided changing anything used in Reps or other places though.

I had created a separate bug (Bug 1549195) for the variables refactoring, but my plan now would be to use that bug to follow up on variables like --error-color and --console-output-color, and maybe try to add variables for log point messages when they land (D29040).

Comment 15

[Florens Verschelde :fvsch]

Try push (mochitests + talos-damp):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=62800490ad43fb70cea4b504d9f0f9dcf442a447

Comment 16

[Victoria Wang [:victoria]]

Attachment: image.png

Thanks for restarting this work Florens! Looking at the previous screenshots, I'm really liking this. One thing - to my eyes, the light mode gray lines look a little too dark. I'd suggest 40%-50% lighter.

This may be going out of scope, but I noticed that in Chrome, input/output is combined into one outlined section. This would be nice to give it more variable rhythm to tone down the striped look.

Comment 17

[Florens Verschelde :fvsch]

Attachment: console-no-border-between-input-and-output.png

I can easily remove the top border from a "result" message in CSS. The hard part is knowing when it's appropriate to do so.

Nicolas, it seems I can get a decent result with:

.message.command + .result {
  border-top-color: transparent;
}

This applies when evaluating an expression in the console, but not when evaluating console.log(...) because the log message is displayed between the command and the result (per the Console API spec I believe).

If we want to remove the borders between those 3 entries, we can perhaps try:

.message.command + .result,
.message.command + .log,
.message.command + .log + .result {
  border-top-color: transparent;
}

But:

1. A lot of messages have the log class, sometimes even log log. It's maybe used too liberally? Or maybe the log log case should have a different class name for "log of type log", something like log basic-log?

2. I guess we could have race conditions and/or logs from other scripts that manage to end up right after a .command (or maybe it's safe?). If that's a risk, can we perhaps have a class specific to calls to console.log/info/debug that come from the Console itself?

Comment 18

[Florens Verschelde :fvsch]

Attachment: console-no-borders-between-command-and-result.png

Trying out:

• #f2f2f4 for the default message border in light theme; somewhere in between Grey 10 (#f9f9fa) and Grey 20 (#ededf0). Grey 10 is very hard to distinguish from white on most screens (especially as a 1px line), so an intermediate value seemed to made sense. For comparison: Chrome uses #f0f0f0.

• No border between command and output, when not separated by a log result. We still keep a border when there is a log between command and output (e.g. if the command is console.log(something)). We keep borders if the result is an Error. Checked in Chrome, and this is what they do too.

Another question: do we want to keep the 3px blue border shown on the left when hovering a row? With borders, it might become less useful?

Comment 19

[Florens Verschelde :fvsch]

Attachment: console-borders-groups.png

Updated screenshot: attachment 9064500 [details]

One small limitation of the current implementation: with the current DOM structure, we can't detect if two messages are on the same indentation level.

In this screenshot, you can see that the console.groupEnd() "command" at a second level is followed by an unrelated first-level "result", so we lose the border between the two.

I think that might be a very rare scenario though, so I could live with it.

One solution would be to change the DOM structure so that we have the indent level information on the message container itself:

<!-- before: -->
<div class="command">
  <span class="indent" data-indent="1" />
</div>
<div class="result">
  <span class="indent" data-indent="0" />
</div>

<!-- after: -->
<div class="command" data-indent="1">
  <span class="indent" />
</div>
<div class="result" data-indent="0">
  <span class="indent" />
</div>

Comment 20

[Florens Verschelde :fvsch]

Attachment: No border between command and result on different indentation levels

(Slightly nicer screenshot.)

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23ab80e6ec53
Add borders between console messages; r=nchevobbe

Comment 22

[Florens Verschelde :fvsch]

From Victoria:

Your latest screenshots there look great! The way the vertical blue line overlays it looks a little odd but I think it’s fine for now. I’m not worried about the rare missing line issue.

Let's land this, and usage will show if we should improve some details.

Comment 23

[Nicolas Chevobbe [:nchevobbe]]

🎉🎉🎉

Comment 24

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/23ab80e6ec53