[Console] Eager Evaluation UI

[digitarald]

PRD - Meta bug

Initial UI is landing in https://phabricator.services.mozilla.com/D58842 . Lets review it to polish the rough edges while it rides the 74 train, and file follow up polish for later.

cc @jasonLaster @nchevobbe @janodvarko @bhackett1024

[digitarald]

https://phabricator.services.mozilla.com/D58842 landed and should be in the next Nightly build 🤞. Enable it via devtools.webconsole.input.eagerEvaluation.

[digitarald]

One aspect for polish I'd like to discuss is error formatting:

The eager eval for those right now looks rather bland compared to the rich output logs. Another aspect is that Chrome does not show errors for eager eval, so we might also want to tone them down as they can be distracting while iterating on code.

[digitarald]

@violasong Nicolas started looking at UI for editor mode, maybe you can help out: https://bugzilla.mozilla.org/show_bug.cgi?id=1609941#c3

[violasong]

Some initial thoughts:

This is awesome!

The eval coloring right now seems a bit too prominent - I wonder if we should gray it out or lower opacity like Chrome does to make it seem more like a preview; something that's not real yet. Right now it all blends in a bit. I may try to adjust the margin a bit to take less space too.

I think we can make the "autofill" effect a lighter gray as well.

The prominence of the error message seems fine to me, given that it's not quite an error yet.

Multi-line with eval at the bottom sounds interesting, would be nice to see that in action. A bit tough to judge how the other option feels without using it.

cc: @nchevobbe

[nchevobbe]

Here's what it looks like with the eager evaluation result at the bottom of the editor:

I must say I quite like it.
For other changes, I'll try to implement some variation and and post pics here

[violasong]

This bottom of multiline UI looks good to me!

Btw, I was playing with our UI vs Chrome's and I realized ours is a lot more active-looking. Partly because chrome doesn't eval many keywords, but also because if the output is just function() they use just the f symbol instead. (e.g. for "alert") Seems like it could be nice to follow suit.

Also, when typing a character (e.g. a) there's a flash of red error text - would be great if we could get rid of that.

[digitarald]

because if the output is just function() they use just the f symbol instead. (e.g. for "alert") Seems like it could be nice to follow suit.

Eager eval definitely highlights some areas of improvement for both autocomplete and object inspector, as they are shown a lot more often than before – making polish in those areas more interesting. It would be great to have your input on what to prioritize.

Also, when typing a character (e.g. a) there's a flash of red error text

Filed bug 1612999

[violasong]

I do like how the f simplifies the appearance. I don't know how commonly understood it is, but it's probably safe enough based on Chrome parity.

This goes off topic, but we should improve our syntax highlighting here as well.

Filed bug 1612999

Thanks!

[violasong]

Does it make sense to have error messages show for keywords that exist in the autocomplete, such as await? I assume it's not useful to see a big bold ReferenceError: "await is not defined" here, because the keyword is probably just going to be part of some input that the user is in the process of typing.

Also, I think we could make the error handling less strict for situations where the casing doesn't match - presumably, the user will type enter and the case will be corrected.

[violasong]

Here are new mockups that show the ideas from my previous comments.

https://www.figma.com/file/cgy83M9scrlGu9o6WbiDsY/instant-eager-eval?node-id=1%3A2

[digitarald]

Does it make sense to have error messages show for keywords that exist in the autocomplete, such as await?

Do you mean keywords that don't exist in the autocomplete? One stop gap would be expose await in the autocomplete.

ReferenceError is also maybe also an error we don't want to show for single-word inputs (as one possible heuristic to show useful errors). WDYT, @nchevobbe?

Also, I think we could make the error handling less strict for situations where the casing doesn't match - presumably, the user will type enter and the case will be corrected.

Yes, that's a bug for sure. ani should just work as Ani for preview.

[violasong]

Do you mean keywords that don't exist in the autocomplete?

await actually does show up in the autocomplete for me (I just picked this at random, but many keywords in the autocomplete do trigger the error)

Yes, that's a bug for sure. ani should just work as Ani for preview.

