Closed Bug 1561549 Opened 9 months ago Closed 9 months ago

WebSocket frame list should use sent/received icon

Categories

(DevTools :: Netmonitor, enhancement, P2)

enhancement

Tracking

(firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: Honza, Assigned: fvsch)

References

(Blocks 1 open bug)

Details

Attachments

(12 files)

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

Type: defect → enhancement
Priority: -- → P3
Attached image image.png

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

Honza

Flags: needinfo?(florens)

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.

Attached image 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

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>".

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)
Assignee: nobody → florens
Status: NEW → ASSIGNED
Flags: needinfo?(florens)

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).

Light theme.

Some color tests.

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%)
Attachment #9076202 - Attachment description: Bug 1561549 - Use icons in the websockets Type column; r=honza → Bug 1561549 - Reorder and resize WebSockets columns; r=honza

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.

Flags: needinfo?(florens)

: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.

Flags: needinfo?(florens)

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).

Attachment #9076202 - Attachment description: Bug 1561549 - Reorder and resize WebSockets columns; r=honza → Bug 1561549 - Move WebSocket type to an icon in Time column and reorder columns; r=honza
Attachment #9076202 - Attachment description: Bug 1561549 - Move WebSocket type to an icon in Time column and reorder columns; r=honza → Bug 1561549 - Move WebSocket type to an icon in Data column and reorder columns; r=honza
Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a87637d6ebe
Move WebSocket type to an icon in Data column and reorder columns; r=Honza
Priority: P3 → P2
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
You need to log in before you can comment on or make changes to this bug.