Add pinned batch task toolbar shortcuts and device panel resizer #10
Conversation
…tor, added draggable resizing, adjusted default panel widths
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds persisted “pinned” batch task shortcuts to the main CC-rClient toolbar and enables opening the existing batch runner from any page, with supporting UI/UX updates and tests.
Changes:
- Introduces
showInToolbar/show_in_toolbarpersisted flag for batch tasks (frontend + Tauri backend) and pin/unpin controls in the batch task list/editor. - Displays pinned batch tasks as toolbar buttons in the main App toolbar and routes clicks to open the batch runner overlay.
- Adds frontend tests for pinning behavior and app-level toolbar integration; updates docs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates product-level feature documentation to mention toolbar pinning for batch tasks. |
| CC-rClient/README.md | Adds CC-rClient feature bullet documenting toolbar shortcuts for batch tasks. |
| CC-rClient/src/plugin/batch/types.ts | Adds showInToolbar to the BatchTask type. |
| CC-rClient/src/plugin/batch/BatchEditor.tsx | Adds an editor checkbox and includes showInToolbar in save payloads. |
| CC-rClient/src/plugin/batch/BatchList.tsx | Adds toolbar pin/unpin button and wiring for onToggleToolbarPin. |
| CC-rClient/src/plugin/batch/BatchPage.tsx | Wires toolbar pin toggling; removes in-page runner rendering in favor of global runner. |
| CC-rClient/src/plugin/batch/BatchContext.tsx | Changes task upsert logic to preserve ordering when updating existing tasks. |
| CC-rClient/src/App.tsx | Shows pinned batch task shortcuts in main toolbar and centralizes runner overlay; also adds station panel resizer + persistence. |
| CC-rClient/src/styles.css | Adds styles for toolbar shortcuts, runner overlay, and station panel resizer/layout tweaks. |
| CC-rClient/src/test/batch-toolbar.test.tsx | Adds component-level tests for pin toggle and save payload. |
| CC-rClient/src/test/app-batch-toolbar.test.tsx | Adds app-level test ensuring pinned tasks render in toolbar and open runner. |
| CC-rClient/src-tauri/src/batch_ops.rs | Persists show_in_toolbar, normalizes it, and adds Rust unit tests for defaults/normalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const STATION_PANEL_WIDTH_STORAGE_KEY = "cc-rclient.station-panel-width"; | ||
|
|
||
| const MIN_BROWSER_PANEL_WIDTH = 320; | ||
| const MIN_DETAIL_PANEL_WIDTH = 360; | ||
| const STATIONS_PANEL_RESIZER_WIDTH = 10; | ||
| const STATIONS_STACK_BREAKPOINT = 1180; |
There was a problem hiding this comment.
This PR introduces a persisted station panel resizer (new storage key, sizing constants, pointer/keyboard handlers, and CSS layout changes), but the PR title/description are focused on pinned batch task toolbar shortcuts. Please either (mandatory) update the PR description to explicitly include the resizable stations workspace change (including persistence + breakpoint behavior), or (preferred) split the resizer work into a separate PR to keep review scope and regression surface smaller.
| {runner.open && runner.task && ( | ||
| <div className="batch-runner-layer" role="dialog" aria-modal="true"> | ||
| <div className="batch-runner-panel"> | ||
| <BatchTaskRunner | ||
| task={runner.task} | ||
| targets={batchTargets} | ||
| onExecute={handleExecuteBatchTask} | ||
| onCancel={closeRunner} | ||
| /> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
The dialog container has role=\"dialog\" aria-modal=\"true\" but no accessible name (aria-label or aria-labelledby). This makes the modal harder to identify for assistive tech. Add an accessible name (e.g., aria-labelledby pointing at the runner’s title heading, or an aria-label), and consider putting role=\"dialog\" on .batch-runner-panel (the actual dialog surface) instead of the full-screen backdrop element.
| {toolbarBatchTasks.length > 0 && ( | ||
| <> | ||
| <div className="toolbar-divider" /> | ||
| <div className="toolbar-batch-shortcuts" aria-label="Pinned batch tasks"> | ||
| {toolbarBatchTasks.map((task) => ( | ||
| <button | ||
| key={task.id} | ||
| className="toolbar-batch-shortcut" | ||
| onClick={() => openRunner(task)} | ||
| title={`Open batch task: ${task.name}`} | ||
| > | ||
| {task.name} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Using aria-label on a plain <div> is not reliably announced unless the element has an appropriate role. If this is meant to be a labeled control group, add role=\"group\" (or role=\"toolbar\" if it behaves like a toolbar region) so the label is meaningful to screen readers.
| <div | ||
| className={`stationsPanelResizer ${isResizingPanels ? "isDragging" : ""}`} | ||
| role="separator" | ||
| aria-orientation="vertical" | ||
| aria-label="Resize device and detail panels" | ||
| aria-valuemin={MIN_BROWSER_PANEL_WIDTH} | ||
| aria-valuemax={2000} | ||
| aria-valuenow={stationPanelWidth ?? undefined} | ||
| tabIndex={isStationsStacked ? -1 : 0} | ||
| onPointerDown={startPanelResize} | ||
| onKeyDown={(event) => { | ||
| if (event.key === "ArrowLeft") { | ||
| event.preventDefault(); | ||
| resizePanelsWithKeyboard(-24); | ||
| } else if (event.key === "ArrowRight") { | ||
| event.preventDefault(); | ||
| resizePanelsWithKeyboard(24); | ||
| } | ||
| }} | ||
| /> |
There was a problem hiding this comment.
aria-valuemax={2000} is a hard-coded value that likely won’t match the real maximum (which is calculated from container width and min panel sizes elsewhere). For role=\"separator\" with aria-valuenow, aria-valuemin/max should reflect the true bounds. Suggest deriving aria-valuemax from the same maxLeft calculation (store in state on resize/start-drag) or omit aria-valuemax/aria-valuenow if you can’t keep them accurate.
| export default function App() { | ||
| return ( | ||
| <BatchProvider> | ||
| <BatchUIProvider> | ||
| <AppShell /> | ||
| </BatchUIProvider> | ||
| </BatchProvider> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Moving BatchProvider to wrap the entire app means its initialization effects (notably loadTargets() in BatchProvider) will run on every app startup, even if a user never opens the batch runner/page. Since AppShell already derives batchTargets from stations for the runner, consider deferring target loading (e.g., only load targets when the runner opens, or make BatchProvider optionally skip loadTargets() until explicitly requested) to reduce startup IPC and improve perceived load time.
| const stationsWorkspaceColumns = (() => { | ||
| if (isStationsStacked) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const leftTrack = | ||
| stationPanelWidth !== null | ||
| ? `${stationPanelWidth}px` | ||
| : viewMode === "split" | ||
| ? "minmax(0, 0.9fr)" | ||
| : "minmax(0, 0.95fr)"; | ||
| const rightTrack = viewMode === "split" ? "minmax(360px, 1.2fr)" : "minmax(360px, 1.3fr)"; | ||
| return `${leftTrack} ${STATIONS_PANEL_RESIZER_WIDTH}px ${rightTrack}`; | ||
| })(); | ||
|
|
||
| function startPanelResize(event: React.PointerEvent<HTMLDivElement>) { | ||
| if (isStationsStacked || !stationsWorkspaceRef.current) { | ||
| return; | ||
| } | ||
|
|
||
| event.preventDefault(); | ||
| const rect = stationsWorkspaceRef.current.getBoundingClientRect(); | ||
| const maxLeft = Math.max( | ||
| MIN_BROWSER_PANEL_WIDTH, | ||
| rect.width - MIN_DETAIL_PANEL_WIDTH - STATIONS_PANEL_RESIZER_WIDTH, | ||
| ); | ||
| const next = Math.min(Math.max(event.clientX - rect.left, MIN_BROWSER_PANEL_WIDTH), maxLeft); | ||
| setStationPanelWidth(Math.round(next)); | ||
| resizingPanelsRef.current = true; | ||
| setIsResizingPanels(true); | ||
| } |
There was a problem hiding this comment.
The stations panel resizing feature adds non-trivial behavior (persisted width via localStorage, pointer drag resizing, keyboard resizing, and breakpoint stacking logic) but there’s no corresponding frontend coverage added here. Since the repo already uses Vitest + RTL for app behavior, add tests that validate: (1) stored width is applied on load, (2) dragging updates gridTemplateColumns and persists, (3) ArrowLeft/ArrowRight on the separator updates width, and (4) resizing the viewport across STATIONS_STACK_BREAKPOINT hides/disables the resizer and clamps persisted widths.
| tasks: (() => { | ||
| const index = prev.tasks.findIndex(t => t.id === result.id); | ||
| if (index === -1) { | ||
| return prev.tasks.concat(result); | ||
| } | ||
|
|
||
| const next = [...prev.tasks]; | ||
| next[index] = result; | ||
| return next; | ||
| })(), |
There was a problem hiding this comment.
The inline IIFE inside setState makes the update harder to scan and reuse. Consider extracting an upsertTaskById(prev.tasks, result) helper (module-level function) or inlining without an IIFE (e.g., compute nextTasks above the return) so future updates (like sorting/filtering changes) remain easier to reason about.
|
@copilot apply changes based on the comments in this thread |
Summary
Testing