Great!

[digitarald]

await actually does show up in the autocomplete for me (I just picked this at random, but many keywords in the autocomplete do trigger the error)

Ah, I see what you mean now, the preview for the autocomplete yields the error. Maybe erroneous autocomplete previews should be ignored.

[violasong]

This mockup should now be complete! (Thanks to discussions from slack and twitter). Let me know if you have any questions:

https://www.figma.com/file/cgy83M9scrlGu9o6WbiDsY/instant-eager-eval?node-id=1%3A2

Ah, I see what you mean now, the preview for the autocomplete yields the error. Maybe erroneous autocomplete previews should be ignored.

Yes, seems like it would be good to err on the side of less noisy error previews.

[nchevobbe]

thanks @violasong , I'll try to implement this today so it ships in 74

[nchevobbe]

Here are some screenshot with an object containing different types of data

Light inline

---

Dark inline

---

Light Editor

---

Dark Editor

---

Does it look good to you @violasong ?

[nchevobbe]

(the changes to the function keyword for logged objects will be done in another bug since it's not related to eager evaluation)

[nchevobbe]

simpler in-line to check height:

when eager evaluation is disabled by the user, the input looks the same way as now:

(the dark block at the bottom is here so you can see the end of the window better :) )

[nchevobbe]

Alternative placement in editor mode:

really not sold on that, it kind of disconnects the result from the input

[jasonLaster]

Thanks @nchevobbe

1. colors - look good
2. bottom placement - it feels good to give it as much space as possible.

[digitarald]

really not sold on that, it kind of disconnects the result from the input

Not perfect, but afaik better than the smaller space; given how large most previews are.

[violasong]

Thanks Nicolas for this awesome fast work!

Tiny spacing adjustments for non-editor instant eval mode: can we make the instant eval part 2px higher, with the eval arrow an extra 1px higher? Also, 1px less top and bottom padding in the input field will make it match the height of a 2-line output.

Question: the magenta text in the eval seems a bit strong - should that be turned light gray, or does it deserve that prominence?

Bottom placement - this does seems odd to me due to the disconnection you mentioned. I wonder if it would be better to truncate instead. Does instant eval mostly just serve as a hint (where users will just glance at it, and only really need the first part) or is it often going to be a reference (where users will want to read the whole thing)?

[nchevobbe]

Question: the magenta text in the eval seems a bit strong - should that be turned light gray, or does it deserve that prominence?

I don't know. In the mockup the numbers were keeping their green colors, so I assumed we might want to do the same for strings. We can try g50 for them maybe?

Bottom placement - this does seems odd to me due to the disconnection you mentioned. I wonder if it would be better to truncate instead. Does instant eval mostly just serve as a hint (where users will just glance at it, and only really need the first part) or is it often going to be a reference (where users will want to read the whole thing)?

So we do truncate now. We can't really now for what users are looking for in this. From my personal experience, I'm mostly looking that I'm getting the right type of data / shape of object/array, but other might have different approaches. One thing to note is that when in editor mode, I suspect people will have a "wide-enough" editor panel, so they can write code more easily (also might not be true). Finally, if they want to see the whole result, they can resize the editor, or can evaluate. We could also make it take the whole width on hover maybe? That could be interesting.

[nchevobbe]

grey-50 (light) / g43 (dark) strings:

---

spacing tweaks

[nchevobbe]

Expanding the eager evaluation result in editor mode on hover:

[jasonLaster]

heh, i think the on hover might be over-thinking it 😉

one of the goals of eager-eval is to see the result immediately, so asking the user to hover might take away some of the benefits.

[digitarald]

Hover seems a stretch as this is a keyboard-only input for most.

[fvsch]

Two small details:

1. Truncation: can we do it with a fade-out mask, rather than an ellipsis (…)? Like the tab title in Firefox tabs.

2. Spacing: when not using the Editor mode, hitting Enter and going from input+eager result to history+actual result is a big vertical jump. Should we make the spacing tighter, similar to what the output will look like? See: console-spacing-of-eager-to-actual-result.mov.zip

