-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Duration/Size columns to the header of view-event list. #55
base: main
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Haoxiqiang, thanks for this! Great move 👍 👍 👍
In general I'm totally on board with this as a feature, it makes perfect sense, but I know some users already have issues with the limited UI space in the view list, and this would make that worse.
I think this would be fine if it was optional though. Would you be up for adding that? I'm imagining a little ...
button in the top right of the headers, where you can toggle columns on & off. I would suggest only including these two new columns in the options there, not the existing ones for now, as toggling the built-in columns may be quite a bit more complicated (due to some new rows with non-matching layouts - see EventTypeColumn
, RTCEventLabel
, BuiltInApiRequestDetails
etc).
Does that make sense?
(durationMs < 100) ? | ||
(sigFig(durationMs, 2) + 'ms') : | ||
(durationMs < 1000 ? sigFig(durationMs, 1) + 'ms' : (durationMs < 10000 ? sigFig(durationMs / 1000, 3) + ' s' : sigFig(durationMs / 1000, 1) + ' s')) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this whole chunk would probably be nicer as a reusable function like formatDuration
somewhere, e.g. src/util/index.ts.
ok, I'll do it later. |
Refactored the code. Most of the time, the space is actually enough, even if it is not enough, the path part can be compressed. |
Great, thanks for refactoring that 👍.
To be clear, I'm imagining something like this: When clicking those dots, it can just open a little menu beneath with checkboxes, something like:
I'm sure you're right that on a normal screen it fits fine in most cases, but there's definitely lots of users with much smaller screens, and at 1024x768 it's already all pretty squeezed! |
…_yarn/terser-4.8.1 Bump terser from 4.8.0 to 4.8.1
e0e350c
to
1d713c8
Compare
The request list shows the duration/size may be more intuitive for analysis.