Bug 169854 - Web Inspector: WebSockets: Time column should have a fixed width
Summary: Web Inspector: WebSockets: Time column should have a fixed width
Status: ASSIGNED
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:
 
Reported: 2017-03-18 15:46 PDT by Nikita Vasilyev
Modified: 2017-06-15 14:45 PDT (History)
3 users (show)

See Also:


Attachments
Patch (13.78 KB, patch)
2017-03-18 16:09 PDT, Nikita Vasilyev
bburg: review-
bburg: commit-queue-
Details | Formatted Diff | Diff
[Image] With patch applied (89.69 KB, image/png)
2017-03-18 16:11 PDT, Nikita Vasilyev
no flags Details
[Image] Truncated data in network (104.68 KB, image/png)
2017-06-15 14:45 PDT, Nikita Vasilyev
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2017-03-18 15:46:28 PDT
I'd like to move away from using DataGrid for WebSocketContentView.

1. It doesn't allow to set a fixed width for a column. There are only two columns, and they don't need to be resizable.
2. It doesn't allow to select multiple rows.

I envision WebSocketContentView to be more like a console view and less like a DataGrid.
Comment 1 Radar WebKit Bug Importer 2017-03-18 15:46:46 PDT
<rdar://problem/31132888>
Comment 2 Nikita Vasilyev 2017-03-18 16:09:41 PDT
Created attachment 304884 [details]
Patch
Comment 3 Nikita Vasilyev 2017-03-18 16:11:12 PDT
Created attachment 304885 [details]
[Image] With patch applied
Comment 4 BJ Burg 2017-03-18 17:05:31 PDT
(In reply to comment #0)
> I'd like to move away from using DataGrid for WebSocketContentView.
> 
> 1. It doesn't allow to set a fixed width for a column. There are only two
> columns, and they don't need to be resizable.

DataGrid also supports searching, hiding columns, sorting, reordering, restoring selections on reload/reopen, and lots of other things. I don't think your preference for not resizing is significant enough to make another leaf view class that must address most of the same things. If preventing resizing is critical to the functionality, then you should modify DataGrid to add an option for making columns not resizable. This is also a bad idea because localization can increase column widths by 20-50% depending on the language.

> 2. It doesn't allow to select multiple rows.

This is a longstanding bug and we need someone to fix it for many other reasons.
 
> I envision WebSocketContentView to be more like a console view and less like
> a DataGrid.
Comment 5 BJ Burg 2017-03-18 17:05:57 PDT
Comment on attachment 304884 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:9
> +        Replace DataGrid with a WebSocketLogView, which uses an HTML table directly.

The rationale needs to be in the changelog as well as the bug. It could change over time as the patch is worked on.

> Source/WebInspectorUI/UserInterface/Views/WebSocketLogView.cssSource/WebInspectorUI/UserInterface/Views/WebSocketDataGridNode.js:27
> +    border-bottom: 0.5px solid hsla(0, 0%, 0%, 0.1);

Does this really need to use alpha?

> Source/WebInspectorUI/UserInterface/Views/WebSocketLogView.cssSource/WebInspectorUI/UserInterface/Views/WebSocketDataGridNode.js:31
> +    background-color: hsl(80, 85%, 92%);

You should really use variables to name colors. I have no idea what color this is.

> Source/WebInspectorUI/UserInterface/Views/WebSocketLogView.cssSource/WebInspectorUI/UserInterface/Views/WebSocketDataGridNode.js:48
> +    padding: 4px 0 4px 0;

Use shorthand.

> Source/WebInspectorUI/UserInterface/Views/WebSocketLogView.cssSource/WebInspectorUI/UserInterface/Views/WebSocketDataGridNode.js:53
> +    padding-right: 6px;

Use a variable.
Comment 6 Nikita Vasilyev 2017-06-15 14:45:08 PDT
Created attachment 313013 [details]
[Image] Truncated data in network

I've tried implementing a DataGrid that is either content aware or at least accepts min-width property for specified column.

This way problematic with the current DataGrid resizing logic. Specifically, column width is stored as a percentage value:

    <colgroup>
        <col style="width: 8.098987626546682%;">
        <col style="width: 4.83689538807649%;">
        ...
    </colgroup>

I think, eventually, we want to be able to set min-width for columns. We can't do that if we use <col> elements, min-width is simply doesn't work on <col>.

Being able to set min-width would be very beneficial for Network and Timeline tabs, as currently a lot data there is truncated and unreadable (see the attached image).

It's a broader task, but I think it's worth doing.