RESOLVED WONTFIX 100302
Web Inspector: Console: vertically align icons with text
https://bugs.webkit.org/show_bug.cgi?id=100302
Summary Web Inspector: Console: vertically align icons with text
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.