Bug 178802 - Web Inspector: Canvas Tab: tab should always show "Canvases" overview component as the first path component
Summary: Web Inspector: Canvas Tab: tab should always show "Canvases" overview compone...
Status: RESOLVED DUPLICATE of bug 178744
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks: WebInspectorCanvasTab
  Show dependency treegraph
 
Reported: 2017-10-25 09:59 PDT by Brian Burg
Modified: 2018-09-13 08:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (9.64 KB, patch)
2017-10-29 23:54 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
[Image] After Patch is applied (158.95 KB, image/png)
2017-10-29 23:54 PDT, Devin Rousso
no flags Details
Patch (13.35 KB, patch)
2018-01-17 22:06 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian 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 Brian 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 ***