Add process detail panel with signal controls#29
Conversation
Adds a new ProcessDetailPanel component that appears on double-clicking a process row. Shows detailed process info, allows sending signals, and supports saving notes per process. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughIntroduces a ProcessDetailPanel component that displays detailed process information in an overlay modal triggered by double-clicking process rows in the ProcessTable. Includes CSS styling, localStorage-based note persistence, and API integration for sending signals to processes. Changes
Sequence DiagramsequenceDiagram
actor User
participant ProcessTable
participant ProcessDetailPanel
participant API
participant localStorage
User->>ProcessTable: Double-click process row
activate ProcessTable
ProcessTable->>ProcessTable: Set detailProcess state
ProcessTable->>ProcessDetailPanel: Render with process data
deactivate ProcessTable
activate ProcessDetailPanel
ProcessDetailPanel-->>User: Display process details
alt User sends signal
User->>ProcessDetailPanel: Enter signal and click Send
ProcessDetailPanel->>API: POST /api/kill with PID and signal
activate API
API-->>ProcessDetailPanel: Return success/error
deactivate API
ProcessDetailPanel-->>User: Display result message
end
alt User saves notes
User->>ProcessDetailPanel: Enter notes and click Save
ProcessDetailPanel->>localStorage: Store notes for PID
localStorage-->>ProcessDetailPanel: Confirm storage
ProcessDetailPanel-->>User: Show confirmation
end
User->>ProcessDetailPanel: Click close button or overlay
ProcessDetailPanel->>ProcessTable: Trigger onClose
deactivate ProcessDetailPanel
activate ProcessTable
ProcessTable->>ProcessTable: Clear detailProcess state
deactivate ProcessTable
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ProcessTable.tsx (1)
140-145:⚠️ Potential issue | 🟠 MajorDetail panel open action is mouse-only (keyboard users are blocked).
The new feature is bound to
onDoubleClickon a non-semantic<div>, so keyboard users cannot trigger it. Please add keyboard access (tabIndex,role,onKeyDown) for parity.Suggested fix
<div key={process.pid} className={`table-row ${selectedPid === process.pid ? 'selected' : ''}`} onClick={() => setSelectedPid(process.pid === selectedPid ? null : process.pid)} + role="button" + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setDetailProcess(process); + } + }} onDoubleClick={() => setDetailProcess(process)} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ProcessTable.tsx` around lines 140 - 145, The row DIV in ProcessTable (the element using className "table-row" and handlers setSelectedPid / setDetailProcess) is only mouse-accessible; add keyboard accessibility by making the element focusable (tabIndex={0}), give it an appropriate role (e.g., role="button"), and implement an onKeyDown handler that invokes setDetailProcess(process) when Enter or Space is pressed (and mirrors the click behavior for toggling selection with setSelectedPid as needed), ensuring you prevent default for Space to avoid page scroll; update the element that currently has onClick/onDoubleClick to use these keyboard handlers so keyboard users can open the detail panel.
🧹 Nitpick comments (1)
src/components/ProcessDetailPanel.tsx (1)
46-51: Add dialog semantics for assistive technologies.This modal should expose dialog semantics (
role="dialog",aria-modal, labelled title) for screen-reader navigation.Suggested fix
- <div className="process-detail-overlay" onClick={onClose}> - <div className="process-detail-panel" onClick={(e) => e.stopPropagation()}> + <div className="process-detail-overlay" onClick={onClose}> + <div + className="process-detail-panel" + role="dialog" + aria-modal="true" + aria-labelledby="process-detail-title" + onClick={(e) => e.stopPropagation()} + > <div className="detail-header"> - <h3>Process Details</h3> + <h3 id="process-detail-title">Process Details</h3> <button className="detail-close" onClick={onClose}>✕</button> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ProcessDetailPanel.tsx` around lines 46 - 51, The modal markup in ProcessDetailPanel lacks ARIA dialog semantics; update the root panel element (the div with className "process-detail-panel" inside the overlay) to include role="dialog", aria-modal="true" and aria-labelledby pointing to the title element; give the <h3> "Process Details" a stable id (e.g. process-detail-title) and reference that id from aria-labelledby so screen readers can identify the dialog title (keep existing onClose handlers and stopPropagation logic unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.css`:
- Around line 563-565: The CSS rule uses a fixed width (width: 480px) which can
overflow on narrow viewports; change it to a responsive pattern (e.g., replace
the fixed width with a fluid width and a cap such as width: 100% combined with
max-width: 480px, or use clamp() to enforce a min/ideal/max) so the panel
shrinks on small screens while keeping the 480px cap; update the block
containing the width: 480px declaration in src/App.css accordingly.
In `@src/components/ProcessDetailPanel.tsx`:
- Around line 14-15: The component currently writes notes to localStorage but
never hydrates the notes state; update ProcessDetailPanel to load saved notes
into the notes useState whenever a process is opened (e.g., in an effect that
depends on the active process id or the panel open flag) by reading the same
storage key you use when saving, parsing it with JSON.parse inside a try/catch
to guard against corrupted data, and falling back to '' (or an empty object) on
error; ensure you call setNotes(...) with the loaded value and clear or reset
corrupted storage if parsing fails. Reference: notes, setNotes, and the
component mount/open handler or useEffect in ProcessDetailPanel.
- Line 9: The code hardcodes API_URL and injects raw signal text into the query
string; change to read the base URL from an environment-configured variable (use
the app's env mechanism, e.g., process.env.REACT_APP_API_URL or
import.meta.env.VITE_API_URL) instead of the literal API_URL constant, and
ensure the kill request builds the URL safely by using URL/URLSearchParams or
encodeURIComponent for the signal parameter (e.g., construct
`${baseUrl}/api/kill?signal=${encodeURIComponent(signal)}` or use new URL(...)
and url.searchParams.set('signal', signal)); update the place that calls the
kill endpoint (references to API_URL and the kill fetch/axios call in
ProcessDetailPanel.tsx) to use the env variable and URL-encoding so deployments
and special characters work correctly.
- Around line 95-98: Replace the unsafe HTML rendering: in ProcessDetailPanel
(the div with className "detail-command" that currently uses
dangerouslySetInnerHTML with process.command) stop using dangerouslySetInnerHTML
and render process.command as plain text (e.g., inject the string as a child/JSX
expression) so the command is escaped and not parsed as HTML; match the approach
used in ProcessTable.tsx to eliminate the XSS risk.
---
Outside diff comments:
In `@src/components/ProcessTable.tsx`:
- Around line 140-145: The row DIV in ProcessTable (the element using className
"table-row" and handlers setSelectedPid / setDetailProcess) is only
mouse-accessible; add keyboard accessibility by making the element focusable
(tabIndex={0}), give it an appropriate role (e.g., role="button"), and implement
an onKeyDown handler that invokes setDetailProcess(process) when Enter or Space
is pressed (and mirrors the click behavior for toggling selection with
setSelectedPid as needed), ensuring you prevent default for Space to avoid page
scroll; update the element that currently has onClick/onDoubleClick to use these
keyboard handlers so keyboard users can open the detail panel.
---
Nitpick comments:
In `@src/components/ProcessDetailPanel.tsx`:
- Around line 46-51: The modal markup in ProcessDetailPanel lacks ARIA dialog
semantics; update the root panel element (the div with className
"process-detail-panel" inside the overlay) to include role="dialog",
aria-modal="true" and aria-labelledby pointing to the title element; give the
<h3> "Process Details" a stable id (e.g. process-detail-title) and reference
that id from aria-labelledby so screen readers can identify the dialog title
(keep existing onClose handlers and stopPropagation logic unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3fba4ff-6d54-475c-b8fb-b0b9462733c5
📒 Files selected for processing (3)
src/App.csssrc/components/ProcessDetailPanel.tsxsrc/components/ProcessTable.tsx
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.41.1)
src/components/ProcessDetailPanel.tsx
[warning] 96-96: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
| width: 480px; | ||
| max-height: 80vh; | ||
| overflow-y: auto; |
There was a problem hiding this comment.
Panel width can overflow on narrow screens.
Using a fixed width: 480px risks clipping on smaller viewports. Prefer a responsive width cap.
Suggested fix
.process-detail-panel {
background: linear-gradient(145deg, `#141c28` 0%, `#0d1219` 100%);
border: 1px solid var(--border-color);
border-radius: 16px;
- width: 480px;
+ width: min(480px, calc(100vw - 24px));
max-height: 80vh;
overflow-y: auto;
box-shadow: 0 8px 40px rgba(0, 0, 0, 0.6), 0 0 30px rgba(34, 211, 238, 0.1);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| width: 480px; | |
| max-height: 80vh; | |
| overflow-y: auto; | |
| width: min(480px, calc(100vw - 24px)); | |
| max-height: 80vh; | |
| overflow-y: auto; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.css` around lines 563 - 565, The CSS rule uses a fixed width (width:
480px) which can overflow on narrow viewports; change it to a responsive pattern
(e.g., replace the fixed width with a fluid width and a cap such as width: 100%
combined with max-width: 480px, or use clamp() to enforce a min/ideal/max) so
the panel shrinks on small screens while keeping the 480px cap; update the block
containing the width: 480px declaration in src/App.css accordingly.
| onClose: () => void; | ||
| } | ||
|
|
||
| const API_URL = 'http://localhost:3001/api'; |
There was a problem hiding this comment.
Kill API call is environment-bound and signal value is not URL-safe.
Hardcoding http://localhost:3001 breaks non-local deployments, and interpolating raw signal into the query string can mis-handle valid values (e.g., +).
Suggested fix
-const API_URL = 'http://localhost:3001/api';
+const API_URL = '/api';
@@
- const response = await fetch(`${API_URL}/process/${process.pid}/kill?signal=${signal}`, {
+ const signalValue = signal.trim();
+ if (!signalValue) {
+ setKillResult('Signal is required');
+ return;
+ }
+ const params = new URLSearchParams({ signal: signalValue });
+ const response = await fetch(`${API_URL}/process/${process.pid}/kill?${params.toString()}`, {
method: 'POST',
});
+ if (!response.ok) {
+ throw new Error(`HTTP ${response.status}`);
+ }Also applies to: 18-23
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ProcessDetailPanel.tsx` at line 9, The code hardcodes API_URL
and injects raw signal text into the query string; change to read the base URL
from an environment-configured variable (use the app's env mechanism, e.g.,
process.env.REACT_APP_API_URL or import.meta.env.VITE_API_URL) instead of the
literal API_URL constant, and ensure the kill request builds the URL safely by
using URL/URLSearchParams or encodeURIComponent for the signal parameter (e.g.,
construct `${baseUrl}/api/kill?signal=${encodeURIComponent(signal)}` or use new
URL(...) and url.searchParams.set('signal', signal)); update the place that
calls the kill endpoint (references to API_URL and the kill fetch/axios call in
ProcessDetailPanel.tsx) to use the env variable and URL-encoding so deployments
and special characters work correctly.
| const [notes, setNotes] = useState(''); | ||
|
|
There was a problem hiding this comment.
Saved notes are never loaded back, so “retrieve notes” is incomplete.
The component writes to localStorage but does not hydrate notes when opening a process. Also, JSON.parse is unguarded and can crash the panel on corrupted storage.
Suggested fix
-import { useState } from 'react';
+import { useEffect, useState } from 'react';
import type { ProcessInfo } from '../types';
@@
const API_URL = 'http://localhost:3001/api';
+const NOTES_KEY = 'process_notes';
@@
const [signal, setSignal] = useState('SIGTERM');
const [killResult, setKillResult] = useState<string | null>(null);
const [notes, setNotes] = useState('');
+
+ const readNotes = (): Record<string, { notes?: string; command?: string; savedAt?: string }> => {
+ try {
+ return JSON.parse(localStorage.getItem(NOTES_KEY) || '{}');
+ } catch {
+ return {};
+ }
+ };
+
+ useEffect(() => {
+ const allNotes = readNotes();
+ setNotes(allNotes[String(process.pid)]?.notes || '');
+ }, [process.pid]);
@@
const handleSaveNotes = () => {
- const allNotes = JSON.parse(localStorage.getItem('process_notes') || '{}');
+ const allNotes = readNotes();
allNotes[process.pid] = {
notes,
command: process.command,
savedAt: new Date().toISOString(),
};
- localStorage.setItem('process_notes', JSON.stringify(allNotes));
+ localStorage.setItem(NOTES_KEY, JSON.stringify(allNotes));
};Also applies to: 28-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ProcessDetailPanel.tsx` around lines 14 - 15, The component
currently writes notes to localStorage but never hydrates the notes state;
update ProcessDetailPanel to load saved notes into the notes useState whenever a
process is opened (e.g., in an effect that depends on the active process id or
the panel open flag) by reading the same storage key you use when saving,
parsing it with JSON.parse inside a try/catch to guard against corrupted data,
and falling back to '' (or an empty object) on error; ensure you call
setNotes(...) with the loaded value and clear or reset corrupted storage if
parsing fails. Reference: notes, setNotes, and the component mount/open handler
or useEffect in ProcessDetailPanel.
| <div | ||
| className="detail-command" | ||
| dangerouslySetInnerHTML={{ __html: process.command }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "ProcessDetailPanel.tsx"Repository: coderabbit-demo/btop
Length of output: 101
🏁 Script executed:
wc -l src/components/ProcessDetailPanel.tsxRepository: coderabbit-demo/btop
Length of output: 105
🏁 Script executed:
cat -n src/components/ProcessDetailPanel.tsx | sed -n '85,105p'Repository: coderabbit-demo/btop
Length of output: 951
🏁 Script executed:
cat -n src/components/ProcessDetailPanel.tsx | head -50Repository: coderabbit-demo/btop
Length of output: 2059
🏁 Script executed:
cat -n src/types.tsxRepository: coderabbit-demo/btop
Length of output: 109
🏁 Script executed:
fd -t f "types\." src/Repository: coderabbit-demo/btop
Length of output: 76
🏁 Script executed:
cat -n src/types.tsRepository: coderabbit-demo/btop
Length of output: 1139
🏁 Script executed:
rg "ProcessInfo\|process.command" src/ -A 2 -B 2Repository: coderabbit-demo/btop
Length of output: 46
🏁 Script executed:
rg "process\.command" src/ -B 3 -A 3Repository: coderabbit-demo/btop
Length of output: 1615
🏁 Script executed:
rg "processes" src/ -B 2 -A 2 | grep -E "(fetch|api|endpoint|ProcessInfo)" -B 2 -A 2Repository: coderabbit-demo/btop
Length of output: 484
🏁 Script executed:
rg "fetch.*api" src/ -B 2 -A 5Repository: coderabbit-demo/btop
Length of output: 676
🏁 Script executed:
rg "API_URL\|/api" src/ -B 1 -A 3Repository: coderabbit-demo/btop
Length of output: 46
🏁 Script executed:
rg "fetch.*process\|SystemMetrics" src/ -B 2 -A 5Repository: coderabbit-demo/btop
Length of output: 46
🏁 Script executed:
fd -t f "App.tsx\|main.tsx\|index.tsx" src/Repository: coderabbit-demo/btop
Length of output: 46
🏁 Script executed:
ls -la src/Repository: coderabbit-demo/btop
Length of output: 645
🏁 Script executed:
cat -n src/App.tsxRepository: coderabbit-demo/btop
Length of output: 3183
Remove dangerouslySetInnerHTML and render command as plain text.
The process.command field is rendered unsanitized as HTML through dangerouslySetInnerHTML, creating an XSS vulnerability. This is inconsistent with ProcessTable.tsx, which renders the same field as plain text. Command strings should always be rendered as text.
Suggested fix
<div className="detail-section">
<span className="detail-label">Command</span>
- <div
- className="detail-command"
- dangerouslySetInnerHTML={{ __html: process.command }}
- />
+ <div className="detail-command">{process.command}</div>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| className="detail-command" | |
| dangerouslySetInnerHTML={{ __html: process.command }} | |
| /> | |
| <div className="detail-command">{process.command}</div> |
🧰 Tools
🪛 ast-grep (0.41.1)
[warning] 96-96: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ProcessDetailPanel.tsx` around lines 95 - 98, Replace the
unsafe HTML rendering: in ProcessDetailPanel (the div with className
"detail-command" that currently uses dangerouslySetInnerHTML with
process.command) stop using dangerouslySetInnerHTML and render process.command
as plain text (e.g., inject the string as a child/JSX expression) so the command
is escaped and not parsed as HTML; match the approach used in ProcessTable.tsx
to eliminate the XSS risk.
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
1 participant
Summary
ProcessDetailPanelcomponent that opens on double-clicking a process rowTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit