Web Inspector: [Resources] Make grid columns set configurable.

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Eugene Klyuchnikov]

Use context menu on grid header to hide/show grid columns.

[Eugene Klyuchnikov]

Created attachment 180710 [details]
Patch

[Pavel Feldman]

Comment on attachment 180710 [details]
Patch

I applied that patch and noticed that you don't persist the visible column set. The header is also misaligned with the grid once you hide one of the columns.

[Eugene Klyuchnikov]

(In reply to comment #2)
> (From update of attachment 180710 [details])
> I applied that patch and noticed that you don't persist the visible column set. The header is also misaligned with the grid once you hide one of the columns.

I'll add persistence.

BUT misalignment is not caused by this patch, so it should be addressed separately.

[Eugene Klyuchnikov]

Created attachment 180797 [details]
Patch

[Pavel Feldman]

Comment on attachment 180797 [details]
Patch

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

> Source/WebCore/inspector/front-end/NetworkPanel.js:158
> +        var hiddenColumns = WebInspector.settings.hiddenNetworkLogColumns.get();

This setting should be defined in the panel.

> Source/WebCore/inspector/front-end/NetworkPanel.js:161
> +            if (this._detailedViewColumnsSet[hiddenColumns[i]])

At some point, we will have more columns and only some of them will be visible by default. So I don't think you need this._detailedViewColumnsSet preset. It is jus that the setting should have such value by default.

> Source/WebCore/inspector/front-end/Settings.js:118
> +    this.hiddenNetworkLogColumns = this.createSetting("hiddenNetworkLogColumns", []);

Move to NetworkPanel.js and have a default value for it.

> Source/WebCore/inspector/front-end/networkLogView.css:561
> +#network-container .hide-method-column .method-column,

Now this stops being scaleable. Adding a column would require adding to this stupid rule... This (and other styles where we enumerate column names) could be addressed in another patch though.

[Eugene Klyuchnikov]

Comment on attachment 180797 [details]
Patch

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

>> Source/WebCore/inspector/front-end/NetworkPanel.js:158
>> +        var hiddenColumns = WebInspector.settings.hiddenNetworkLogColumns.get();
> 
> This setting should be defined in the panel.

Done

>> Source/WebCore/inspector/front-end/NetworkPanel.js:161
>> +            if (this._detailedViewColumnsSet[hiddenColumns[i]])
> 
> At some point, we will have more columns and only some of them will be visible by default. So I don't think you need this._detailedViewColumnsSet preset. It is jus that the setting should have such value by default.

We use this._detailedViewColumnsSet both to hold visibility and create menu items, because not all columns could be toggled, for example name and timeline are mandatory.

>> Source/WebCore/inspector/front-end/Settings.js:118
>> +    this.hiddenNetworkLogColumns = this.createSetting("hiddenNetworkLogColumns", []);
> 
> Move to NetworkPanel.js and have a default value for it.

Done.

>> Source/WebCore/inspector/front-end/networkLogView.css:561
>> +#network-container .hide-method-column .method-column,
> 
> Now this stops being scaleable. Adding a column would require adding to this stupid rule... This (and other styles where we enumerate column names) could be addressed in another patch though.

Totally agree. It was a hard choice between scaleability and least invasiveness, but I chose the last.
Fixed.

[Eugene Klyuchnikov]

Created attachment 182289 [details]
Patch

[WebKit Review Bot]

Comment on attachment 182289 [details]
Patch

Clearing flags on attachment: 182289

Committed r139452: <http://trac.webkit.org/changeset/139452>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.