RESOLVED INVALID 100489
Web Inspector: Visual cleanup (mostly whitespace).
https://bugs.webkit.org/show_bug.cgi?id=100489
Summary Web Inspector: Visual cleanup (mostly whitespace).
Mike West
Reported 2012-10-26 03:21:38 PDT
Created attachment 170866 [details] Animated GIF! Text in the console is squeezed together pretty tightly. I'd suggest adding a bit more whitespace for readability. I'd also like to clean up the positioning of the log type icons, and distinguish the active console more clearly from the messages above it. The attached image should help evaluate the changes I'm suggesting.
Attachments
Animated GIF! (136.46 KB, image/gif)
2012-10-26 03:21 PDT, Mike West
no flags
Patch (3.84 KB, patch)
2012-10-26 03:27 PDT, Mike West
no flags
Patch (3.85 KB, patch)
2012-10-26 03:37 PDT, Mike West
pfeldman: review-
Mike West
Comment 1 2012-10-26 03:27:03 PDT
WebKit Review Bot
Comment 2 2012-10-26 03:32:01 PDT
Attachment 170870 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:13: Line contains tab character. [whitespace/tab] [5] Source/WebCore/ChangeLog:15: Line contains tab character. [whitespace/tab] [5] Source/WebCore/ChangeLog:17: Line contains tab character. [whitespace/tab] [5] Total errors found: 3 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mike West
Comment 3 2012-10-26 03:37:32 PDT
Pavel Feldman
Comment 4 2012-10-26 06:49:28 PDT
Comment on attachment 170874 [details] Patch Could you please attach the screenshot? Have you seen https://bugs.webkit.org/show_bug.cgi?id=100302 ? :)
Mike West
Comment 5 2012-10-26 06:51:33 PDT
(In reply to comment #4) > (From update of attachment 170874 [details]) > Could you please attach the screenshot? Have you seen https://bugs.webkit.org/show_bug.cgi?id=100302 ? :) Screenshot is the first attachment: https://bug-100489-attachments.webkit.org/attachment.cgi?id=170866 I haven't seen the other bug! Ha! I'll take a look at it now.
Mike West
Comment 6 2012-10-26 06:54:23 PDT
Ok. Looks like he's doing everything this patch does and more. Awesome. :) I'll strip this patch down to fixing the one actual bug (when I flipped borders from the bottom of messages to the top, I accidentally introduced a border between console commands and their results).
Pavel Feldman
Comment 7 2012-10-26 07:00:51 PDT
Lets land your changes one by one since they are about the UI :) I am not a huge fan of bigger rows - I really like the dense view. Not sure what the behavior of the dashed line is - is it just the last separator?
Mike West
Comment 8 2012-10-26 07:08:04 PDT
(In reply to comment #7) > Lets land your changes one by one since they are about the UI :) As long as lots of tiny patches don't bother you, I'm fine with splitting everything up. > I am not a huge fan of bigger rows - I really like the dense view. When I was young and spry, I would have agreed with you. Now I'm old, and my eyes are broken. :) I like the "after" version in the attached screenshot much more than the "before". But it's certainly something that's at least partially a question of personal taste. > Not sure what the behavior of the dashed line is - is it just the last separator? It's just a differently styled border to separate the console bit where you can type things from the non-active bit where you see things that have happened in the past. The dark and dashed style is a strawman; I'm not convinced it should look exactly like that, but I do think the area where you can type new commands should be better distinguished from the rest.
dubroy
Comment 9 2012-10-26 07:44:31 PDT
+1 on fixing the icon alignment. I like the increased line height in the console -- it makes multiple lines of text more readable. However, the extra padding (dinstance b/w text and grey lines) might be a bit much. I'd try keeping the line-height change but reducing the padding.
Pavel Feldman
Comment 10 2012-10-31 03:13:49 PDT
Comment on attachment 170874 [details] Patch r- as per comment #7
Mike West
Comment 11 2012-10-31 03:17:30 PDT
Right. Closing this out in favor of a million tiny patches; sorry, thought I'd already done that.
Note You need to log in before you can comment on or make changes to this bug.