Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds the network tab debugging panel to Web Lab 2, allowing students to inspect network requests made by their HTML projects.
Changes:
- Added a debug panel component displaying network request/response information
- Integrated panel into the vertical layout below the preview area
- Added support for clearing network requests on preview refresh/reload
Reviewed changes
Copilot reviewed 9 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/src/weblab2/layout/vertical-layout.module.scss | Added styling for debug panel container with fixed height |
| apps/src/weblab2/layout/VerticalLayout.tsx | Integrated DebugPanel component into layout |
| apps/src/weblab2/htmlPreview/HTMLPreview.tsx | Added clearRequests dispatch calls on refresh/reload |
| apps/src/weblab2/debugPanel/*.module.scss | Styling for debug panel components |
| apps/src/weblab2/debugPanel/images/*.svg | Visual indicators for request/response states |
| apps/src/weblab2/debugPanel/DebugPanel.tsx | Main debug panel component with request selection and details |
| apps/src/weblab2/debugPanel/DetailsBox.tsx | Component for displaying request/response details |
| apps/src/weblab2/debugPanel/NetworkRequestChip.tsx | Selectable chip component for each network request |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fisher-alice
left a comment
fisher-alice
left a comment
There was a problem hiding this comment.
Nice! Didn't realized how involved this debug panel was!
Left small non-blocking questions.
There was a problem hiding this comment.
Would it make sense to put these in apps/static?
There was a problem hiding this comment.
I don't have a strong opinion. They are super tiny files because they are svgs, and we have other image files in the codebridge folder alongside the code.
| }; | ||
|
|
||
| const requestSuccess = useMemo(() => { | ||
| if (selectedRequest) { |
There was a problem hiding this comment.
Just confirming that whether the api is in the allowlist or not will be the only determinant of a request success/failure?
There was a problem hiding this comment.
Yeah I think so. It will be if it failed due to the content security policy, which should generally be because it was not in the allow-list. If the url doesn't exist it would also fail here, because that non-existent url is not in the allow-list.
Other csp failures will end up here too, I'm not sure what else might get caught here. I'm starting with a generic message about the allow-list and we can get more complicated if we need to!
| value: selectedRequest?.response?.status, | ||
| }, | ||
| { | ||
| label: 'Time', |
There was a problem hiding this comment.
I see 'Request Time' vs 'Time', so I wonder if maybe using 'Duration' or 'Elapsed Time' instead of 'Time' would be more clear? Question for @moshebaricdo
There was a problem hiding this comment.
Let's discuss as a potential follow-up. This was the text from the mock.
There was a problem hiding this comment.
I think "duration" makes more sense here / is more descriptive
| if (selectedRequest?.request.cspDirectiveViolated) { | ||
| let requestDomain = selectedRequest.request.url; | ||
| try { | ||
| const url = new URL(selectedRequest.request.url); |
There was a problem hiding this comment.
So I think other possible request errors (network/dns failures, timeout, ...) will not be displayed as request errors and treated as response errors?
| <div>No network requests</div> | ||
| ) : ( | ||
| <div className={moduleStyles.debugPanelContainer}> | ||
| <div className={moduleStyles.networkSummary}> |
There was a problem hiding this comment.
Nit: subcomponent for NetworkSummary?
There was a problem hiding this comment.
I might look at this as a follow-up.
3 participants
This adds the initial implementation of the Web Lab 2 network tab. It is currently behind an experiment flag,
weblab2-show-debug, so I can iterate on it before releasing it widely. See follow-up work for what is not included in this PR from the mocks.Screenshots/Screen recording
Full page with panel with successful request
Failed response due to non-allowed url
Response failed on server
Currently placeholder for no requests
Screen recording
Screen.Recording.2026-02-13.at.9.53.26.AM.mov
Links
Testing story
Tested locally. This is behind a flag so there's minimal risk.
Follow-up work
Here is what is not yet included:
PR Creation Checklist: