I want to add tabs that only exist in engineering builds. This would be done by putting the relevant models and content views in the Debug/ directory and registering it inside Bootstrap.js. But, the list of tab types is hardcoded. We should make it dynamic.
<rdar://problem/23659417>
Created attachment 266146 [details] Proposed Fix
Comment on attachment 266146 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=266146&action=review Looks good. r=me > Source/WebInspectorUI/ChangeLog:10 > + for debugging purposes. Though the relevant models and views can be > + put Debug/ directory to exclude from production builds, there's no Grammar: "can be put Debug/" needs refinement. > Source/WebInspectorUI/UserInterface/Base/Main.js:444 > + return new Set(this._knownTabClassesByType.values()); Maybe this should just return the values and not a set? > Source/WebInspectorUI/UserInterface/Views/ConsoleTabContentView.js:38 > + static tabInfo() Style: We've been putting "static" functions inside a "// Static" block. But I've also considered just dropping that block and putting it above the "// Public" block like the constructor. > Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js:137 > + allowedTabClasses.sort(function(a, b) { > + return a.tabInfo().title.localeCompare(b.tabInfo().title); > + }); Arrow function? foo.sort((a, b) => a.tabInfo().title.localeCompare(b.tabInfo().title))
(In reply to comment #3) > Comment on attachment 266146 [details] > Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=266146&action=review > > Looks good. r=me > > > Source/WebInspectorUI/ChangeLog:10 > > + for debugging purposes. Though the relevant models and views can be > > + put Debug/ directory to exclude from production builds, there's no > > Grammar: "can be put Debug/" needs refinement. > > > Source/WebInspectorUI/UserInterface/Base/Main.js:444 > > + return new Set(this._knownTabClassesByType.values()); > > Maybe this should just return the values and not a set? I want to expose the fact that there are no duplicates as part of the API. > > Source/WebInspectorUI/UserInterface/Views/ConsoleTabContentView.js:38 > > + static tabInfo() > > Style: We've been putting "static" functions inside a "// Static" block. But > I've also considered just dropping that block and putting it above the "// > Public" block like the constructor. The block was more useful when we couldn't use the static keyword. I will move this up. > > > Source/WebInspectorUI/UserInterface/Views/NewTabContentView.js:137 > > + allowedTabClasses.sort(function(a, b) { > > + return a.tabInfo().title.localeCompare(b.tabInfo().title); > > + }); > > Arrow function? foo.sort((a, b) => > a.tabInfo().title.localeCompare(b.tabInfo().title))
Committed r193432: <http://trac.webkit.org/changeset/193432>