Bug 183316 - Web Inspector: Sources: add SourcesTabContentView and SourcesSidebarPanel classes
Summary: Web Inspector: Sources: add SourcesTabContentView and SourcesSidebarPanel cla...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks: 183420
  Show dependency treegraph
 
Reported: 2018-03-03 16:18 PST by Nikita Vasilyev
Modified: 2018-03-08 17:32 PST (History)
4 users (show)

See Also:


Attachments
Patch (28.55 KB, patch)
2018-03-03 16:32 PST, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Patch (31.17 KB, patch)
2018-03-08 15:38 PST, Nikita Vasilyev
mattbaker: review+
Details | Formatted Diff | Diff
Patch (30.97 KB, patch)
2018-03-08 16:23 PST, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2018-03-03 16:18:03 PST
"Enable Sources tab" experimental setting should add a new Sources tab. At first,
it will be an exact copy of Resources tab with "resources" renamed to "sources"
where it makes sense.
Comment 1 Radar WebKit Bug Importer 2018-03-03 16:18:21 PST
<rdar://problem/38107639>
Comment 2 Nikita Vasilyev 2018-03-03 16:32:03 PST
Created attachment 334969 [details]
Patch
Comment 3 Matt Baker 2018-03-08 14:26:27 PST
Comment on attachment 334969 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Main.html:745
> +    <script src="Views/SourceSidebarPanel.js"></script>

We should be consistent, and pick Source or Sources for these new classes. I'm in favor of SourcesSidebarPanel/SourcesTabContentView, because it stands out from SourceCode* SourceMap* prefixed classes.

> Source/WebInspectorUI/UserInterface/Views/SourceSidebarPanel.js:30
> +        super("source", WI.UIString("Sources"), true);

"sources"

> Source/WebInspectorUI/UserInterface/Views/SourceSidebarPanel.js:37
> +        var scopeItemPrefix = "source-sidebar-";

"sources-sidebar-"

> Source/WebInspectorUI/UserInterface/Views/SourceSidebarPanel.js:38
> +        var scopeBarItems = [];

I realize this file was copy-pasted from ResourceNavigationSidebarPanel, but we might as well find-replace var/let.

> Source/WebInspectorUI/UserInterface/Views/SourceSidebarPanel.js:49
> +        this._scopeBar = new WI.ScopeBar("source-sidebar-scope-bar", scopeBarItems, scopeBarItems[0], true);

"sources-sidebar-scope-bar"
Comment 4 Nikita Vasilyev 2018-03-08 15:38:27 PST
Created attachment 335356 [details]
Patch
Comment 5 Matt Baker 2018-03-08 16:18:20 PST
Comment on attachment 335356 [details]
Patch

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

r=me

> Source/WebInspectorUI/ChangeLog:57
> +        added and the identifier changed from "resources" to "sources".

I think both this, and the previous comment, can be merged into the top-level change log comment:

Add Sources tab and sidebar panel, which are copies of the corresponding Resources classes.
The Sources tab is shown when it's enabled in the experimental settings. This patch doesn't
remove existing Resources and Debugger tabs.
Comment 6 Nikita Vasilyev 2018-03-08 16:23:26 PST
Created attachment 335366 [details]
Patch
Comment 7 WebKit Commit Bot 2018-03-08 17:32:56 PST
Comment on attachment 335366 [details]
Patch

Clearing flags on attachment: 335366

Committed r229443: <https://trac.webkit.org/changeset/229443>
Comment 8 WebKit Commit Bot 2018-03-08 17:32:58 PST
All reviewed patches have been landed.  Closing bug.