feat: tab description editing in overlay menu#472
feat: tab description editing in overlay menu#472felipeggv wants to merge 7 commits intoRunMaestro:mainfrom
Conversation
Added optional `description?: string` to AITab for user-defined tab context annotations. This is the data model foundation for the tab description feature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Handlers Adds a new handler for updating AI tab descriptions with trim and empty-string-to-undefined normalization. Follows existing immutable state update pattern via useSessionStore. Includes 4 tests covering set, trim, empty, and whitespace-only cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… TabBar Thread the tab description handler from useTabHandlers through useMainPanelProps → App.tsx → MainPanel → TabBar. Gate with encoreFeatures.tabDescription flag (default: false). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a description section to the AI tab hover overlay menu with two modes: - Display mode: FileText icon + description text (2-line clamp) or italic placeholder - Edit mode: auto-focus textarea with Enter to save, Shift+Enter for newline, Escape to cancel Feature-gated behind onUpdateTabDescription prop. Only calls handler when value actually changed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…edit Add cleanup logic so that if the overlay closes while the user is editing a tab description, the current draft is saved rather than discarded. Uses a useRef to track the latest draft value, avoiding stale closure issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n tab overlay Adds requestAnimationFrame-based focus management so that when the user saves or cancels description editing, focus returns to the description display button for keyboard accessibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds 2 missing useTabHandlers tests (non-active session isolation, other tabs isolation) and 7 TabBar description UI tests covering rendering, edit mode, Enter/Escape behavior, and feature gating. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR introduces a feature for editing and persisting per-tab descriptions. It adds type definitions for the description field on tabs, a feature flag to control availability, a new handler to update descriptions, UI components for inline editing with keyboard controls, prop threading through the component hierarchy, and comprehensive test coverage. Changes
Sequence DiagramsequenceDiagram
actor User
participant TabBar as TabBar<br/>(Component)
participant useTabHandlers as useTabHandlers<br/>(Hook)
participant AITab as AITab<br/>(State)
User->>TabBar: Click description area
activate TabBar
TabBar->>TabBar: Enter edit mode<br/>(isEditingDescription=true)
TabBar->>TabBar: Show textarea with<br/>current description
deactivate TabBar
User->>TabBar: Type description + Enter
activate TabBar
TabBar->>TabBar: Trim description
TabBar->>useTabHandlers: onUpdateTabDescription(tabId,<br/>trimmedDescription)
deactivate TabBar
activate useTabHandlers
useTabHandlers->>AITab: Update description on<br/>active session's tab
deactivate useTabHandlers
activate TabBar
TabBar->>TabBar: Sync draft with<br/>new tab.description
TabBar->>TabBar: Exit edit mode
deactivate TabBar
TabBar-->>User: Display updated description
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds user-editable descriptions to AI tabs, accessible through the tab hover overlay menu. The implementation is clean and well-tested: Implementation highlights:
Code quality:
Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 21bcf98 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/components/TabBar.tsx (1)
763-765: Avoid callback-ref autofocus on every render.
ref={(el) => el?.focus()}is recreated each render and can cause repeated refocus. Prefer a stable ref +useEffectkeyed byisEditingDescription.♻️ Proposed refactor
+const descriptionInputRef = useRef<HTMLTextAreaElement>(null); +useEffect(() => { + if (isEditingDescription) { + descriptionInputRef.current?.focus(); + } +}, [isEditingDescription]); -<textarea - ref={(el) => el?.focus()} +<textarea + ref={descriptionInputRef} value={descriptionDraft}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/TabBar.tsx` around lines 763 - 765, The textarea currently uses an inline callback ref ref={(el) => el?.focus()} which is recreated each render and can cause repeated refocus; replace it with a stable React ref (e.g., const descriptionRef = useRef<HTMLTextAreaElement | null>(null)) and remove the inline callback from the JSX (keep value={descriptionDraft}), then add a useEffect that watches isEditingDescription and, when true, calls descriptionRef.current?.focus(); ensure to clear or guard focus logic so it only runs when entering edit mode. Reference the textarea, descriptionDraft, isEditingDescription, and useEffect to locate where to apply the change.
🤖 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/renderer/components/TabBar.tsx`:
- Around line 490-500: The Enter/Escape handlers in handleDescriptionKeyDown
trigger save/cancel even during IME composition; update handleDescriptionKeyDown
to first check for IME composition (e.nativeEvent.isComposing) and do nothing if
composing, then proceed to preventDefault and call
handleDescriptionSave(descriptionDraft) or handleDescriptionCancel() only when
not composing; this ensures keys are ignored while composing and maintains
existing behavior otherwise.
---
Nitpick comments:
In `@src/renderer/components/TabBar.tsx`:
- Around line 763-765: The textarea currently uses an inline callback ref
ref={(el) => el?.focus()} which is recreated each render and can cause repeated
refocus; replace it with a stable React ref (e.g., const descriptionRef =
useRef<HTMLTextAreaElement | null>(null)) and remove the inline callback from
the JSX (keep value={descriptionDraft}), then add a useEffect that watches
isEditingDescription and, when true, calls descriptionRef.current?.focus();
ensure to clear or guard focus logic so it only runs when entering edit mode.
Reference the textarea, descriptionDraft, isEditingDescription, and useEffect to
locate where to apply the change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/__tests__/renderer/components/TabBar.test.tsxsrc/__tests__/renderer/hooks/useTabHandlers.test.tssrc/renderer/App.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.ts
| const handleDescriptionKeyDown = useCallback( | ||
| (e: React.KeyboardEvent<HTMLTextAreaElement>) => { | ||
| if (e.key === 'Enter' && !e.shiftKey) { | ||
| e.preventDefault(); | ||
| handleDescriptionSave(descriptionDraft); | ||
| } else if (e.key === 'Escape') { | ||
| e.preventDefault(); | ||
| handleDescriptionCancel(); | ||
| } | ||
| }, | ||
| [descriptionDraft, handleDescriptionSave, handleDescriptionCancel] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's verify the file exists and check the lines around 490-500
head -520 src/renderer/components/TabBar.tsx | tail -35Repository: RunMaestro/Maestro
Length of output: 1130
🏁 Script executed:
# Also check the indentation style of the file
head -20 src/renderer/components/TabBar.tsx | cat -ARepository: RunMaestro/Maestro
Length of output: 410
🏁 Script executed:
# Search for similar composition handling patterns in the codebase
rg "isComposing|keyCode.*229" --type ts --type tsxRepository: RunMaestro/Maestro
Length of output: 89
🏁 Script executed:
# Check if there are other keyboard handlers in the same file
rg "KeyboardEvent|onKeyDown|onKeyUp" src/renderer/components/TabBar.tsx -A 3Repository: RunMaestro/Maestro
Length of output: 432
🏁 Script executed:
# Search for composition handling patterns (without type specification)
rg "isComposing|keyCode" src/renderer --max-count 20Repository: RunMaestro/Maestro
Length of output: 120
🏁 Script executed:
# Search for other textarea or input handlers with similar patterns
rg "onKeyDown|onKeyUp" src/renderer/components -B 2 -A 5Repository: RunMaestro/Maestro
Length of output: 50374
🏁 Script executed:
# Check React event typing to verify proposed fix syntax
rg "React.KeyboardEvent" src/renderer -A 2 -B 1Repository: RunMaestro/Maestro
Length of output: 25742
🌐 Web query:
IME composition handling React KeyboardEvent isComposing
💡 Result:
In React, IME (Korean/Japanese/Chinese, dead keys, etc.) often fires keydown/keyup while the user is still composing text. You generally want to ignore shortcut/submit keys (e.g., Enter) during that composition session.
What isComposing means
KeyboardEvent.isComposing === true when the key event happens after compositionstart and before compositionend. During an active composition session, key events are still dispatched and should have isComposing: true. [1][2]
The React gotcha: use e.nativeEvent.isComposing
React’s handler receives a SyntheticEvent. In practice, the reliable flag is on the underlying native event:
function onKeyDown(e: React.KeyboardEvent<HTMLInputElement>) {
// ignore Enter while IME is composing
if (e.key === "Enter" && e.nativeEvent.isComposing) return;
if (e.key === "Enter") {
// submit
}
}Most robust pattern (track composition explicitly)
Some behaviors (especially around input/compositionend ordering across browsers) mean it’s often best to track composition state via composition events, and gate your key handling on that state. [3][2]
function SearchBox() {
const [composing, setComposing] = React.useState(false);
return (
<input
onCompositionStart={() => setComposing(true)}
onCompositionEnd={() => setComposing(false)}
onKeyDown={(e) => {
if (composing) return; // safest
if (e.key !== "Enter") return;
// submit
}}
/>
);
}Rule of thumb: for “Enter to submit”, “hotkeys”, “preventDefault on keydown”, etc., skip handling while composing using e.nativeEvent.isComposing and/or onCompositionStart/End. [1][2]
Sources
[1] MDN: KeyboardEvent.isComposing definition and meaning https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/isComposing
[2] W3C UI Events: key events during composition must have isComposing = true https://www.w3.org/TR/uievents/
[3] Mozilla dev-platform discussion: cross-browser input vs compositionend ordering impacts isComposing handling https://groups.google.com/a/mozilla.org/g/dev-platform/c/yAbeVQR2D4Y
🌐 Web query:
React textarea Enter key IME input method editor composition state
💡 Result:
When using an IME (Japanese/Korean/Chinese, etc.), Enter is often used to confirm the in-progress composition, so your React <textarea> “submit on Enter” handler must ignore Enter while composing.
The key signal: KeyboardEvent.isComposing
Browsers expose KeyboardEvent.isComposing === true for key events fired between compositionstart and compositionend. (developer.mozilla.org)
In React, that flag is typically on the native event:
function ChatBox() {
const onKeyDown = (e: React.KeyboardEvent<HTMLTextAreaElement>) => {
if (e.key !== "Enter") return;
// Ignore Enter used to accept/finish IME composition
if (e.nativeEvent.isComposing) return;
e.preventDefault(); // prevent newline
submitMessage();
};
return <textarea onKeyDown={onKeyDown} />;
}React note: many people access it as e.nativeEvent.isComposing (not e.isComposing). (zenn.dev)
Optional: track composition explicitly (most robust)
function ChatBox() {
const composingRef = React.useRef(false);
return (
<textarea
onCompositionStart={() => (composingRef.current = true)}
onCompositionEnd={() => (composingRef.current = false)}
onKeyDown={(e) => {
if (e.key !== "Enter") return;
if (composingRef.current || e.nativeEvent.isComposing) return;
e.preventDefault();
submitMessage();
}}
/>
);
}Composition events exist specifically for IME “text composition” sessions. (stackoverflow.com)
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/isComposing?utm_source=openai
- 2: https://zenn.dev/takky94/articles/f3096bb59761ee?utm_source=openai
- 3: https://stackoverflow.com/questions/35876470/react-composition-events?utm_source=openai
Guard Enter/Escape handling during IME composition.
Enter/Escape currently triggers save/cancel even while composing text with an IME, which can prematurely commit/cancel for CJK input. This affects users typing in Chinese, Japanese, or Korean.
💡 Proposed fix
const handleDescriptionKeyDown = useCallback(
(e: React.KeyboardEvent<HTMLTextAreaElement>) => {
+ const native = e.nativeEvent as KeyboardEvent;
+ if (native.isComposing || native.keyCode === 229) return;
+
if (e.key === 'Enter' && !e.shiftKey) {
e.preventDefault();
handleDescriptionSave(descriptionDraft);
} else if (e.key === 'Escape') {
e.preventDefault();
handleDescriptionCancel();
}
},
[descriptionDraft, handleDescriptionSave, handleDescriptionCancel]
);📝 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.
| const handleDescriptionKeyDown = useCallback( | |
| (e: React.KeyboardEvent<HTMLTextAreaElement>) => { | |
| if (e.key === 'Enter' && !e.shiftKey) { | |
| e.preventDefault(); | |
| handleDescriptionSave(descriptionDraft); | |
| } else if (e.key === 'Escape') { | |
| e.preventDefault(); | |
| handleDescriptionCancel(); | |
| } | |
| }, | |
| [descriptionDraft, handleDescriptionSave, handleDescriptionCancel] | |
| const handleDescriptionKeyDown = useCallback( | |
| (e: React.KeyboardEvent<HTMLTextAreaElement>) => { | |
| const native = e.nativeEvent as KeyboardEvent; | |
| if (native.isComposing || native.keyCode === 229) return; | |
| if (e.key === 'Enter' && !e.shiftKey) { | |
| e.preventDefault(); | |
| handleDescriptionSave(descriptionDraft); | |
| } else if (e.key === 'Escape') { | |
| e.preventDefault(); | |
| handleDescriptionCancel(); | |
| } | |
| }, | |
| [descriptionDraft, handleDescriptionSave, handleDescriptionCancel] | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/TabBar.tsx` around lines 490 - 500, The Enter/Escape
handlers in handleDescriptionKeyDown trigger save/cancel even during IME
composition; update handleDescriptionKeyDown to first check for IME composition
(e.nativeEvent.isComposing) and do nothing if composing, then proceed to
preventDefault and call handleDescriptionSave(descriptionDraft) or
handleDescriptionCancel() only when not composing; this ensures keys are ignored
while composing and maintains existing behavior otherwise.
📝 WalkthroughWalkthroughThis change introduces a tab description editing feature throughout the application. It adds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/renderer/components/TabBar.tsx (1)
490-500:⚠️ Potential issue | 🟠 MajorGuard Enter/Escape during IME composition (still unresolved).
Line 492 and Line 495 process save/cancel shortcuts even while users are composing text with an IME, which can prematurely commit or cancel input.
💡 Proposed fix
const handleDescriptionKeyDown = useCallback( (e: React.KeyboardEvent<HTMLTextAreaElement>) => { + const native = e.nativeEvent as KeyboardEvent; + if (native.isComposing || native.keyCode === 229) return; + if (e.key === 'Enter' && !e.shiftKey) { e.preventDefault(); handleDescriptionSave(descriptionDraft); } else if (e.key === 'Escape') { e.preventDefault(); handleDescriptionCancel(); } }, [descriptionDraft, handleDescriptionSave, handleDescriptionCancel] );In React textarea onKeyDown handlers, is checking `e.nativeEvent.isComposing` (with a `keyCode === 229` fallback) the recommended way to ignore Enter/Escape while IME composition is active?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/TabBar.tsx` around lines 490 - 500, The handler handleDescriptionKeyDown currently triggers save/cancel on Enter/Escape even during IME composition; update the function to first check and ignore events when composition is active by returning early if (e.nativeEvent as any).isComposing is true or e.keyCode === 229 (IME fallback), then proceed to call handleDescriptionSave(descriptionDraft) or handleDescriptionCancel; ensure the dependency list for useCallback remains [descriptionDraft, handleDescriptionSave, handleDescriptionCancel].
🧹 Nitpick comments (2)
src/renderer/hooks/tabs/useTabHandlers.ts (1)
973-983: Optional: short-circuit unchanged updates to reduce avoidable re-renders.The active session object is recreated even when
tabIdis missing or the normalized description is unchanged.♻️ Proposed no-op guard
setSessions((prev: Session[]) => prev.map((s) => { if (s.id !== activeSessionId) return s; - return { - ...s, - aiTabs: s.aiTabs.map((tab) => - tab.id === tabId ? { ...tab, description: trimmed || undefined } : tab - ), - }; + const nextDescription = trimmed || undefined; + let changed = false; + const aiTabs = s.aiTabs.map((tab) => { + if (tab.id !== tabId) return tab; + if (tab.description === nextDescription) return tab; + changed = true; + return { ...tab, description: nextDescription }; + }); + return changed ? { ...s, aiTabs } : s; }) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/tabs/useTabHandlers.ts` around lines 973 - 983, The update recreates the active session even when nothing changes; modify the setSessions updater (used with setSessions, Session, activeSessionId, aiTabs, tabId, description, trimmed) to short-circuit: inside the updater, locate the active session and its target tab first, compare the normalized trimmed value to the existing tab.description and if unchanged (or if tabId not found) return prev unchanged; only map/return a new sessions array when the target tab's description actually differs, so you avoid recreating session objects and unnecessary re-renders.src/__tests__/renderer/components/TabBar.test.tsx (1)
5841-5911: Add tests for the remaining description-editor branches.This block validates Enter/Escape well, but it still misses assertions for Shift+Enter newline and “save draft when overlay closes during edit,” which are both part of the new behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/TabBar.test.tsx` around lines 5841 - 5911, Add tests covering the remaining description-editor branches: (1) a test that pressing Shift+Enter inside the textarea inserts a newline and does NOT call onUpdateTabDescription (use the existing pattern: render TabBar, openOverlay('Tab 1'), click the Add/Edit button, get textarea, fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: true }) and assert mockOnUpdateTabDescription not called and that textarea value contains a newline); (2) a test that starting to edit a description and then closing the overlay saves a draft by calling onUpdateTabDescription with the trimmed value (render TabBar, openOverlay('Tab 1'), click edit, change textarea to e.g. ' draft ', then simulate closing the overlay and assert mockOnUpdateTabDescription was called with 'tab-1' and 'draft'). Ensure you reference the existing mocks (mockOnUpdateTabDescription), the TabBar component, and the aria labels ('Add tab description' / 'Edit tab description' / 'Tab description') so the new tests locate the elements consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/renderer/components/TabBar.tsx`:
- Around line 490-500: The handler handleDescriptionKeyDown currently triggers
save/cancel on Enter/Escape even during IME composition; update the function to
first check and ignore events when composition is active by returning early if
(e.nativeEvent as any).isComposing is true or e.keyCode === 229 (IME fallback),
then proceed to call handleDescriptionSave(descriptionDraft) or
handleDescriptionCancel; ensure the dependency list for useCallback remains
[descriptionDraft, handleDescriptionSave, handleDescriptionCancel].
---
Nitpick comments:
In `@src/__tests__/renderer/components/TabBar.test.tsx`:
- Around line 5841-5911: Add tests covering the remaining description-editor
branches: (1) a test that pressing Shift+Enter inside the textarea inserts a
newline and does NOT call onUpdateTabDescription (use the existing pattern:
render TabBar, openOverlay('Tab 1'), click the Add/Edit button, get textarea,
fireEvent.keyDown(textarea, { key: 'Enter', shiftKey: true }) and assert
mockOnUpdateTabDescription not called and that textarea value contains a
newline); (2) a test that starting to edit a description and then closing the
overlay saves a draft by calling onUpdateTabDescription with the trimmed value
(render TabBar, openOverlay('Tab 1'), click edit, change textarea to e.g. '
draft ', then simulate closing the overlay and assert
mockOnUpdateTabDescription was called with 'tab-1' and 'draft'). Ensure you
reference the existing mocks (mockOnUpdateTabDescription), the TabBar component,
and the aria labels ('Add tab description' / 'Edit tab description' / 'Tab
description') so the new tests locate the elements consistently.
In `@src/renderer/hooks/tabs/useTabHandlers.ts`:
- Around line 973-983: The update recreates the active session even when nothing
changes; modify the setSessions updater (used with setSessions, Session,
activeSessionId, aiTabs, tabId, description, trimmed) to short-circuit: inside
the updater, locate the active session and its target tab first, compare the
normalized trimmed value to the existing tab.description and if unchanged (or if
tabId not found) return prev unchanged; only map/return a new sessions array
when the target tab's description actually differs, so you avoid recreating
session objects and unnecessary re-renders.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/__tests__/renderer/components/TabBar.test.tsxsrc/__tests__/renderer/hooks/useTabHandlers.test.tssrc/renderer/App.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/TabBar.tsxsrc/renderer/hooks/props/useMainPanelProps.tssrc/renderer/hooks/tabs/useTabHandlers.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.ts
Summary
descriptionfield toAITabtypehandleUpdateTabDescriptionhandler inuseTabHandlersencoreFeatures.tabDescriptionflagTest plan
npm run lintpasses (zero errors)npm run lint:eslintpasses (zero warnings)npm run testpasses (21,026 tests pass; 3 pre-existing CodexSessionStorage timeout flakes)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests