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.
<rdar://problem/28291166>
Created attachment 288760 [details] Patch
Created attachment 288762 [details] [Image] With patch applied
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.
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.
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.
(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.
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.
Created attachment 288884 [details] [Image] Half grayscale old sessions Can we settle on `filter: grayscale(0.5)`?
Created attachment 288888 [details] [Image] After 5 reloads
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
Created attachment 288898 [details] Patch
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.
Created attachment 289009 [details] Patch
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 :/
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.
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?
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.
(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?
(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.
(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.
(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>.
Created attachment 289145 [details] Patch
(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.
Comment on attachment 289145 [details] Patch Clearing flags on attachment: 289145 Committed r206057: <http://trac.webkit.org/changeset/206057>
All reviewed patches have been landed. Closing bug.