Bug 100302

Summary: Web Inspector: Console: vertically align icons with text
Product: WebKit Reporter: Nikita Vasilyev <me>
Component: Web Inspector (Deprecated)Assignee: Nikita Vasilyev <me>
Status: RESOLVED WONTFIX    
Severity: Normal CC: apavlov, dubroy, keishi, loislo, mkwst, mmatyas, noam, ossy, pfeldman, pmuellr, vsevik, web-inspector-bugs, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 116924    
Attachments:
Description Flags
Hover bug
none
Mockup (before/after)
none
Patch
none
Mockup. Align icons with text and increase padding between messages by 2px
none
Patch. Align icons with text and increase padding between messages by 2px
none
Mockup. Align icons with text and increase padding between messages by 2px
none
Patch
none
Updated patch ossy: review-, ossy: commit-queue-

Nikita Vasilyev
Reported 2012-10-24 16:09:56 PDT
Created attachment 170500 [details] Hover bug Key points: — Add subtle background color for console.error and console.warn to make it more apparent — Properly align icons with text — Add subtle gradient for group titles — Use non-monospace font for source links to make them more readable and more distinguishable from the code — Remove gray background on mouse over (introduced by https://bugs.webkit.org/show_bug.cgi?id=98701). It overlays background of elements; see attached hover-bug.png
Attachments
Hover bug (9.87 KB, image/png)
2012-10-24 16:09 PDT, Nikita Vasilyev
no flags
Mockup (before/after) (128.27 KB, image/png)
2012-10-24 16:10 PDT, Nikita Vasilyev
no flags
Patch (6.77 KB, patch)
2012-10-24 16:17 PDT, Nikita Vasilyev
no flags
Mockup. Align icons with text and increase padding between messages by 2px (18.73 KB, image/gif)
2012-11-01 06:05 PDT, Nikita Vasilyev
no flags
Patch. Align icons with text and increase padding between messages by 2px (4.63 KB, patch)
2012-11-01 06:13 PDT, Nikita Vasilyev
no flags
Mockup. Align icons with text and increase padding between messages by 2px (40.62 KB, image/gif)
2012-11-01 06:45 PDT, Nikita Vasilyev
no flags
Patch (4.75 KB, patch)
2012-11-01 09:56 PDT, Nikita Vasilyev
no flags
Updated patch (4.73 KB, patch)
2013-06-10 05:30 PDT, Mátyás Mustoha
ossy: review-
ossy: commit-queue-
Nikita Vasilyev
Comment 1 2012-10-24 16:10:40 PDT
Created attachment 170501 [details] Mockup (before/after)
Nikita Vasilyev
Comment 2 2012-10-24 16:17:38 PDT
Nikita Vasilyev
Comment 3 2012-10-24 16:24:27 PDT
Code used on the mockup: console.log('This is log message'); console.debug('This is debug message'); console.error('This is error message'); console.info('This is info message'); console.warn('This is warning message'); console.log(''); console.group('Group 1'); console.log('content of a group'); console.group('Nested group'); for (var i = 3; i < 7; i++) console.log('%i * Pi =', i, i * Math.PI); var j = 42; while (j--) console.warn('ACHTUNG'); console.log('Multiline message\nsecond line\nthird line.'); console.groupEnd('Nested group'); throw new Error('Example'); console.groupEnd('Group 1');
Pavel Feldman
Comment 4 2012-10-25 05:05:57 PDT
(In reply to comment #0) > Created an attachment (id=170500) [details] > Hover bug > > Key points: > > — Add subtle background color for console.error and console.warn to make it more apparent Not sure I like this one. > — Properly align icons with text > — Add subtle gradient for group titles I definitely don't like this one. > — Use non-monospace font for source links to make them more readable and more distinguishable from the code Console should be monospace > — Remove gray background on mouse over (introduced by https://bugs.webkit.org/show_bug.cgi?id=98701). It overlays background of elements; see attached hover-bug.png
Nikita Vasilyev
Comment 5 2012-10-25 09:15:16 PDT
(In reply to comment #4) > Console should be monospace I made non-monospace only links. Why should they be monospace? Benefits of monospace font: — predictable caret movement between rows. — l, I, i, o, O, and 0 are easy to distinguish. Downsides: — too wide — less readable
Pavel Feldman
Comment 6 2012-10-25 09:24:12 PDT
> I made non-monospace only links. Why should they be monospace? - Maybe because I perceive console as a terminal. - Mostly, because whenever something is related to source code (and I think file path is somewhat related to code), I try to use <code> (monospace).
Nikita Vasilyev
Comment 7 2012-11-01 06:05:59 PDT
Created attachment 171825 [details] Mockup. Align icons with text and increase padding between messages by 2px https://bugs.webkit.org/show_bug.cgi?id=100489 I dig animated GIF by Mike.
Nikita Vasilyev
Comment 8 2012-11-01 06:13:40 PDT
Created attachment 171827 [details] Patch. Align icons with text and increase padding between messages by 2px Pavel, I made a small patch that doesn’t have changes you don’t like
Alexander Pavlov (apavlov)
Comment 9 2012-11-01 06:26:07 PDT
Comment on attachment 171827 [details] Patch. Align icons with text and increase padding between messages by 2px View in context: https://bugs.webkit.org/attachment.cgi?id=171827&action=review Please make sure you don't break what https://bugs.webkit.org/show_bug.cgi?id=100105 fixed. Can you attach a screenshot with all possible combinations of console message types? (nested groups, errors, warnings et al.) > Source/WebCore/inspector/front-end/inspector.css:-909 > - border-top: 1px solid rgb(240, 240, 240); I would not touch this without a severe reason, since it was flipped from border-bottom a week ago, to fix https://bugs.webkit.org/show_bug.cgi?id=100105.
Nikita Vasilyev
Comment 10 2012-11-01 06:45:23 PDT
Created attachment 171835 [details] Mockup. Align icons with text and increase padding between messages by 2px This patch doesn’t seem to introduce any bugs. However, https://bugs.webkit.org/show_bug.cgi?id=100105 seems to do: take a look at a link right next to the "ACHTUNG".
Nikita Vasilyev
Comment 11 2012-11-01 09:56:52 PDT
Created attachment 171879 [details] Patch I’ve made a testcase for bug 100105. http://elv1s.ru/x/long-js-filename-test.html My `.console-message {overflow: hidden}` solution works exactly the same as the one by Mike but with less code, e.g. 1 line of code versus 5.
Pavel Feldman
Comment 12 2012-11-13 23:58:49 PST
Comment on attachment 171879 [details] Patch Is this still relevant? Please restore r? if it is.
Nikita Vasilyev
Comment 13 2012-11-14 00:27:30 PST
Comment on attachment 171879 [details] Patch Yes, still relevant.
Pavel Feldman
Comment 14 2013-03-19 09:51:12 PDT
Comment on attachment 171879 [details] Patch Clearing r? - it has been there for ages, probably obsolete.
Mátyás Mustoha
Comment 15 2013-06-10 05:30:57 PDT
Created attachment 204156 [details] Updated patch
Csaba Osztrogonác
Comment 16 2013-10-21 03:52:44 PDT
Comment on attachment 204156 [details] Updated patch r-, because the old inspector UI was removed.
Note You need to log in before you can comment on or make changes to this bug.