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)

defect

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.
Screenshot of triangles
What if the commands had the ">>" icon, and return values didn't have any icon/glyph? +==============================================================+ | >> 40 + 2 | | 42 | | >> obj = { ... } | | Object { ... } | | | +--------------------------------------------------------------+ | >> | +==============================================================+
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 ;)
(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.
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)
(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)
Attached image icon-indicator-input.svg (obsolete) —
Flags: needinfo?(shorlander)
Attached image icon-indicator-output.svg (obsolete) —
I will make a patch for this. Thanks Stephen
Assignee: nobody → gioyik
How they would look in context.
(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)
Attached image icon-devTools-webconsole.svg (obsolete) —
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)
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)
Status: NEW → ASSIGNED
Attachment #8570654 - Attachment is obsolete: true
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]
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.
(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?
I'll wrap this one up
Assignee: gioyik → bgrinstead
Flags: needinfo?(gioyik)
Attached patch console-icons.patch (obsolete) — Splinter Review
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)
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
Attached image webconsole.svg (obsolete) —
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
Attached image webconsole.svg
A bit more cleanup
Attachment #8618295 - Attachment is obsolete: true
(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?
(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.
(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
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+
Keywords: checkin-needed
(In reply to Brian Grinstead [:bgrins] from comment #37) > Created attachment 8620637 [details] > screenshot-after-patch.png Looks great!
Whiteboard: [devedition-40][difficulty=easy] → [fixed-in-fx-team][devedition-40][difficulty=easy]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
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]
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: