Add a "Stop" (⏹️) button to interrupt cell execution #212
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a stop button functionality to interrupt cell execution in Jupyter notebooks. The stop button appears during cell execution and replaces the run button to allow users to interrupt running cells.
Key changes:
- Added CSS styling for stop button with proper visibility states during execution
- Implemented stop button logic with kernel interruption and fallback to restart
- Added stop cell icon to the icon registry
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| style/base.css | Added CSS rules for stop button styling and visibility states during execution |
| src/run-button.ts | Implemented stop button functionality with execution state tracking and kernel interruption |
| src/icons.ts | Added stop cell icon import and registration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (kernel && typeof kernel.interrupt === 'function') { | ||
| await kernel.interrupt(); | ||
| } else { | ||
| await panel.sessionContext.restartKernel(); |
There was a problem hiding this comment.
Falling back to kernel restart when interrupt is unavailable may be too aggressive and could cause data loss. Consider providing user confirmation before restarting the kernel, or implement a more graceful degradation strategy.
There was a problem hiding this comment.
Yes, makes sense and I'm aware – I've added because interrupts don't really work.
| } catch (err) { | ||
| console.warn('Failed to stop execution (interrupt/restart):', err); | ||
| } |
There was a problem hiding this comment.
Error handling silently logs to console without user feedback. Users should be notified when stop operation fails so they understand the cell may still be running.
| if (isExecuting) { | ||
| runEl.style.pointerEvents = 'none'; | ||
| stopEl.style.pointerEvents = 'auto'; | ||
| } else { | ||
| runEl.style.pointerEvents = 'auto'; | ||
| stopEl.style.pointerEvents = 'none'; |
There was a problem hiding this comment.
Direct DOM style manipulation should be avoided in favor of CSS classes. The CSS already includes pointer-events rules that could be toggled with classes instead of inline styles.
| if (isExecuting) { | |
| runEl.style.pointerEvents = 'none'; | |
| stopEl.style.pointerEvents = 'auto'; | |
| } else { | |
| runEl.style.pointerEvents = 'auto'; | |
| stopEl.style.pointerEvents = 'none'; | |
| // Remove both classes first to ensure a clean state | |
| runEl.classList.remove('je-pointer-events-none', 'je-pointer-events-auto'); | |
| stopEl.classList.remove('je-pointer-events-none', 'je-pointer-events-auto'); | |
| if (isExecuting) { | |
| runEl.classList.add('je-pointer-events-none'); | |
| stopEl.classList.add('je-pointer-events-auto'); | |
| } else { | |
| runEl.classList.add('je-pointer-events-auto'); | |
| stopEl.classList.add('je-pointer-events-none'); |
1 participant
This will remain a draft until we have support for complete kernel interruption (we only have partial interruption via #198 as of now). However, the button works well and is styled correctly; suggestions on the code so far are still welcome (not sure if I have the best approach)! The idea is to display the "Stop" button for cells that make the kernel busy, replacing the "Run" button established in #205.
This PR follows on the Google Colab-style buttons being introduced via upstream PRs jupyterlab/jupyterlab#16602 and jupyterlab/jupyterlab#17775