Closed
Bug 1044050
Opened 10 years ago
Closed 10 years ago
Triangle icons in the console look like expandos, but aren't
Categories
(DevTools :: Console, defect, P1)
DevTools
Console
Tracking
(firefox41 verified)
VERIFIED
FIXED
Firefox 41
Tracking | Status | |
---|---|---|
firefox41 | --- | verified |
People
(Reporter: fitzgen, Assigned: bgrins)
References
Details
(Whiteboard: [polish-backlog][difficulty=easy][bugday-20150805])
Attachments
(6 files, 8 obsolete files)
(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.
Reporter | ||
Comment 1•10 years ago
|
||
Screenshot of triangles
Reporter | ||
Comment 2•10 years ago
|
||
What if the commands had the ">>" icon, and return values didn't have any icon/glyph?
+==============================================================+
| >> 40 + 2 |
| 42 |
| >> obj = { ... } |
| Object { ... } |
| |
+--------------------------------------------------------------+
| >> |
+==============================================================+
Assignee | ||
Comment 3•10 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; }
Reporter | ||
Comment 4•10 years ago
|
||
(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•10 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.
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 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)
Updated•10 years ago
|
Flags: needinfo?(dhenein) → needinfo?(shorlander)
Comment 8•10 years ago
|
||
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•10 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)
Comment 10•10 years ago
|
||
Flags: needinfo?(shorlander)
Comment 11•10 years ago
|
||
Comment 12•10 years ago
|
||
Comment 13•10 years ago
|
||
Comment 14•10 years ago
|
||
I will make a patch for this. Thanks Stephen
Updated•10 years ago
|
Assignee: nobody → gioyik
Comment 15•10 years ago
|
||
How they would look in context.
Assignee | ||
Comment 16•10 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)
Comment 17•10 years ago
|
||
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)
Comment 18•10 years ago
|
||
Assignee | ||
Comment 19•10 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•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Attachment #8570654 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8570656 -
Attachment is obsolete: true
Comment 20•10 years ago
|
||
Sure, not problem. I will do.
I keep the ni to not forget it.
Updated•10 years ago
|
Whiteboard: [devedition-40][difficulty=medium]
Assignee | ||
Updated•10 years ago
|
Whiteboard: [devedition-40][difficulty=medium] → [devedition-40][difficulty=easy]
Comment 22•10 years ago
|
||
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•10 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•10 years ago
|
||
I'll wrap this one up
Assignee: gioyik → bgrinstead
Flags: needinfo?(gioyik)
Assignee | ||
Comment 25•10 years ago
|
||
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•10 years ago
|
Summary: Triangles look like expandos, but aren't → Triangle icons in the console look like expandos, but aren't
Comment 26•10 years ago
|
||
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•10 years ago
|
||
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
Assignee | ||
Comment 29•10 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•10 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?
Comment 31•10 years ago
|
||
(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•10 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•10 years ago
|
||
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 34•10 years ago
|
||
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•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 35•10 years ago
|
||
Keywords: checkin-needed
Assignee | ||
Comment 36•10 years ago
|
||
Assignee | ||
Comment 37•10 years ago
|
||
Comment 38•10 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #37)
> Created attachment 8620637 [details]
> screenshot-after-patch.png
Looks great!
Assignee | ||
Updated•10 years ago
|
Whiteboard: [devedition-40][difficulty=easy] → [fixed-in-fx-team][devedition-40][difficulty=easy]
Comment 39•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][devedition-40][difficulty=easy] → [devedition-40][difficulty=easy]
Target Milestone: --- → Firefox 41
Updated•9 years ago
|
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Comment 40•9 years ago
|
||
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•9 years ago
|
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•