Created attachment 326131 [details] Screenshot Visit a page without any canvases, then open Canvas Tab. It just says "No this.. No that..". We should have a better experience, perhaps some text that explains that canvases appear in a grid as they are found, but this page doesn't have any. It should also mention that you can import previous recordings and provide an inline button to click that shows this. Lastly, this might be a good place to add a link to our reference documentation or an article about Canvas Tab. That is out of scope for now.
<rdar://problem/35842036>
Created attachment 328402 [details] [Image] New "button help" UI New descriptive content in the empty Canvas tab overview, which surfaces the Import button from the sidebar. (the Import button style is new, and will be applied to the sidebar in https://webkit.org/b/178744)
Created attachment 328405 [details] Patch
Chrome and Firefox have similar UI in various locations to surface toolbar buttons, and provide helpful hints when no content is shown.
Created attachment 328407 [details] Patch
(In reply to Brian Burg from comment #0) > Created attachment 326131 [details] > Screenshot > > Visit a page without any canvases, then open Canvas Tab. > > It just says "No this.. No that..". > > We should have a better experience, perhaps some text that explains that > canvases appear in a grid as they are found, but this page doesn't have any. > It should also mention that you can import previous recordings and provide > an inline button to click that shows this. This builds on https://webkit.org/b/179330, which added explanatory text to the content view. This addresses the part about mentioning the ability to import previous recordings, by surfacing the "Import" button from the sidebar.
Comment on attachment 328407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328407&action=review > Source/WebInspectorUI/ChangeLog:13 > + This patch adds a new method, WI.createNavigationItemHelp, for creating command > + help to display in a message text view. The method accepts a format string and > + NavigationItem, and returns a DOM element. For example, > + > + WI.createNavigationItemHelp("Press %s to do X.", navigationItem) Neat! > Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js:703 > +localizedStrings["Press %s to load a recording from file."] = "Press %s to load a recording from file."; This could be hard to localize because of the wording in the help button replacement, but I'm not sure what would make it better. > Source/WebInspectorUI/UserInterface/Base/Main.js:2271 > + String.format(formatString, [wrapperElement], String.standardFormatters, containerElement, append); Clever. > Source/WebInspectorUI/UserInterface/Views/CanvasOverviewContentView.js:39 > + let importNavigationItem = new WI.ButtonNavigationItem("import-recording", WI.UIString("Import"), "Images/Import.svg", 15, 15); > + importNavigationItem.buttonStyle = WI.ButtonNavigationItem.Style.ImageAndText; > + importNavigationItem.addEventListener(WI.ButtonNavigationItem.Event.Clicked, () => { WI.canvasManager.importRecording(); }); Anyway to reuse the real ButtonNavigationItem from RecordingNavigationSidebarPanel? > Source/WebInspectorUI/UserInterface/Views/Main.css:204 > +.message-text-view .navigation-item-help .navigation-bar > .item { Text size being the same larger size as the body of the message bothers me. I kind of expect this text to exactly match the navigation bar text.
Comment on attachment 328407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328407&action=review >> Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js:703 >> +localizedStrings["Press %s to load a recording from file."] = "Press %s to load a recording from file."; > > This could be hard to localize because of the wording in the help button replacement, but I'm not sure what would make it better. It does mean that both the help string and any formatted navigation items may need to be considered together when localizing. This does add an additional burden, but how big a burden I couldn't say. I expect that even without this change, many UI strings cannot be localized outside of some context (other UI strings). >> Source/WebInspectorUI/UserInterface/Base/Main.js:2271 >> + String.format(formatString, [wrapperElement], String.standardFormatters, containerElement, append); > > Clever. I was surprised how easy this was! >> Source/WebInspectorUI/UserInterface/Views/CanvasOverviewContentView.js:39 >> + importNavigationItem.addEventListener(WI.ButtonNavigationItem.Event.Clicked, () => { WI.canvasManager.importRecording(); }); > > Anyway to reuse the real ButtonNavigationItem from RecordingNavigationSidebarPanel? It would be awkward from a layering point of view to expose the button to the content view. The big issue is that if the Import button text/image/style changes, it now needs to be updated in multiple places. I've been thinking it would be useful to have a CommandController (accessible from AppController) that has entries for all frontend UI commands. If we ever wanted to add a command palette to the Inspector (Chrome, most editors) this would be a step in that direction. >> Source/WebInspectorUI/UserInterface/Views/Main.css:204 >> +.message-text-view .navigation-item-help .navigation-bar > .item { > > Text size being the same larger size as the body of the message bothers me. I kind of expect this text to exactly match the navigation bar text. This was unintentional, good catch! A nice side effect is that fixing the font size makes the -1px adjustment unnecessary.
Comment on attachment 328407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328407&action=review >>> Source/WebInspectorUI/UserInterface/Views/Main.css:204 >>> +.message-text-view .navigation-item-help .navigation-bar > .item { >> >> Text size being the same larger size as the body of the message bothers me. I kind of expect this text to exactly match the navigation bar text. > > This was unintentional, good catch! A nice side effect is that fixing the font size makes the -1px adjustment unnecessary. I spoke too soon. Reducing the font to 11px requires 1px of vertical padding to make the button look good, which makes the -1px still necessary.
Created attachment 328602 [details] Patch for landing
Comment on attachment 328602 [details] Patch for landing Clearing flags on attachment: 328602 Committed r225587: <https://trac.webkit.org/changeset/225587>
All reviewed patches have been landed. Closing bug.