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

VERIFIED FIXED in Firefox 41

Status

P1
normal
VERIFIED FIXED
4 years ago
5 months ago

People

(Reporter: fitzgen, Assigned: bgrins)

Tracking

unspecified
Firefox 41

Firefox Tracking Flags

(firefox41 verified)

Details

(Whiteboard: [polish-backlog][difficulty=easy][bugday-20150805])

Attachments

(6 attachments, 8 obsolete attachments)

13.37 KB, image/png
Details
11.05 KB, image/png
Details
5.16 KB, image/svg+xml
Details
20.04 KB, patch
past
: review+
Details | Diff | Splinter Review
30.47 KB, image/png
Details
32.81 KB, image/png
Details
(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.
Created attachment 8462638 [details]
Screen Shot 2014-07-25 at 8.59.16 AM.png

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

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

Comment 3

4 years ago
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/webconsole@2x.png

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; }
(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 ;)
(Assignee)

Comment 5

4 years ago
(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.
(Assignee)

Comment 7

4 years ago
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/webconsole@2x.png
Flags: needinfo?(dhenein)
Flags: needinfo?(dhenein) → needinfo?(shorlander)
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?
Flags: needinfo?(bgrinstead)
(Assignee)

Comment 9

4 years ago
(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.
Flags: needinfo?(bgrinstead)
Created attachment 8570653 [details]
icon-indicator-input.svg
Flags: needinfo?(shorlander)
Created attachment 8570654 [details]
icon-indicator-input-lightTheme.svg
Created attachment 8570655 [details]
icon-indicator-output.svg
Created attachment 8570656 [details]
icon-indicator-output-lightTheme.svg
I will make a patch for this. Thanks Stephen
Assignee: nobody → gioyik
Created attachment 8570658 [details]
Updated Icons in Context - 01

How they would look in context.
(Assignee)

Comment 16

4 years ago
(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/webconsole@2x.png 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
Flags: needinfo?(shorlander)
Created attachment 8571623 [details]
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.
Attachment #8570653 - Attachment is obsolete: true
Attachment #8570655 - Attachment is obsolete: true
Flags: needinfo?(shorlander)
Created attachment 8571624 [details]
icon-devTools-webconsole-lightTheme.svg
(Assignee)

Comment 19

4 years ago
Thanks Stephen.  Giovanny, will you have a chance to take a shot at this with the new assets?  We can remove the webconsole@2px.png 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).
Flags: needinfo?(gioyik)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
Attachment #8570654 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8570656 - Attachment is obsolete: true
Sure, not problem. I will do.

I keep the ni to not forget it.
Whiteboard: [devedition-40][difficulty=medium]
(Assignee)

Updated

4 years ago
Whiteboard: [devedition-40][difficulty=medium] → [devedition-40][difficulty=easy]
The new icons look great, let's ship them :) P1.
Priority: -- → P1
Sorry, I forget it. I had the patch but forget attach it. I am going to rebase it and put it for review.
(Assignee)

Comment 23

4 years ago
(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?
(Assignee)

Comment 24

4 years ago
I'll wrap this one up
Assignee: gioyik → bgrinstead
Flags: needinfo?(gioyik)
(Assignee)

Comment 25

4 years ago
Created attachment 8617605 [details] [diff] [review]
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.
Attachment #8617605 - Flags: review?(past)
(Assignee)

Updated

4 years ago
Summary: Triangles look like expandos, but aren't → Triangle icons in the console look like expandos, but aren't
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
Created attachment 8618295 [details]
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
Attachment #8571623 - Attachment is obsolete: true
Attachment #8571624 - Attachment is obsolete: true
Created attachment 8618847 [details]
webconsole.svg

A bit more cleanup
Attachment #8618295 - Attachment is obsolete: true
(Assignee)

Comment 29

4 years ago
(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?
(Assignee)

Comment 30

4 years ago
(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?
(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.
(Assignee)

Comment 32

4 years ago
(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
(Assignee)

Comment 33

4 years ago
Created attachment 8620374 [details] [diff] [review]
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
Attachment #8617605 - Attachment is obsolete: true
Attachment #8617605 - Flags: review?(past)
Attachment #8620374 - Flags: review?(past)
Comment on attachment 8620374 [details] [diff] [review]
console-icons.patch

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

Looks good on Linux, too!
Attachment #8620374 - Flags: review?(past) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
(Assignee)

Comment 36

4 years ago
Created attachment 8620636 [details]
screenshot-before-patch.png
(Assignee)

Comment 37

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

Looks great!
(Assignee)

Updated

4 years ago
Whiteboard: [devedition-40][difficulty=easy] → [fixed-in-fx-team][devedition-40][difficulty=easy]
https://hg.mozilla.org/mozilla-central/rev/4c380bfa5e3c
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40][difficulty=easy] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 41
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
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
QA Whiteboard: [bugday-20150805]
Whiteboard: [polish-backlog][difficulty=easy] → [polish-backlog][difficulty=easy][bugday-20150805]
(Assignee)

Updated

3 years ago
Status: RESOLVED → VERIFIED
status-firefox41: fixed → verified

Updated

5 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.