[violasong]

Truncation: can we do it with a fade-out mask, rather than an ellipsis (…)? Like the tab title in Firefox tabs.

Interesting idea - we don't currently do this in DevTools (I think), but this could look nice and remove the risk of the ellipsis looking like part of the code.

Spacing: when not using the Editor mode, hitting Enter and going from input+eager result to history+actual result is a big vertical jump. Should we make the spacing tighter, similar to what the output will look like? See: console-spacing-of-eager-to-actual-result.mov.zip

The new tweaks should fix this by matching the height of a double row. Though, from the newest screenshots, it looks like there might be 1px too little padding on the top and bottom of the input now?

In the mockup the numbers were keeping their green colors, so I assumed we might want to do the same for strings. We can try g50 for them maybe?

Thanks for trying this out! I was just looking at this more and it looks like our normal string color and number color is a dark blue, at least in the Debugger. I'm not sure why the colors are magenta, black, or green in certain situations. Your current screenshot looks good, but I could also see using the dark blue for both strings and numbers.

For multi-line, I think the truncation solution is still best for the UI. Hover-reveal would be very cool for other UIs I've thought about though, so I'll keep that in mind :)

[fvsch]

@violasong: I'm not sure why the colors are magenta, black, or green in certain situations

So I looked into it and it looks like we have two sets of colors for JS code.

1. Our CodeMirror theme, used in Debugger files and for Console input
2. The Reps component, used in the Debugger sidebar (e.g. in Watch Expressions), and in Console output (e.g. when you do console.log(document)).

For strings and numbers in the light theme, in CodeMirror I'm finding:

.cm-s-mozilla .cm-number {
    color: var(--theme-highlight-purple); /* Blue 70 */
}
.cm-s-mozilla .cm-string {
    color: var(--theme-highlight-purple); /* Blue 70 */
}

and in Reps I'm finding:

.theme-dark,
.theme-light {
  --number-color: var(--theme-highlight-green); /* Green 70 */
  --string-color: var(--theme-highlight-red); /* Magenta 65 */
}

We may want to unify those two color sets at one point. I guess one reason it wasn't done is that there's a lot of technical debt to muddle through.

In the short term I'd go for grays in eager evaluation.

[fvsch]

@violasong @digitarald We need your help for a UI decision. :)

We can make the height of the Console's input and instant evaluation result match the height of console messages, so that when you hit Enter the transition will be seamless in many cases.
The downside is that it makes the input area more cramped, a bit like Chrome does.

I have a working implementation and also prototyped adding some padding back, and recorded a few videos:

console-input-height-20px-screen-captures.zip

Can you take a look (ideally at a zoom level that looks natural) and see how it feels?
Which way do we want to go?

[digitarald]

@fvsch I kinda prefer the seamless transition, assuming that better layout stability improves usability more than whitespace.

[violasong]

Nice! I'm interested in a seamless and more top-padded solution :)

Nicolas mentioned in the phab:

could we keep some top padding and reduce the space between the input and the instant result element instead?

And I'm wondering if we can also reduce the space between the two rows in the messages to match. I.e. the first of the rows could be moved down 2px. This seems like better alignment to me anyway.

So instead of this:

This: (super rough modified screenshot)

[fvsch]

Nice! I'm interested in a seamless and more top-padded solution :)

Not sure we can have our cake and eat it too. :D

Impact on message height

To be seamless, the input area has to have the exact same metrics as the messages.

Currently messages are structured like this:

-----------------------
3px top padding
14px line-height (text)
3px bottom padding
-----------------------

So they're 20px tall (excluding borders between messages).
If we want to increase the padding by 2px, making it a 5px padding, our messages would be 24px tall. Are we comfortable with that? We can also use 4px padding -> 22px tall messages.

Bringing input and output closer together

When we have an input message and and output message following each other, we remove the top border of the output message. We could bring it a bit higher by 2-3 pixels like in your screenshot, both in the list of messages and for the space between input and the eager evaluation result.

I'll prototype it in code and make screenshots.

