Bug 179329

Summary: Web Inspector: Canvas Tab initial user experience is poor when no canvases exist
Product: WebKit Reporter: Blaze Burg <bburg>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, mattbaker, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 175485    
Attachments:
Description Flags
Screenshot
none
[Image] New "button help" UI
none
Patch
none
Patch
none
Patch for landing none

Blaze Burg
Reported 2017-11-06 10:26:31 PST
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.
Attachments
Screenshot (217.48 KB, image/png)
2017-11-06 10:26 PST, Blaze Burg
no flags
[Image] New "button help" UI (247.16 KB, image/png)
2017-12-04 16:03 PST, Matt Baker
no flags
Patch (14.10 KB, patch)
2017-12-04 16:27 PST, Matt Baker
no flags
Patch (14.21 KB, patch)
2017-12-04 16:32 PST, Matt Baker
no flags
Patch for landing (14.23 KB, patch)
2017-12-06 11:56 PST, Matt Baker
no flags
Radar WebKit Bug Importer
Comment 1 2017-12-04 16:00:06 PST
Matt Baker
Comment 2 2017-12-04 16:03:15 PST
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)
Matt Baker
Comment 3 2017-12-04 16:27:10 PST
Matt Baker
Comment 4 2017-12-04 16:28:30 PST
Chrome and Firefox have similar UI in various locations to surface toolbar buttons, and provide helpful hints when no content is shown.
Matt Baker
Comment 5 2017-12-04 16:32:02 PST
Matt Baker
Comment 6 2017-12-04 16:34:51 PST
(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.
Timothy Hatcher
Comment 7 2017-12-05 23:09:56 PST
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.
Matt Baker
Comment 8 2017-12-06 11:48:14 PST
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.
Matt Baker
Comment 9 2017-12-06 11:53:43 PST
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.
Matt Baker
Comment 10 2017-12-06 11:56:03 PST
Created attachment 328602 [details] Patch for landing
WebKit Commit Bot
Comment 11 2017-12-06 12:28:30 PST
Comment on attachment 328602 [details] Patch for landing Clearing flags on attachment: 328602 Committed r225587: <https://trac.webkit.org/changeset/225587>
WebKit Commit Bot
Comment 12 2017-12-06 12:28:32 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.