Bug 161938 – Web Inspector: Make console session dividers more pronounced

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Created attachment 288755 [details]
[Image] Session separator

Over the past two years I have been asked numerous times to implement "Clear Log on Navigation" in the Console.

(Web Inspector have had this feature for a few years but it's only available via context menu and, evidently, it isn't easily discoverable.)

I've asked, why do they want console to be cleared. Often, the answer is "it's hard to see which messages were added before or after the page was reloaded".

Currently, we use a gray dashed line as a session divider. Some people didn't understand what it means or didn't even notice it.

[Radar WebKit Bug Importer]

<rdar://problem/28291166>

[Nikita Vasilyev]

Created attachment 288760 [details]
Patch

[Nikita Vasilyev]

Created attachment 288762 [details]
[Image] With patch applied

[Joseph Pecoraro]

Comment on attachment 288760 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=288760&action=review

What does it look like after 5 reloads on a page without any console logs? Could you include a screenshot? Maybe we should merge / collapse empty sessions.

I really think we should do the `filter: grayscale(1)` on old messages. If the problem trying to be solved is that its hard to distinguish old and new messages, the grayscale makes it obvious at any scroll position, not just the divider.

> Source/WebInspectorUI/ChangeLog:16
> +        * Localizations/en.lproj/localizedStrings.js:

The diff to this file should not be binary anymore. You may need to update an alias? extract-localizable-js-strings should have a --utf8 switch.

> Source/WebInspectorUI/ChangeLog:26
> +        Session separator wasn't appended when navigating to a different web page. I consider it a bug.

Instead of "I consider it a bug", this comment could describe the change concisely:

    Always dispatch SessionStarted events when the main resource changes.
    Distinguish between reload and navigation.

> Source/WebInspectorUI/UserInterface/Views/ConsoleSession.js:54
> +        case WebInspector.ConsoleSession.NewSessionReason.ConsoleCleared:
> +            headerText = WebInspector.UIString("Console cleared at %s");
> +            break;
> +        default:
> +            headerText = WebInspector.UIString("Console opened at %s");
> +            break;

Is it worth showing a divider in these cases? I think I'd rather have a clear console log then seeing this message.

[Nikita Vasilyev]

Created attachment 288837 [details]
[Image] 5 reloads