[violasong]

Bringing input and output closer together

Ah yes! Sorry I wasn't more clear - I do want to keep the same 20px height for both, but bring the top message of each UI lower by 2px.

[violasong]

(Even though there's an equal amount of padding above and below the dual messages, it currently looks like there's more whitespace below. So this would hopefully make it more vertically centered as well)

[fvsch]

I do want to keep the same 20px height for both, but bring the top message of each UI lower by 2px.

I see. I don't think we can do that because:

• a single message with top and bottom border would look unbalanced (5px padding top, 1px padding bottom)
• on the CSS side we don't know if an "input" message is part of an "input+result" group or if it appears on its own, we only know about the second message (with a CSS selector like .message.input + .message.result { border-top: 0; })

[violasong]

Ah, I see! I played with this a bit (just looking at the logs) and your previous comment about adding margin-top: -3px to .message.input + .message.result does look nice.

I think we could also do padding-top: 4px; padding-bottom: 2px; on all messages to take the text down 1px. To my eye, 1px more at the top looks better-centered than 1px more on the bottom. Icons would need margin-top: -1px;

/* webconsole.css | chrome://devtools/skin/webconsole.css */

:root {
  /* --console-output-vertical-padding: 3px; */
}

.message {
  padding-top: 4px;
  padding-bottom: 2px;
}

.message.command + .result.log {
  margin-top: -3px;
}

.message > .indent[data-indent="0"] + .icon {
  margin-top: -1px;
}

[fvsch]

I think we could also do padding-top: 4px; padding-bottom: 2px; on all messages to take the text down 1px. To my eye, 1px more at the top looks better-centered than 1px more on the bottom.

Hmm the current symmetrical padding looks better to my eyes (on macOS).

Also worth noting that this depends strongly on the OS and font, and where the font places its baseline relative to its line-height. For instance, on Windows the system font we use tends to have a low baseline.

In my experience when we try to balance a line of text's look with padding-top: N+1; padding-bottom: N-1; (or the other way round), it breaks quickly on a different OS, font, or resolution.

[violasong]

Sounds fair - I'll go with your instinct on this. Thanks for the extra considerations!

[fvsch]

@violasong @digitarald In the latest Nightly we landed some CSS fixes, including:

• The spacing around the input and instant eval result is now similar when Instant Evaluation is on or off.
• To be conservative, we used 2px of extra padding on the input/instant eval, so we're not seamless (for now).

We can live with that for a few weeks, and figure out if we want to tweak it later.

If you want to test the seamless-and-compact look you can open the Browser Toolbox, inspect the console, and set this variable to zero:

:root {
  --console-input-extra-padding: 2px;
}

If we want to move to a seamless look that is a tiny bit less cramped, one option is to make rows a tad higher:

• Make the console messages 22px instead of 20px (1px of extra padding)
• For .message.command + .message.result pairs, bring the result a couple pixels closer to the command.
• Align the input+eager eval result layout to these metrics

Which looks like:

(IMO it looks alright but maybe a bit uneven.)

Or we could have a "compact" mode with 20px (or even 18px?) rows, and a "large/normal" mode with 24px rows. :D

[violasong]

I'm noticing an issue that's distracting from the seams vs no seams question (I actually don't see a seam in Nightly) - right when I hit enter on a string, there's a bit of a flashing effect in the eval/output line. (Like maybe it disappears for a split second? Or... the eval isn't disappearing fast enough?) Definitely a very minor issue (and Chrome's flashes too :D).

I like your screenshot but mainly because I just like those input/output lines being a little closer together relatively. I don't know if we actually need the extra height.

[digitarald]

right when I hit enter on a string, there's a bit of a flashing effect in the eval/output line. (Like maybe it disappears for a split second? Or... the eval isn't disappearing fast enough?)

I also noticed that. There seems few frames delay until the reps component renders. Maybe the output could use the existing instant eval result to render optimistically. I filed bug 1618971 to discuss.

[violasong]

Closing this issue, as I think any remaining tweaks are all in bugzilla. Congrats all on the release!