[webkit-reviews] review granted: [Bug 200891] Web Inspector: Sources: move the resource type scope bar to be next to the filter : [Attachment 376698] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Aug 19 21:31:00 PDT 2019
Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 200891: Web Inspector: Sources: move the resource type scope bar to be next
to the filter
https://bugs.webkit.org/show_bug.cgi?id=200891
Attachment 376698: Patch
https://bugs.webkit.org/attachment.cgi?id=376698&action=review
--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 376698
--> https://bugs.webkit.org/attachment.cgi?id=376698
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=376698&action=review
Nice! r=me
• I think we should be able to eliminate the "Group By" label
• I think we will want to better handle filtering at the bottom if a long name
filter like "Documents" is selected. Perhaps hiding the fields when clicking
into the filter field.
>
Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:209
> + this._resourceGroupingModeNavigationBar = new WI.NavigationBar;
It still seems like `resourcesNavigationBar` is apt. It only has 1 thing in it
right now, but it is still the navigation bar over the resource section.
>
Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js:222
> + let resourceGroupingModeScopeBarItems =
Object.values(this._resourceGroupingModeScopeBarItems);
> + resourceGroupingModeScopeBarItems.unshift(new
WI.ScopeBarItem("sources-resource-grouping-mode-label", WI.UIString("Group
by:"), {disabled: true, exclusive: true}));
I tend to avoid unshift when possible (it can be slow). Here it doesn't seem
necessary and you can avoid the Object.values allocation all-together without
any significant readability loss.
let items = [];
items.push(new WI.ScopeBarItem("sources-resource-grouping-mode-label",
WI.UIString("Group by:"), {disabled: true, exclusive: true}));
for (let key in this._resourceGroupingModeScopeBarItems)
items.push(this._resourceGroupingModeScopeBarItems[key]);
Up to you.
---
I'd really love to eliminate this `Group by` label. It already looks funky and
it is weird to have so many `.disabled` styles that apply to just it when it
could just be a label in the NavigationBar.
Is just `( Type ) ( Path )` really that bad?
* Xcode has `( Hierarchical ) ( Flat )` in its Symbol navigator.
* Xcode has `( Buildtime ) ( Runtime )` in its Issue navigator.
* Xcode has `( By Group ) ( By Time )` in its Report navigator.
* Finder / Mail have `Search: ( This Mac ) ( "Foo" )` in their find window
* Other places (Safari) use segmented controls
I kind of like the idea of `( By Type ) ( By Path )`. But I think just `( Type
) ( Path )` are good enough!
More information about the webkit-reviews
mailing list