1307929 - Photon updates for new webconsole UI

Status: VERIFIED FIXED

(DevTools :: Console, enhancement, P1)

Description

[Lin Clark [:linclark]]

Originally posted by:jnachtigall

see https://github.com/devtools-html/gecko-dev/issues/350

Dupe of https://bugzilla.mozilla.org/show_bug.cgi?id=879310 I would like to raise awareness about. 

I think the line items would be much easier to read if they had alternating background colors like usually done for table rows with CSS `:nth-child(even)` or `:nth-child(odd)`. Especially if you have line items that stretch more than one line (which happens often when the devtools are attached at the right side and not at bottom).

Comment 1

[Julian Descottes [:jdescottes]]

Adding here a comment from Nicolas left on github (and updating summary):
===================================================

I don't really think zebra would be something we want for this, as it would be too "loud". But we can definitely experiments with things that would improve the readibility ( like more vertical padding around the message, separation lines on each message, ... ).
That's a job for @helenvholmes 

Comment 2

[Jens]

Attachment: Colors_Old_vs_new_Console.png

I've just hit this again when *not* seeing failed CORs. See attached screenshot. At least I think that the warning icon should be red so that it is better visible especially if CSS and XHR filters are enabled, then it's too much grey in grey and lines hard to differentiate.

When doing a re-coloring here, please also have a look at the Google Console which has a nice color appeal for specific lines and different log levels. So Error, Warning, Info lines have different color making it really nice. 

I guess there's many more 'tail' or logging like software out there to get inspired from about how to do the colors. Still room to improve :)

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

See https://docs.google.com/document/d/1AJ7oHC7akXJiliheHjhFc4fTonaN31Eo_P2M8gssL8s/edit#heading=h.o8ous6wzxnb5

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1307929 - Photon updates for new webconsole UI; .

This patch does not change the color for Reps, which is done in
Github and will be available in the 0.0.13 Reps bundle.

Review commit: https://reviewboard.mozilla.org/r/179670/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/179670/

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8907995 [details]
Bug 1307929 - Photon updates for new webconsole UI; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179670/diff/1-2/

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8907995 [details]
Bug 1307929 - Photon updates for new webconsole UI; .

https://reviewboard.mozilla.org/r/179670/#review184958

::: devtools/client/themes/webconsole.css:725
(Diff revision 2)
>    -moz-user-focus: normal;
> +  color: var(--console-output-color);
> +  --console-output-indent-width: 1rem;
> +  --console-output-indent-border-color: #A2D1FF;
> +}
> +/* Webconsole specific theme variables */

Needs a new line to separate the rules

Comment 7

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8907995 [details]
Bug 1307929 - Photon updates for new webconsole UI; .

https://reviewboard.mozilla.org/r/179670/#review184982

R+ from me but some feedback.

1) Errors have the same red color as standard logs. I think that errors should be easily distnguishable from logs, so it isn't easy to overlook them. Isn't red color being de facto standared for errors? 
2) CSS errors are not red.
3) Errors in the dark theme are not red (while standard logs are).

Otherwise, I really like how the UI has been refreshed and how it looks now!

Honza

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Jan Honza Odvarko [:Honza] from comment #7)
> Comment on attachment 8907995 [details]
> Bug 1307929 - Photon updates for new webconsole UI; .
> 
> https://reviewboard.mozilla.org/r/179670/#review184982
> 
> R+ from me but some feedback.
> 
> 1) Errors have the same red color as standard logs. I think that errors
> should be easily distnguishable from logs, so it isn't easy to overlook
> them. Isn't red color being de facto standared for errors? 
> 2) CSS errors are not red.
> 3) Errors in the dark theme are not red (while standard logs are).
> 
> Otherwise, I really like how the UI has been refreshed and how it looks now!
> 
> Honza
To be clear, my feedback is more for designers behind
the photon colors, not so much for Nicolas ;-)

Honza

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8907995 [details]
Bug 1307929 - Photon updates for new webconsole UI; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179670/diff/2-3/

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8907995 [details]
Bug 1307929 - Photon updates for new webconsole UI; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/179670/diff/3-4/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea1956ea8a50
Photon updates for new webconsole UI; r=gl,Honza.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ea1956ea8a50

Comment 13

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Investigated this issue on 57.0b3 build 1 (20170925150345), using Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6. I noticed some inconsistencies between the spec doc details provided in comment 3 and the font colors and some symbols (e.g. Errors) used in the above mentioned 57 build for Errors, Warnings, CSS, XHR and Requests. See here the screenshots for each case https://goo.gl/i8wGKL.

Comment 14

[Nicolas Chevobbe [:nchevobbe]]

Hello Iulia,

We tweaked the design a bit while doing this work, that's why you see differences.
I went through your screenshots and everything seems good to me :)

Thanks !

Comment 15

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

Thanks you Nicolas for your reply! 
Based on comment 13 and comment 14, I will mark this as verified fixed.