Bug 178802

Summary: Web Inspector: Canvas Tab: tab should always show "Canvases" overview component as the first path component
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: bburg, hi, inspector-bugzilla-changes, joepeck, mattbaker, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=181772
Bug Depends on:    
Bug Blocks: 175485    
Attachments:
Description Flags
Patch
none
[Image] After Patch is applied
none
Patch none

Description BJ Burg 2017-10-25 09:59:39 PDT
We discussed this on IRC. This gives yet another way to go back to the overview. It's also weird to have the overview showing no path components at all, so this would fix that.
Comment 1 Radar WebKit Bug Importer 2017-10-25 10:00:05 PDT
<rdar://problem/35176006>
Comment 2 Devin Rousso 2017-10-29 23:54:28 PDT
Created attachment 325328 [details]
Patch
Comment 3 Devin Rousso 2017-10-29 23:54:58 PDT
Created attachment 325329 [details]
[Image] After Patch is applied
Comment 4 Matt Baker 2017-10-30 00:43:22 PDT
Comment on attachment 325328 [details]
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.
Comment 5 Joseph Pecoraro 2017-10-30 11:59:13 PDT
> > 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.

We can solve this by making a Path Component that can just be clicked. The Heap Profiler Path Components could also benefit from this to go back to the list of Snapshots. I really should have implemented it a long time ago.
Comment 6 Devin Rousso 2017-10-30 17:19:13 PDT
Comment on attachment 325328 [details]
Patch

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

>> 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.

I agree with this.  I think we should table this for now and see how it feels without an "Overview" item.
Comment 7 Matt Baker 2017-10-30 18:39:29 PDT
(In reply to Devin Rousso from comment #6)
> Comment on attachment 325328 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=325328&action=review
> 
> >> 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.
> 
> I agree with this.  I think we should table this for now and see how it
> feels without an "Overview" item.

I disagree. Without an overview item how does one return to the overview after making a recording? Wouldn’t the user have to close and re-open the Canvas tab?
Comment 8 Devin Rousso 2017-10-30 21:58:31 PDT
Comment on attachment 325328 [details]
Patch

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

>>>> 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.
>> 
>> I agree with this.  I think we should table this for now and see how it feels without an "Overview" item.
> 
> I disagree. Without an overview item how does one return to the overview after making a recording? Wouldn’t the user have to close and re-open the Canvas tab?
I don't think was very clear.  I think we should keep what we have currently, which is a "<" back arrow that is only visible when viewing a recording.  I don't think having an ever-present "Overview" is that useful, especially if nothing is selected when looking at the Canvas tab.  I personally am fine with having nothing in the path when nothing is selected.
Comment 9 BJ Burg 2017-11-10 09:39:06 PST
(In reply to Devin Rousso from comment #8)
> Comment on attachment 325328 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=325328&action=review
> 
> >>>> 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.
> >> 
> >> I agree with this.  I think we should table this for now and see how it feels without an "Overview" item.
> > 
> > I disagree. Without an overview item how does one return to the overview after making a recording? Wouldn’t the user have to close and re-open the Canvas tab?
> I don't think was very clear.  I think we should keep what we have
> currently, which is a "<" back arrow that is only visible when viewing a
> recording.  I don't think having an ever-present "Overview" is that useful,
> especially if nothing is selected when looking at the Canvas tab.  I
> personally am fine with having nothing in the path when nothing is selected.

The need for this is smaller now because there is other ways to get back to the overview. However, it's really jarring to select a card and suddenly the jump bar goes from zero to two path components. What do you think?

I believe that the back arrow is going to be moved into the navigation sidebar for a recording.
Comment 10 Matt Baker 2017-11-10 12:45:44 PST
(In reply to Brian Burg from comment #9)
> (In reply to Devin Rousso from comment #8)
> > Comment on attachment 325328 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=325328&action=review
> > 
> > >>>> 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.
> > >> 
> > >> I agree with this.  I think we should table this for now and see how it feels without an "Overview" item.
> > > 
> > > I disagree. Without an overview item how does one return to the overview after making a recording? Wouldn’t the user have to close and re-open the Canvas tab?
> > I don't think was very clear.  I think we should keep what we have
> > currently, which is a "<" back arrow that is only visible when viewing a
> > recording.  I don't think having an ever-present "Overview" is that useful,
> > especially if nothing is selected when looking at the Canvas tab.  I
> > personally am fine with having nothing in the path when nothing is selected.
> 
> The need for this is smaller now because there is other ways to get back to
> the overview. However, it's really jarring to select a card and suddenly the
> jump bar goes from zero to two path components. What do you think?

I agree that it is jarring. The overview path component has returned in <https://webkit.org/b/178744>, and renamed to "All Canvases". Let's hold off on any further changes here until that patch lands.

> I believe that the back arrow is going to be moved into the navigation
> sidebar for a recording.
Comment 11 Joseph Pecoraro 2018-01-17 16:02:19 PST
I think we should revisit this. Making the Overview an item that can just be clicked will be a big win for usability.
Comment 12 Matt Baker 2018-01-17 16:04:29 PST
(In reply to Joseph Pecoraro from comment #11)
> I think we should revisit this. Making the Overview an item that can just be
> clicked will be a big win for usability.

Related to this, we previously discussed making path components with no siblings just be clickable, instead of opening a one item select menu.
Comment 13 Devin Rousso 2018-01-17 17:31:11 PST
(In reply to Matt Baker from comment #12)
> (In reply to Joseph Pecoraro from comment #11)
> > I think we should revisit this. Making the Overview an item that can just be
> > clicked will be a big win for usability.
> 
> Related to this, we previously discussed making path components with no
> siblings just be clickable, instead of opening a one item select menu.
<https://webkit.org/b/181772>
Comment 14 Devin Rousso 2018-01-17 22:06:15 PST
Created attachment 331594 [details]
Patch
Comment 15 Devin Rousso 2018-02-19 17:51:57 PST

*** This bug has been marked as a duplicate of bug 178744 ***