Improve running cell UX and auto-scroll streaming output#8893
Improve running cell UX and auto-scroll streaming output#8893axsseldz wants to merge 1 commit intomarimo-team:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
62b6186 to
1251863
Compare
There was a problem hiding this comment.
Pull request overview
Improves the notebook editing experience by making running cells more visually identifiable and by automatically keeping streaming console output scrolled to the latest content during execution.
Changes:
- Add a running-state visual treatment for cells and a clearer “Running” status pill.
- Add auto-follow behavior for streaming console output while a cell is running (with user scroll override).
- Update/extend frontend tests and snapshots; minor cleanup in a Plotly example.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/css/app/Cell.css | Adds running-cell highlight styles/animation. |
| frontend/src/components/editor/cell/cell-status.css | Styles the running “pill” + pulse indicator. |
| frontend/src/components/editor/cell/CellStatus.tsx | Adds “Running” label next to the timer. |
| frontend/src/components/editor/notebook-cell.tsx | Applies running class to cells and passes running into ConsoleOutput. |
| frontend/src/components/editor/output/console/ConsoleOutput.tsx | Implements conditional auto-scroll-follow for streaming output while running. |
| frontend/src/components/editor/output/console/tests/ConsoleOutput.test.tsx | Adds tests around the new auto-scroll behavior. |
| frontend/src/components/editor/renderers/vertical-layout/vertical-layout.tsx | Passes the new running prop (currently hard-coded false). |
| frontend/src/components/scratchpad/scratchpad.tsx | Passes running state into ConsoleOutput for scratchpad. |
| frontend/src/tests/snapshots/CellStatus.test.tsx.snap | Updates snapshot for the new running label. |
| examples/third_party/plotly/histogram.py | Removes a stray line artifact. |
| const appendedOutput = | ||
| consoleOutputs.length > prevRenderedOutputCountRef.current; | ||
| prevRenderedOutputCountRef.current = consoleOutputs.length; | ||
|
|
||
| if (!running || !appendedOutput || !shouldFollowOutputRef.current) { | ||
| return; | ||
| } | ||
|
|
||
| el.scrollTop = el.scrollHeight - el.clientHeight; |
There was a problem hiding this comment.
Auto-scroll only triggers when consoleOutputs.length increases. Because collapseConsoleOutputs can merge streaming updates into an existing message (length stays the same while data grows/changes, e.g. carriage-return progress updates), the console may stop following output even though new content is rendered. Consider tracking a more reliable “new content rendered” signal (e.g., last output reference/timestamp or combined text length) instead of only the array length.
| <ConsoleOutput | ||
| consoleOutputs={consoleOutputs} | ||
| stale={outputStale} | ||
| running={false} |
There was a problem hiding this comment.
VerticalCell already has access to status, but ConsoleOutput is hard-coded with running={false}. This disables the new follow-streaming-output behavior in vertical layout even when a cell is actually running. Pass running={status === "running"} (or equivalent) so behavior is consistent across renderers.
| running={false} | |
| running={status === "running"} |
| it("follows newly appended output while running when already at the bottom", () => { | ||
| const { rerender } = renderWithProvider( | ||
| <ConsoleOutput | ||
| {...defaultProps} | ||
| consoleOutputs={[createOutput("line 1")]} | ||
| />, | ||
| ); | ||
|
|
||
| const consoleArea = screen.getByTestId("console-output-area"); | ||
| setScrollMetrics(consoleArea, { | ||
| clientHeight: 100, | ||
| scrollHeight: 300, | ||
| scrollTop: 200, | ||
| }); | ||
|
|
||
| rerender( | ||
| <TooltipProvider> | ||
| <ConsoleOutput | ||
| {...defaultProps} | ||
| consoleOutputs={[createOutput("line 1"), createOutput("line 2")]} | ||
| /> | ||
| </TooltipProvider>, | ||
| ); | ||
|
|
||
| expect(consoleArea.scrollTop).toBe(200); |
There was a problem hiding this comment.
The “follows newly appended output” test doesn't currently validate that scrolling happens: scrollHeight/clientHeight are kept constant across the rerender, so scrollTop would remain 200 even if the auto-scroll effect never ran. To make this test meaningful, simulate the content growth by increasing scrollHeight before rerender and assert that scrollTop updates to the new bottom offset.
|
thanks for sharing this design. i quite like the idea of a stronger indicator. if its ok with you, ill see if we can bring a designer in to iterate on the exact design for this. |
📝 Summary
Improve UX for running cells by making the active cell easier to identify and automatically following streaming output.
🔍 Changes
Runningstatus pillBefore
After
💬 Discussion
This PR is meant as a lightweight UX proposal to explore how running cells could be more clearly communicated without introducing major visual changes. The intention is to start a conversation and gather feedback on whether this direction feels natural within marimo’s design.
Open to suggestions on:
📋 Checklist
@mscolnick @nojaf