Add borders to Console rows

RESOLVED FIXED in Firefox 68

Status

enhancement
P2
normal
RESOLVED FIXED
5 months ago
Last month

People

(Reporter: fvsch, Assigned: fvsch)

Tracking

(Depends on 1 bug, Blocks 1 bug)

unspecified
Firefox 68
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(7 attachments, 2 obsolete attachments)

Assignee

Description

5 months ago

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

Assignee

Comment 1

5 months ago

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

Assignee: nobody → florens
Status: NEW → ASSIGNED
Duplicate of this bug: 1397260
Severity: normal → enhancement
Priority: -- → P2
Assignee

Comment 3

5 months ago
- 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
Assignee

Comment 4

5 months ago

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.

Assignee

Comment 5

5 months ago

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.

Iteration: --- → 67.1 - Jan 28 - Feb 10
Depends on: 1520440
Target Milestone: --- → Future

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

Assignee

Comment 7

3 months ago

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.

Duplicate of this bug: 1495724
Duplicate of this bug: 1004752
Duplicate of this bug: 879310
Assignee

Comment 11

Last month

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

Attachment #9036693 - Attachment is obsolete: true
Assignee

Comment 12

Last month

New patch in progress.
Result in dark theme.

Assignee

Comment 13

Last month

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?

Flags: needinfo?(victoria)
Assignee

Comment 14

Last month

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).

Assignee

Updated

Last month
Blocks: 1549195
Posted image 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.

Flags: needinfo?(victoria)
Assignee

Comment 17

Last month

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?

Assignee

Comment 18

Last month

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?

Flags: needinfo?(victoria)
Assignee

Comment 19

Last month
Posted image console-borders-groups.png (obsolete) —

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>
Assignee

Updated

Last month
Attachment #9064499 - Attachment is obsolete: true

Comment 21

Last month
Pushed by florens@fvsch.com:
https://hg.mozilla.org/integration/autoland/rev/23ab80e6ec53
Add borders between console messages; r=nchevobbe
Assignee

Comment 22

Last month

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.

Flags: needinfo?(victoria)

🎉🎉🎉

Comment 24

Last month
bugherder
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: Future → Firefox 68
Duplicate of this bug: 878519
You need to log in before you can comment on or make changes to this bug.