Bug 179329 – Web Inspector: Canvas Tab initial user experience is poor when no canvases exist

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

[Blaze Burg]

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.

[Radar WebKit Bug Importer]

<rdar://problem/35842036>

[Matt Baker]

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]

Created attachment 328405 [details]
Patch

[Matt Baker]

Chrome and Firefox have similar UI in various locations to surface toolbar buttons, and provide helpful hints when no content is shown.

[Matt Baker]

Created attachment 328407 [details]
Patch

[Matt Baker]

(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 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 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 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]

Created attachment 328602 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 328602 [details]
Patch for landing

Clearing flags on attachment: 328602

Committed r225587: <https://trac.webkit.org/changeset/225587>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.