Conversation
…ate workspace layout
…out for running state
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29b897a155
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| executionState = inject(ExecutionStateService); | ||
|
|
||
| runCode(){ | ||
| this.executionState.setStatus('running') |
There was a problem hiding this comment.
Complete runs instead of leaving the status at running
Every Run Code click only sets the singleton status to running; the repository contains no execution call or subsequent path that sets success, error, or invokes reset(). Consequently, the results panel remains on its hard-coded evaluating branch indefinitely after the first click, and the user cannot receive a result without reloading the application.
Useful? React with 👍 / 👎.
| result = signal<ExecutionResult>({ | ||
| message: '', | ||
| output: '' | ||
| }); |
There was a problem hiding this comment.
Feed execution results into the panel
This creates private result state initialized with empty strings rather than an input or shared execution result. The container separately creates its own result signal but mounts <app-results-panel/> without binding it, so neither that initial message nor any future execution output can reach the displayed panel; the new spec's setInput('result', ...) also targets an input that does not exist.
Useful? React with 👍 / 👎.
|
|
||
| <div class="result-block"> | ||
| <span class="result-label">Expected</span> | ||
| <pre><code>{{ result().output ?? '—' }}</code></pre> |
There was a problem hiding this comment.
Render the expected value instead of duplicating the output
Whenever an execution produces an output, the Expected block reads the same result().output property as the Output block. A wrong answer will therefore be presented as matching the expected result, which makes the judge feedback inaccurate; the result model needs a distinct expected-value field and this block must render it.
Useful? React with 👍 / 👎.
| bottom: 56px; | ||
| left: 16px; | ||
| z-index: 1; | ||
| transform: translateY(100%) |
There was a problem hiding this comment.
Keep the results panel above the footer
The panel's bottom is already anchored 56px above the footer's bottom, but translating it downward by its full height moves its top to the footer and the rest below the viewport. Because it is absolutely positioned, it does not expand the grid row, so the initial 340px results panel—and the running view—appears below the workspace rather than above the action bar.
Useful? React with 👍 / 👎.
| } @else { | ||
|
|
||
| <section class="results-panel" [class.collapsed]="collapsed()" aria-label="Test results" aria-live="polite"> | ||
| <div class="results-tabs" (click)="togglepanel()" role="tablist" aria-label="Result mode"> |
There was a problem hiding this comment.
Implement tab selection instead of collapsing on every click
Clicking either tab bubbles to this tablist handler and only toggles the panel's collapsed state; there is no selected-tab state or Custom Input content, while aria-selected remains permanently hard-coded to Test Results. Thus the visible Custom Input tab cannot perform its advertised action and exposes incorrect tab semantics to assistive technology.
AGENTS.md reference: AGENTS.md:L103-L105
Useful? React with 👍 / 👎.
| <div class="result-block result-input"> | ||
| <div class="result-label-row"> | ||
| <span>Input</span> | ||
| <button class="copy-button" type="button" aria-label="Copy input"> |
There was a problem hiding this comment.
Wire the copy button to the clipboard
The newly rendered Copy input button has no click binding, and a repository-wide search finds no clipboard operation or delegated handler. Therefore, activating this visible control has no effect for every user; it should copy the displayed input or be removed until that behavior exists.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,104 @@ | |||
| @if (executionState.status() === 'running') { | |||
|
|
|||
| <section class="results-panel-running"> | |||
There was a problem hiding this comment.
Announce the transition to the running state
When Run Code is activated, Angular removes the existing section that owns aria-live="polite" and replaces it with this section, which has neither live-region semantics nor a status role. Screen-reader users consequently receive no indication that evaluation began or is still running; preserve the live region across the state change or mark this status content appropriately.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
| overflow: hidden; | ||
| height: 340px; |
There was a problem hiding this comment.
Let stacked mobile results scroll within the panel
At widths up to 560px, the responsive rule stacks Output and Expected into separate rows, but this container remains fixed at 340px and clips overflow. The test-case list, input block, and two stacked value blocks exceed that height even with short values, leaving the lower result—typically Expected—inaccessible on mobile; allow the content area to scroll or let the expanded panel grow.
Useful? React with 👍 / 👎.
|
|
||
| <div class="results-content"> | ||
| <div class="test-cases" aria-label="Test cases"> | ||
| <button class="test-case test-case-passed" type="button"> |
There was a problem hiding this comment.
Make enabled test-case controls perform an action
Cases 1 through 3 are rendered as enabled buttons, but none has a click binding and the component has no selected-case state. Keyboard and pointer users can activate these controls without any change to the displayed input or output; either implement case selection or render them as noninteractive status elements instead.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
| <div class="results-content"> | ||
| <div class="test-cases" aria-label="Test cases"> | ||
| <button class="test-case test-case-passed" type="button"> | ||
| <span aria-hidden="true"></span>Case 1 |
There was a problem hiding this comment.
Expose each test case's pass or failure status
The accessible names of these case buttons contain only Case 1, Case 2, and Case 3; pass and failure are represented solely through CSS classes and an aria-hidden colored dot. A screen-reader user therefore cannot determine which cases passed or failed, so include the status in visible or accessible text whenever it changes.
AGENTS.md reference: AGENTS.md:L104-L104
Useful? React with 👍 / 👎.
No description provided.