1044050 - Triangle icons in the console look like expandos, but aren't

Status: VERIFIED FIXED

(DevTools :: Console, defect, P1)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(We were talking about this on irc yesterday.)

The triangles showing what is evaluated and returned in the console look like they should be expandable (like the profiler's tree/table or something) but they aren't. People unfamiliar with our console have been getting confused by this, saying that our console doesn't expand objects, when in reality it does and they need to click the result to have it open in the sidebar.

One possible solution would be to change them into unicode arrows: ← and →

Would also be interested in possible solutions that result in less glyphs in the console, so it doesn't look so noisy.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Screen Shot 2014-07-25 at 8.59.16 AM.png

Screenshot of triangles

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

What if the commands had the ">>" icon, and return values didn't have any icon/glyph?

+==============================================================+
| >> 40 + 2                                                    |
| 42                                                           |
| >> obj = { ... }                                             |
| Object { ... }                                               |
|                                                              |
+--------------------------------------------------------------+
| >>                                                           |
+==============================================================+

Comment 3

[Brian Grinstead [:bgrins]]

Occasionally output will have icons anyway:

>> foo
X Reference Error: foo is not defined.

But for a normal output I'm fine with trying out no icon on the left.

Changing this would simply require changing the left pointing arrow in these icons to look more like the ">>" prompt: 

https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/images/webconsole.png
https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/images/[e-mail]

We could probably remove the right pointing arrow from that image, then add this CSS:

.message[category="output"] > .icon { visibility: hidden; } 

Or if we want it to line up flush to the left like in Comment 2, then remove

.message[category="output"] > .icon { display: none; }

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Brian Grinstead [:bgrins] from comment #3)
> Occasionally output will have icons anyway:
> 
> >> foo
> X Reference Error: foo is not defined.
> 
> But for a normal output I'm fine with trying out no icon on the left.

Yes, was only suggesting this for normal logs (not warns, errors) and return values. Thanks for clarifying.

> We could probably remove the right pointing arrow from that image, then add
> this CSS:
> 
> .message[category="output"] > .icon { visibility: hidden; } 
> 
> Or if we want it to line up flush to the left like in Comment 2, then remove
> 
> .message[category="output"] > .icon { display: none; }

Or, you know, not put the icon in the dom ;)

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Nick Fitzgerald [:fitzgen] from comment #4)
> (In reply to Brian Grinstead [:bgrins] from comment #3)
> > Occasionally output will have icons anyway:
> > 
> > >> foo
> > X Reference Error: foo is not defined.
> > 
> > But for a normal output I'm fine with trying out no icon on the left.
> 
> Yes, was only suggesting this for normal logs (not warns, errors) and return
> values. Thanks for clarifying.
> 
> > We could probably remove the right pointing arrow from that image, then add
> > this CSS:
> > 
> > .message[category="output"] > .icon { visibility: hidden; } 
> > 
> > Or if we want it to line up flush to the left like in Comment 2, then remove
> > 
> > .message[category="output"] > .icon { display: none; }
> 
> Or, you know, not put the icon in the dom ;)

In my opinion it would be best leave it in the DOM with visibility hidden so that there is still a nice strong vertical line when scanning output.

Comment 6

[Jeff Griffiths (:canuckistani) (:⚡︎)]

See also: https://bugzilla.mozilla.org/show_bug.cgi?id=755534

Comment 7

[Brian Grinstead [:bgrins]]

Darrin, any chance you could modify the console icon image to replace the grey left arrow with a grey version of the console prompt: ">>", and remove the right facing arrow?

https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/images/webconsole.png
https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/images/[e-mail]

Comment 8

[Giovanny Gongora [:gioyik]]

This seems to be a good candidate to be a good-first-bug. I would want to take it if is possible but is necessary the modified image. What you think?

Comment 9

[Brian Grinstead [:bgrins]]

