[webkit-reviews] review denied: [Bug 178802] Web Inspector: Canvas Tab: tab should always show "Canvases" overview component as the first path component : [Attachment 325328] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 30 00:43:22 PDT 2017


Matt Baker <mattbaker at apple.com> has denied Devin Rousso
<webkit at devinrousso.com>'s request for review:
Bug 178802: Web Inspector: Canvas Tab: tab should always show "Canvases"
overview component as the first path component
https://bugs.webkit.org/show_bug.cgi?id=178802

Attachment 325328: Patch

https://bugs.webkit.org/attachment.cgi?id=325328&action=review




--- Comment #4 from Matt Baker <mattbaker at apple.com> ---
Comment on attachment 325328
  --> https://bugs.webkit.org/attachment.cgi?id=325328
Patch

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

r- for the following:

- When viewing a recording, selecting the overview path component doesn't
switch to the CanvasOverviewContentView.
- When viewing the overview, selecting the overview path component doesn't
deselect the selected canvas.

When I was struggling with some of these same issues, I considered adding
direct support for represented object hierarchies with a root/overview tree
element. The ContentBrowser (maybe a new subclass of ContentBrowser) could
create a button + divider for the root, instead of a path component followed by
a ">" separator (to reinforce the fact that it isn't a regular path component
and has button behavior).

> Source/WebInspectorUI/UserInterface/Views/CanvasTabContentView.js:-55
> -	   this.contentBrowser.navigationBar.insertNavigationItem(new
WI.DividerNavigationItem, 3);

The main motivation for having a back button rather than a path component/tree
element is that it can be activated with a single click, rather than bringing
up a <select> with only one item.

> Source/WebInspectorUI/UserInterface/Views/CanvasTabContentView.js:208
> +		  
this.contentBrowser.currentContentView.setSelectedItem(null);

A selection change event isn't being triggered when the overview path component
is selected.


More information about the webkit-reviews mailing list