feat(trace-viewer): Improve spacing and layout in and between network details sections #35425
Conversation
… details sections
This comment has been minimized.
This comment has been minimized.
Test results for "tests 1"2 flaky39017 passed, 805 skipped Merge workflow run. |
agg23
left a comment
agg23
left a comment
There was a problem hiding this comment.
Thank you for the contribution! I have a few design comments.
| const keyLength = Object.keys(data).length; | ||
|
|
||
| return ( | ||
| <details className='network-details' open={isOpen} aria-label={title}> |
There was a problem hiding this comment.
The chevron is misaligned. It should be aligned with the close button in the top bar.
| event.preventDefault(); | ||
| setIsOpen(!isOpen); | ||
| }}> | ||
| {title} {!isOpen && showCountWhenCollapsed && `(${keyLength})`} |
There was a problem hiding this comment.
There should be more spacing between the title and the chevron.
| event.preventDefault(); | ||
| setIsOpen(!isOpen); | ||
| }}> | ||
| {title} {!isOpen && showCountWhenCollapsed && `(${keyLength})`} |
There was a problem hiding this comment.
I would just leave the count onscreen at all times.
|
|
||
| {requestBody && <div className='network-request-details-header'>Request Body</div>} | ||
| {requestBody && <CodeMirrorWrapper text={requestBody.text} mimeType={requestBody.mimeType} readOnly lineNumbers={true}/>} | ||
| <DetailsSection title='General' data={Object.entries({ |
| ` | ||
| ); | ||
|
|
||
| await page.getByText('endpoint').click(); |
There was a problem hiding this comment.
Please add at least one test dealing with expansion and collapse of the new DetailsSections.
| overflow: hidden; | ||
| margin-left: 10px; | ||
| } | ||
| .network-details { |
There was a problem hiding this comment.
Probably should scope this class name better, as it's more generic than we typically have.
| event.preventDefault(); | ||
| setIsOpen(!isOpen); | ||
| }}> | ||
| {title} {!isOpen && showCountWhenCollapsed && `(${keyLength})`} |
There was a problem hiding this comment.
Disable text selection on these headers.
|
It is a little hard to review this PR since it introduces new ways of styling and changes fair amount of code. You have not yet achieved enough trust / ownership of this component for us to do a quick review and reviewing it thoroughly requires time. Try to take it easy and start with the small changes. |
3 participants
References: #35214