(In reply to comment #4)
> Comment on attachment 288760 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=288760&action=review
> 
> What does it look like after 5 reloads on a page without any console logs?
> Could you include a screenshot? Maybe we should merge / collapse empty
> sessions.

Yes, we should probably do that.

[Nikita Vasilyev]

Created attachment 288838 [details]
[Image] Console cleared message

(In reply to comment #4)
> > Source/WebInspectorUI/UserInterface/Views/ConsoleSession.js:54
> > +        case WebInspector.ConsoleSession.NewSessionReason.ConsoleCleared:
> > +            headerText = WebInspector.UIString("Console cleared at %s");
> > +            break;
> > +        default:
> > +            headerText = WebInspector.UIString("Console opened at %s");
> > +            break;
> 
> Is it worth showing a divider in these cases? I think I'd rather have a
> clear console log then seeing this message.

This is how it looks. It has a time stamp and I thought it wouldn't be too noisy.

[Nikita Vasilyev]

(In reply to comment #4)
> I really think we should do the `filter: grayscale(1)` on old messages. If
> the problem trying to be solved is that its hard to distinguish old and new
> messages, the grayscale makes it obvious at any scroll position, not just
> the divider.

I'm ambivalent about this. I'm concerned about message selection color — it becomes light gray. In addition to that, grayscale filter disables subpixel font rendering.

However, it does show old messages very distinctly. I'm thinking how to make the best of both worlds.

[Nikita Vasilyev]

Created attachment 288883 [details]
Patch

cq- because I still don't know how to make my git treat localizedStrings.js as utf8 and non-binary.

[Nikita Vasilyev]

Created attachment 288884 [details]
[Image] Half grayscale old sessions

Can we settle on `filter: grayscale(0.5)`?

[Nikita Vasilyev]

Created attachment 288888 [details]
[Image] After 5 reloads

[Nikita Vasilyev]

Created attachment 288897 [details]
Patch

Same patch as before, but with:

    ./Tools/Scripts/extract-localizable-js-strings --utf8 Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js Source/WebInspectorUI/UserInterface/
    git diff --binary

[Nikita Vasilyev]

Created attachment 288898 [details]
Patch

[Nikita Vasilyev]

Created attachment 289008 [details]
[Image] Timelines: Paint time with grayscale filter

Unfortunately, grayscale filter makes scrolling very jittery. Using grayscale filter increases paint time 10 times.

[Nikita Vasilyev]

Created attachment 289009 [details]
Patch

[Devin Rousso]

So I was thinking about this, and what about doing something like a `position: sticky;` where the opened/reloaded/cleared messages stay on the bottom of the console when scrolling?  e.g. when the user has reloaded the page after logging a bunch of messages (so the "Page Reloaded" meta-message is added), if they scroll back up to be in the "area" of the messages before the reload, the meta-message will be "stuck" to the bottom of the console area (right above the prompt).  I would do a mock, but my photoshop skills are in the negatives on a 1-10 scale :/

[Blaze Burg]

Comment on attachment 289008 [details]
[Image] Timelines: Paint time with grayscale filter

That is pretty hideously slow! This seems like a rendering bug to me.

[Blaze Burg]

Comment on attachment 289009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=289009&action=review

r=me but please adjust styles as noted.

> Source/WebInspectorUI/UserInterface/Controllers/JavaScriptLogViewController.js:108
> +        let consoleSession = new WebInspector.ConsoleSession(isFirstSession, data);

See below.

> Source/WebInspectorUI/UserInterface/Views/ConsoleSession.js:59
> +        let timestamp = isFirstSession ? Date.now() : data.timestamp;

Please get rid of isFirstSession and just assign data.timestamp in the code path for first session.

> Source/WebInspectorUI/UserInterface/Views/LogContentView.css:168
> +    border-top: .5px dashed hsl(0, 0%, 0%);

The tiny dashed line between reloads/navigations is visually jarring as we don't use 100% black elsewhere in the console or the inspector really.

The background looks too hard. Maybe we can reuse the gradient style from the tab bar to make this less flat. It might need to be lightened further.

    background-image: linear-gradient(to bottom, hsl(0, 0%, 87%), hsl(0, 0%, 82%));

> Source/WebInspectorUI/UserInterface/Views/LogContentView.css:178
> +    color: hsl(0, 0%, 60%);

Do we have variables for any of these theme colors yet?

[Matt Baker]

Comment on attachment 289009 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=289009&action=review

>> Source/WebInspectorUI/UserInterface/Views/LogContentView.css:178
>> +    color: hsl(0, 0%, 60%);
> 
> Do we have variables for any of these theme colors yet?

This is close to --border-color-dark, which was added recently.

[Nikita Vasilyev]

(In reply to comment #18)
> Comment on attachment 289009 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289009&action=review
> 
> >> Source/WebInspectorUI/UserInterface/Views/LogContentView.css:178
> >> +    color: hsl(0, 0%, 60%);
> > 
> > Do we have variables for any of these theme colors yet?
> 
> This is close to --border-color-dark, which was added recently.

Wouldn't it be strange to use --border-color-dark for text color, not border?

[Nikita Vasilyev]

(In reply to comment #15)
> So I was thinking about this, and what about doing something like a
> `position: sticky;` where the opened/reloaded/cleared messages stay on the
> bottom of the console when scrolling?  e.g. when the user has reloaded the
> page after logging a bunch of messages (so the "Page Reloaded" meta-message
> is added), if they scroll back up to be in the "area" of the messages before
> the reload, the meta-message will be "stuck" to the bottom of the console
> area (right above the prompt).  I would do a mock, but my photoshop skills
> are in the negatives on a 1-10 scale :/

I can picture it being stuck to the top, not to the bottom. Regardless, I'm concerned about wasting screen space.

It may worth exploring once this is landed.

[Matt Baker]

(In reply to comment #19)
> (In reply to comment #18)
> > Comment on attachment 289009 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=289009&action=review
> > 
> > >> Source/WebInspectorUI/UserInterface/Views/LogContentView.css:178
> > >> +    color: hsl(0, 0%, 60%);
> > > 
> > > Do we have variables for any of these theme colors yet?
> > 
> > This is close to --border-color-dark, which was added recently.
> 
> Wouldn't it be strange to use --border-color-dark for text color, not border?

Good call. Reusing color variable names would also tie colors together that aren't actually related. I filed a bug for cleaning up our UI colors: https://bugs.webkit.org/show_bug.cgi?id=162047.

[Nikita Vasilyev]

(In reply to comment #16)
> Comment on attachment 289008 [details]
> [Image] Timelines: Paint time with grayscale filter
> 
> That is pretty hideously slow! This seems like a rendering bug to me.

I filed <rdar://problem/28343648>.

[Nikita Vasilyev]

Created attachment 289145 [details]
Patch

[Nikita Vasilyev]

(In reply to comment #17)
> Comment on attachment 289009 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289009&action=review
> 
> r=me but please adjust styles as noted.
> 
> > Source/WebInspectorUI/UserInterface/Controllers/JavaScriptLogViewController.js:108
> > +        let consoleSession = new WebInspector.ConsoleSession(isFirstSession, data);
> 
> See below.
> 
> > Source/WebInspectorUI/UserInterface/Views/ConsoleSession.js:59
> > +        let timestamp = isFirstSession ? Date.now() : data.timestamp;
> 
> Please get rid of isFirstSession and just assign data.timestamp in the code
> path for first session.
> 
> > Source/WebInspectorUI/UserInterface/Views/LogContentView.css:168
> > +    border-top: .5px dashed hsl(0, 0%, 0%);
> 
> The tiny dashed line between reloads/navigations is visually jarring as we
> don't use 100% black elsewhere in the console or the inspector really.
> 
> The background looks too hard. Maybe we can reuse the gradient style from
> the tab bar to make this less flat. It might need to be lightened further.
> 
>     background-image: linear-gradient(to bottom, hsl(0, 0%, 87%), hsl(0, 0%,
> 82%));

I lighten it up but I didn't use the gradient. I think, in Web Inspector and in
macOS in general, gradients are primarily used for toolbars and buttons.

[WebKit Commit Bot]

Comment on attachment 289145 [details]
Patch

Clearing flags on attachment: 289145

Committed r206057: <http://trac.webkit.org/changeset/206057>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.