Bug 182950

Summary: Web Inspector: Canvas tab: don't automatically select a recording when viewing a canvas
Product: WebKit Reporter: Devin Rousso <drousso>
Component: Web InspectorAssignee: Devin Rousso <drousso>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, mattbaker, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 173807, 175485    
Attachments:
Description Flags
Patch
none
[Image] After Patch is applied
none
Patch none

Description Devin Rousso 2018-02-19 17:47:01 PST
I find it annoying that when I select a Canvas from the overview, it automatically selects the first recording, causing the recording preview to be shown instead of the current content.  We should change this to be where we only select a recording when we are directly viewing a recording (e.g. when a recording finishes or if the user selects one from the menu in the overview).
Comment 1 Devin Rousso 2018-02-19 17:55:16 PST
Created attachment 334215 [details]
Patch
Comment 2 Devin Rousso 2018-02-19 17:55:28 PST
Created attachment 334216 [details]
[Image] After Patch is applied
Comment 3 Matt Baker 2018-05-10 12:56:36 PDT
Comment on attachment 334215 [details]
Patch

I'd like to see this behavior pushed into ScopeBar itself. ScopeBar's constructor takes a defaultItem, which AFAIK is never null (although we don't assert this). You could refactor ScopeBar to allow it to be null, and use ScopeBar.prototype.resetToDefault whenever you want to set the selection to none.
Comment 4 Devin Rousso 2018-05-14 13:18:08 PDT
(In reply to Matt Baker from comment #3)
> Comment on attachment 334215 [details]
> Patch
> 
> I'd like to see this behavior pushed into ScopeBar itself. ScopeBar's
> constructor takes a defaultItem, which AFAIK is never null (although we
> don't assert this). You could refactor ScopeBar to allow it to be null, and
> use ScopeBar.prototype.resetToDefault whenever you want to set the selection
> to none.
My worry about doing this is that it would break other uses of `WI.ScopeBar`, as I don't think any of them currently support a non-selectable default item (or even want that functionality).  It might be worth doing as a followup, but I'm not sure how applicable it would be elsewhere.
Comment 5 Matt Baker 2018-05-14 13:30:26 PDT
Comment on attachment 334215 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/CanvasTabContentView.js:42
> +        this._canvasTreeOutline.allowsRepeatSelection = true;

I think this change deserves an explanation in the change log.
Comment 6 Devin Rousso 2018-05-14 15:11:32 PDT
Comment on attachment 334215 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/CanvasTabContentView.js:42
>> +        this._canvasTreeOutline.allowsRepeatSelection = true;
> 
> I think this change deserves an explanation in the change log.

I'm not actually sure why this was there 😅.  I tried removing it and found no difference.
Comment 7 Devin Rousso 2018-05-14 15:16:05 PDT
Created attachment 340366 [details]
Patch
Comment 8 WebKit Commit Bot 2018-05-14 16:01:30 PDT
Comment on attachment 340366 [details]
Patch

Clearing flags on attachment: 340366

Committed r231773: <https://trac.webkit.org/changeset/231773>
Comment 9 WebKit Commit Bot 2018-05-14 16:01:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2018-05-14 16:03:40 PDT
<rdar://problem/40237015>