1561549 - WebSocket frame list should use sent/received icon

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P2)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

WebSocket frame list is currently using a Type column with Sent and Received values to indicate whether given frame has been sent or received.

We should use an icon (perhaps as part of the Payload column) and remove the Type column entirely. This will also help to save valuable horizontal space in the side panel.

Honza

Comment 1

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: image.png

@Florens, do we have some arrow icons we could use here?

Honza

Comment 2

[Florens Verschelde :fvsch]

I don't think we have them already, but it should be easy to make one.
Can you show me what the current UI looks like?

I can enable devtools.netmonitor.features.webSockets but I'm not sure on which page I could test websockets.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: image.png

This is how it looks now.

You need patch from bug 1559398 to try it on your machine (or wait till it's fixed, it should land soon)

I was thinking about:

• get rid of the Type column (to safe space)
• put the icon into the Payload column (at the left side)

Honza

Comment 4

[Florens Verschelde :fvsch]

Nicolas was suggesting putting the Time column first, maybe we could put the type icon there?
So it would read as "Sent at <time>" and "Received at <time>".

Comment 5

[Florens Verschelde :fvsch]

Attachment: Bug 1561549 - Move WebSocket type to an icon in Data column and reorder columns; r=honza

Comment 6

[Florens Verschelde :fvsch]

So instead of putting an icon in the Time column, or in the Data column like Chrome does, I figured it would be better to keep the "Type" column but make it less text-heavy and more visual.

I have a patch up that:

• Visually hides the header's label (we keep the label around for accessibility)
• Replaces the "received" and "sent" labels in each row with an icon (with the original text as alt text)

Comment 7

[Florens Verschelde :fvsch]

Attachment: websockets-type-column-light.png

Here's how it looks.

Not sure about the colors. Chrome is using green for sent and orange for received, is that a convention that exists? We try to stay away from orange (it's reserved for brand stuff).

Comment 8

[Florens Verschelde :fvsch]

Attachment: websockets-type-column-light.png

Light theme.

Comment 9

[Florens Verschelde :fvsch]

Attachment: websockets-type-colors.png

Some color tests.

Comment 10

[Florens Verschelde :fvsch]

Attachment: websockets-column-reordering.png

Reordering columns as per :nchevobbe's suggestions, and setting some default widths for columns:

1. Type (24px)
2. Time (15%)
3. Size (15%)
4. Payload (auto width, ~27%)
5. OpCode (10%)
6. MaskBit (10%)
7. FinBit (10%)

Comment 11

[Hengyeow (:tanhengyeow)]

Hi :fvsch ! The patch looks good and thanks a lot for working on this, appreciate it :)

I can't post review comments in the Phabricator patch as I don't have enough permissions for that so I'll be posting them here. Please refer to the review-comment-*.png files.

For review-comment-4.png, the link is here.

Comment 12

[Hengyeow (:tanhengyeow)]

Attachment: review-comment-1.png

Comment 13

[Hengyeow (:tanhengyeow)]

Attachment: review-comment-2.png

Comment 14

[Hengyeow (:tanhengyeow)]

Attachment: review-comment-3.png

Comment 15

[Hengyeow (:tanhengyeow)]

Attachment: review-comment-4.png

Comment 16

[Florens Verschelde :fvsch]

:tanhengyeow, I think you can probably submit your comments on Phabricator reviews, but the Phabricator UI for that is a unclear. You have to go to the bottom of the page and submit the comment form even with an empty comment, in order to submit your comments on specific files and lines.

Regarding your comments:

1. and 3. I'm removing the devtools-toolbar class because table headers are not toolbars and we're working hard to fight styles intended for actual toolbars. Instead what we want are "toolbar-like" background and text colors, and thankfully for that we have design primitives defined as CSS variables: --theme-toolbar-color and --theme-toolbar-background. (I've also worked on some style changes for toolbars across panels in a different bug; that work is stalled right now, but having those classes in the Network tables was a problem.)

2. Indeed I'm not using the button-text-hidden class, I ended up using a different selector so this was dead code. Good catch!

3. Thanks for the heads up re. localization.

Comment 17

[Hengyeow (:tanhengyeow)]

Attachment: phab-review-access-denied.png

You have to go to the bottom of the page and submit the comment form even with an empty comment, in order to submit your comments on specific files and lines.

I tried that before but I was shown this error message. I'm guessing commit access level 3 is required (I'm at commit access level 1 now).

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9a87637d6ebe
Move WebSocket type to an icon in Data column and reorder columns; r=Honza

Comment 19

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9a87637d6ebe