(In reply to Giovanny Gongora [:gioyik] from comment #8)
> This seems to be a good candidate to be a good-first-bug. I would want to
> take it if is possible but is necessary the modified image. What you think?

I've talked with shorlander about this - once we have updated assets this would be a good bug to work on.

Comment 10

[Stephen Horlander [:shorlander]]

Attachment: icon-indicator-input.svg

Comment 11

[Stephen Horlander [:shorlander]]

Attachment: icon-indicator-input-lightTheme.svg

Comment 12

[Stephen Horlander [:shorlander]]

Attachment: icon-indicator-output.svg

Comment 13

[Stephen Horlander [:shorlander]]

Attachment: icon-indicator-output-lightTheme.svg

Comment 14

[Giovanny Gongora [:gioyik]]

I will make a patch for this. Thanks Stephen

Comment 15

[Stephen Horlander [:shorlander]]

Attachment: Updated Icons in Context - 01

How they would look in context.

Comment 16

[Brian Grinstead [:bgrins]]

(In reply to Giovanny Gongora [:gioyik] from comment #14)
> I will make a patch for this. Thanks Stephen

I know you'd be able to make it work using the separate icons, but I don't think it's consistent with how the rest of the console UI is working.  Stephen, would it be possible to update the webconsole.png/[e-mail] sprites with these new icons so that we don't have to special case this?

If we have an updated sprite, then we will just need to update the assets and then change the rule in webconsole.inc.css [1] to allow for light and dark themes.

https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/webconsole.inc.css#46

Comment 17

[Stephen Horlander [:shorlander]]

Attachment: icon-devTools-webconsole.svg

I updated the sprite sheet with the new icons and converted it to SVG. I also had to make the icon dimensions larger, so it went from 8x8 to 12x12.

Comment 18

[Stephen Horlander [:shorlander]]

Attachment: icon-devTools-webconsole-lightTheme.svg

Comment 19

[Brian Grinstead [:bgrins]]

Thanks Stephen.  Giovanny, will you have a chance to take a shot at this with the new assets?  We can remove the [e-mail] now that we have SVG icons attached here, and just add in a selector to change the image used in the light theme (as in Comment 16).

Comment 20

[Giovanny Gongora [:gioyik]]

Sure, not problem. I will do.

I keep the ni to not forget it.

Comment 21

[Jeff Griffiths (:canuckistani) (:⚡︎)]

The new icons look great, let's ship them :) P1.

Comment 22

[Giovanny Gongora [:gioyik]]

Sorry, I forget it. I had the patch but forget attach it. I am going to rebase it and put it for review.

Comment 23

[Brian Grinstead [:bgrins]]

(In reply to Giovanny Gongora [:gioyik] from comment #22)
> Sorry, I forget it. I had the patch but forget attach it. I am going to
> rebase it and put it for review.

Hi Giovanny, any chance you can attach a patch for this, or should we have someone else take it on?

Comment 24

[Brian Grinstead [:bgrins]]

I'll wrap this one up

Comment 25

[Brian Grinstead [:bgrins]]

Attachment: console-icons.patch

This was a bit more work than I thought it would be at first.  Since the dimensions for each icon changed from 8px to 12px I had to multiply each positioning value by 1.5 to make it look right.  Tested on hidpi on osx and 1x on win7 and it looks OK to me.

Comment 26

[Tim Nguyen :ntim]

Comment on attachment 8617605 [details] [diff] [review]
console-icons.patch

Review of attachment 8617605 [details] [diff] [review]:
-----------------------------------------------------------------

Lots of comments about the SVG, applies to webconsole-light.svg as well.

::: browser/themes/shared/devtools/images/webconsole-light.svg
@@ +34,5 @@
> +      .icon-colorSwatch-network { fill: #000; }
> +      .icon-colorSwatch-css { fill: #00b6f0; }
> +      .icon-colorSwatch-js { fill: #fb9500; }
> +      .icon-colorSwatch-logging { fill: #808080; }
> +      .icon-colorSwatch-security { fill: #ec1e0d; }

These colors are the same as webconsole.svg (only the input/output icons have different colors).

Can this be possibly merged with the other file ? 
Probably with some kind of symlink around the "special" icons :

<style>
#icon-indicator-input {
  fill: #8fa1b2;
}
#icon-indicator-output {
  fill="#667380;
}
#light-icons:target #icon-indicator-input {
  fill: #45494d;
}
#light-icons:target #icon-indicator-output {
  fill: #8a9199;
}
</style>
...
<g id="light-icons">
  <path id="icon-indicator-input" d="M6.5,1.2L5.4,2.3L9,6L5.3,9.7l1.1,1.1L11,6L6.5,1.2z M1.5,1.2 L0.4,2.3L4,6L0.3,9.7l1.1,1.1L6,6L1.5,1.2z" transform="translate(48 36)"/>
  <polygon id="icon-indicator-output" points="10,5 4.3,5 6.8,2.4 5.5,1.2 1,6 5.5,10.8 6.9,9.6 4.3,7 10,7" transform="translate(60 36)"/>
</g>

::: browser/themes/shared/devtools/images/webconsole.svg
@@ +2,5 @@
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
> +

nit : Please remove the doctype and the blank line

@@ +4,5 @@
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
> +
> +<svg version="1.1"
> +     id="icon-devTools-webconsole"

nit : id and version can be removed

@@ +8,5 @@
> +     id="icon-devTools-webconsole"
> +     xmlns="http://www.w3.org/2000/svg"
> +     xmlns:xlink="http://www.w3.org/1999/xlink"
> +     x="0"
> +     y="0"

x=0 and y=0 can be removed (no effect)

@@ +12,5 @@
> +     y="0"
> +     width="72"
> +     height="60"
> +     viewBox="0 0 72 60">
> +

nit : useless blank line

@@ +16,5 @@
> +
> +  <defs>
> +    <rect id="glyphShape-colorSwatch" width="8" height="8" ry="2" rx="2" />
> +    <rect id="glyphShape-colorSwatch-border" width="10" height="10" ry="2" rx="2" />
> +

nit : useless blank line

@@ +18,5 @@
> +    <rect id="glyphShape-colorSwatch" width="8" height="8" ry="2" rx="2" />
> +    <rect id="glyphShape-colorSwatch-border" width="10" height="10" ry="2" rx="2" />
> +
> +    <polygon id="glyphShape-errorX" points="9.9,8.5 8.5,9.9 6,7.4 3.6,9.8 2.2,8.4 4.6,6 2.2,3.6 3.6,2.2 6,4.6 8.4,2.2 9.8,3.6 7.4,6 " />
> +

nit : useless blank line

@@ +21,5 @@
> +    <polygon id="glyphShape-errorX" points="9.9,8.5 8.5,9.9 6,7.4 3.6,9.8 2.2,8.4 4.6,6 2.2,3.6 3.6,2.2 6,4.6 8.4,2.2 9.8,3.6 7.4,6 " />
> +
> +    <path id="glyphShape-warningTriangle" d="M9.9,8.6l-3.1-6C6.6,2.2,6.3,2,6,2C5.7,2,5.4,2.2,5.2,2.5l-3.1,6C2,8.9,2,9.3,2.1,9.6C2.3,9.8,2.6,10,2.9,10 h6.1c0.4,0,0.6-0.2,0.8-0.4C10,9.3,10,8.9,9.9,8.6z" />
> +    <path id="glyphShape-exclamationPoint" d="M6,7.7c-0.6,0-1,0.4-1,0.8C5,9,5.4,9.3,6,9.3c0.6,0,1-0.4,1-0.8 C7,8.1,6.6,7.7,6,7.7z M6,7c0.6,0,1-0.4,1-1V5c0-0.6-0.4-1-1-1S5,4.4,5,5v1C5,6.6,5.4,7,6,7z" />
> +

nit : useless blank line

@@ +24,5 @@
> +    <path id="glyphShape-exclamationPoint" d="M6,7.7c-0.6,0-1,0.4-1,0.8C5,9,5.4,9.3,6,9.3c0.6,0,1-0.4,1-0.8 C7,8.1,6.6,7.7,6,7.7z M6,7c0.6,0,1-0.4,1-1V5c0-0.6-0.4-1-1-1S5,4.4,5,5v1C5,6.6,5.4,7,6,7z" />
> +
> +    <circle id="glyphShape-infoCircle" cx="6" cy="6" r="4" />
> +    <path id="glyphShape-infoGlyph" d="M6,6C5.4,6,5,6.4,5,7v1c0,0.6,0.4,1,1,1s1-0.4,1-1V7C7,6.4,6.6,6,6,6z M6,5c0.6,0,1-0.4,1-1S6.6,3,6,3S5,3.4,5,4S5.4,5,6,5z" />
> +

nit : useless blank line

@@ +25,5 @@
> +
> +    <circle id="glyphShape-infoCircle" cx="6" cy="6" r="4" />
> +    <path id="glyphShape-infoGlyph" d="M6,6C5.4,6,5,6.4,5,7v1c0,0.6,0.4,1,1,1s1-0.4,1-1V7C7,6.4,6.6,6,6,6z M6,5c0.6,0,1-0.4,1-1S6.6,3,6,3S5,3.4,5,4S5.4,5,6,5z" />
> +
> +    <style type="text/css">

nit : type attribute not needed

@@ +27,5 @@
> +    <path id="glyphShape-infoGlyph" d="M6,6C5.4,6,5,6.4,5,7v1c0,0.6,0.4,1,1,1s1-0.4,1-1V7C7,6.4,6.6,6,6,6z M6,5c0.6,0,1-0.4,1-1S6.6,3,6,3S5,3.4,5,4S5.4,5,6,5z" />
> +
> +    <style type="text/css">
> +    <![CDATA[
> +

nit : please remove the CDATA line and the blank line

@@ +29,5 @@
> +    <style type="text/css">
> +    <![CDATA[
> +
> +      .icon-colorSwatch-border { fill: #ffffff; fill-opacity: .7; }
> +

nit : please remove this blank line, shorthex would be better as well for the previous line

@@ +38,5 @@
> +      .icon-colorSwatch-security { fill: #ec1e0d; }
> +
> +      .icon-glyphOverlay { fill: #fff; }
> +
> +    ]]>

nit : Please remove these 2 last lines

@@ +42,5 @@
> +    ]]>
> +    </style>
> +
> +  </defs>
> +

nit : Please remove the blank lines

@@ +94,5 @@
> +  </g>
> +
> +  <path id="icon-indicator-input" fill="#8fa1b2" d="M6.5,1.2L5.4,2.3L9,6L5.3,9.7l1.1,1.1L11,6L6.5,1.2z M1.5,1.2 L0.4,2.3L4,6L0.3,9.7l1.1,1.1L6,6L1.5,1.2z" transform="translate(48 36)" />
> +  <polygon id="icon-indicator-output" fill="#667380" points="10,5 4.3,5 6.8,2.4 5.5,1.2 1,6 5.5,10.8 6.9,9.6 4.3,7 10,7 "  transform="translate(60 36)" />
> +

nit : useless blank line

Comment 27

[Tim Nguyen :ntim]

Attachment: webconsole.svg

Here's an updated SVG that combines both SVG, and cleans them up.

To access dark theme icons :
chrome://browser/skin/devtools/webconsole.svg

To access light theme icons :
chrome://browser/skin/devtools/webconsole.svg#light-icons

Comment 28

[Tim Nguyen :ntim]

Attachment: webconsole.svg

A bit more cleanup

Comment 29

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] from comment #28)
> Created attachment 8618847 [details]
> webconsole.svg
> 
> A bit more cleanup

Is there a tool that I can run these through in the future to make sure that SVGs are appropriately cleaned up?  Or at least some documentation about with some guidelines?

Comment 30

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] from comment #28)
> Created attachment 8618847 [details]
> webconsole.svg
> 
> A bit more cleanup

The license was removed from this, is that intentional?

Comment 31

[Tim Nguyen :ntim]

(In reply to Brian Grinstead [:bgrins] from comment #29)
> (In reply to Tim Nguyen [:ntim] from comment #28)
> > Created attachment 8618847 [details]
> > webconsole.svg
> > 
> > A bit more cleanup
> 
> Is there a tool that I can run these through in the future to make sure that
> SVGs are appropriately cleaned up?  Or at least some documentation about
> with some guidelines?

None yet. Still need to post to the mailing list about this.

(In reply to Brian Grinstead [:bgrins] from comment #30)
> (In reply to Tim Nguyen [:ntim] from comment #28)
> > Created attachment 8618847 [details]
> > webconsole.svg
> > 
> > A bit more cleanup
> 
> The license was removed from this, is that intentional?

Nope.

Comment 32

[Brian Grinstead [:bgrins]]

(In reply to Tim Nguyen [:ntim] from comment #31)
> (In reply to Brian Grinstead [:bgrins] from comment #30)
> > The license was removed from this, is that intentional?
> 
> Nope.

OK, I'll re-add it

Comment 33

[Brian Grinstead [:bgrins]]

Attachment: console-icons.patch

Just updated the svg file (merged the two into a single file that can be toggled via the #light-icons fragment).  Here's a try push, just in case: https://treeherder.mozilla.org/#/jobs?repo=try&revision=35b4d310628a

Comment 34

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8620374 [details] [diff] [review]
console-icons.patch

Review of attachment 8620374 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good on Linux, too!

Comment 35

[Brian Grinstead [:bgrins]]

remote:   https://hg.mozilla.org/integration/fx-team/rev/4c380bfa5e3c

Comment 36

[Brian Grinstead [:bgrins]]

Attachment: screenshot-before-patch.png

Comment 37

[Brian Grinstead [:bgrins]]

Attachment: screenshot-after-patch.png

Comment 38

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Brian Grinstead [:bgrins] from comment #37)
> Created attachment 8620637 [details]
> screenshot-after-patch.png

Looks great!

Comment 39

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4c380bfa5e3c

Comment 40

[Nazir Ahmed Sabbir [:NaSb]]

Successfully reproduce this bug on Aurora 33.0a2 (2014-07-25)

This Bug is now verified as fixed on Latest Aurora 41.0a2 (2015-08-05)

Build ID: 20150805004014
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0