-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: add type column to WebLogView #372
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.
LGTM
src/components/WebLogView/Row.tsx
Outdated
@@ -53,6 +54,11 @@ export function Row({ data, isSelected, onSelectRequest }: RowProps) { | |||
/> | |||
</ResponseStatusBadge> | |||
</Table.Cell> | |||
<Table.Cell> | |||
<Text size="1"> | |||
{isNonStaticAssetResponse(data) ? 'Dynamic' : 'Static'} |
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.
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.
@going-confetti I made a change to include a more descriptive "type" based on the content-type returned by the request.
|
||
export function getRequestType(data: ProxyData) { | ||
if (isNonStaticAssetResponse(data)) { | ||
return 'fetch' |
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.
ChromeDevTools categorizes this a bit more. I was debating whether to call it "document", "dynamic" or "fetch". Any thoughts?
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.
Perhaps, we can make use of the content-type header to categorize different types of dynamic requests?
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.
@going-confetti and I discussed that most dynamic requests have the following content types:
- text/plain
- text/html
- application/json
This makes it tricky to differentiate them so we'll leave them as fetch
for now and come back later on if needed.
Description
This PR adds a new
Type
column to theWebLogView
component to differentiate static assets from dynamic requests.How to Test
Checklist
npm run lint
) and all checks pass.npm test
) and all tests pass.Screenshots (if appropriate):
Related PR(s)/Issue(s)
Resolves #325