fix(cli): close hover state when copy button is clicked

brandonkachen · brandonkachen · commit e160a8a53153 · 2025-12-15T13:14:31.000-08:00
Fixes bug where the copy button would remain in hover state after showing
the "copied" message. Added hover.closeNow() call on click to ensure
the button returns to its collapsed state after the copied state ends.

Also adds unit tests for CopyIconButton hover state behavior.
diff --git a/cli/src/__tests__/unit/copy-icon-button.test.ts b/cli/src/__tests__/unit/copy-icon-button.test.ts
@@ -0,0 +1,153 @@
+import { describe, test, expect, beforeEach, afterEach, mock } from 'bun:test'
+
+import { createHoverToggleControllerForTest } from '../mocks/hover-toggle-controller'
+
+/**
+ * Tests for the CopyIconButton hover state behavior.
+ * 
+ * The key behavior being tested:
+ * - When the copy button is clicked and shows "copied", the hover state should be closed
+ * - This prevents the button from appearing in hover state after the "copied" message disappears
+ */
+describe('CopyIconButton hover state behavior', () => {
+  let originalSetTimeout: typeof setTimeout
+  let originalClearTimeout: typeof clearTimeout
+
+  let timers: { id: number; ms: number; fn: Function; active: boolean }[]
+  let nextId: number
+
+  const runAll = () => {
+    for (const t of timers) {
+      if (t.active) t.fn()
+    }
+    timers = []
+  }
+
+  beforeEach(() => {
+    timers = []
+    nextId = 1
+    originalSetTimeout = setTimeout
+    originalClearTimeout = clearTimeout
+
+    globalThis.setTimeout = ((fn: Function, ms?: number) => {
+      const id = nextId++
+      timers.push({ id, ms: Number(ms ?? 0), fn, active: true })
+      return id as any
+    }) as any
+
+    globalThis.clearTimeout = ((id?: any) => {
+      const rec = timers.find((t) => t.id === id)
+      if (rec) rec.active = false
+    }) as any
+  })
+
+  afterEach(() => {
+    globalThis.setTimeout = originalSetTimeout
+    globalThis.clearTimeout = originalClearTimeout
+  })
+
+  test('hover state closes when button is clicked (simulates copy action)', () => {
+    const hover = createHoverToggleControllerForTest()
+    
+    // Simulate: user hovers over button, hover state opens
+    hover.scheduleOpen()
+    runAll()
+    expect(hover.isOpen).toBe(true)
+    
+    // Simulate: user clicks the button (copy action)
+    // The fix: closeNow() should be called to close hover state
+    hover.closeNow()
+    
+    // Hover state should now be closed
+    expect(hover.isOpen).toBe(false)
+  })
+
+  test('hover state remains closed after copied state ends', () => {
+    const hover = createHoverToggleControllerForTest()
+    let isCopied = false
+    
+    // Simulate: user hovers, hover opens
+    hover.scheduleOpen()
+    runAll()
+    expect(hover.isOpen).toBe(true)
+    
+    // Simulate: user clicks copy button
+    isCopied = true
+    hover.closeNow() // This is the fix we added
+    
+    expect(hover.isOpen).toBe(false)
+    
+    // Simulate: 2 seconds pass, isCopied resets to false
+    isCopied = false
+    
+    // Without re-hovering, the hover state should still be closed
+    expect(hover.isOpen).toBe(false)
+  })
+
+  test('without closeNow fix, hover would persist after copy (demonstrates the bug)', () => {
+    const hover = createHoverToggleControllerForTest()
+    let isCopied = false
+    
+    // Simulate: user hovers, hover opens
+    hover.scheduleOpen()
+    runAll()
+    expect(hover.isOpen).toBe(true)
+    
+    // Simulate old buggy behavior: click happens but closeNow is NOT called
+    isCopied = true
+    // hover.closeNow() was NOT called in the buggy version
+    
+    // BUG: hover state is still open
+    expect(hover.isOpen).toBe(true)
+    
+    // After isCopied resets, hover is STILL open (this was the bug)
+    isCopied = false
+    expect(hover.isOpen).toBe(true) // Bug: should be false
+  })
+
+  test('mouse out does not close hover while in copied state', () => {
+    const hover = createHoverToggleControllerForTest()
+    let isCopied = false
+    
+    // Simulate: user hovers, hover opens
+    hover.scheduleOpen()
+    runAll()
+    expect(hover.isOpen).toBe(true)
+    
+    // Simulate: user clicks, enters copied state, hover closes
+    isCopied = true
+    hover.closeNow()
+    expect(hover.isOpen).toBe(false)
+    
+    // Simulate: mouse out handler - should not schedule close when isCopied
+    // (This mirrors the component logic: handleMouseOut only calls scheduleClose when !isCopied)
+    if (!isCopied) {
+      hover.scheduleClose()
+    }
+    
+    // No timers should be scheduled since we're in copied state
+    expect(timers.filter(t => t.active).length).toBe(0)
+  })
+
+  test('mouse over does not open hover while in copied state', () => {
+    const hover = createHoverToggleControllerForTest()
+    let isCopied = false
+    
+    // Start with hover closed
+    expect(hover.isOpen).toBe(false)
+    
+    // Enter copied state
+    isCopied = true
+    
+    // Simulate: mouse over handler - should not schedule open when isCopied
+    // (This mirrors the component logic: handleMouseOver only calls scheduleOpen when !isCopied)
+    if (!isCopied) {
+      hover.clearCloseTimer()
+      hover.scheduleOpen()
+    }
+    
+    // No timers should be scheduled since we're in copied state
+    expect(timers.filter(t => t.active).length).toBe(0)
+    expect(hover.isOpen).toBe(false)
+  })
+})
diff --git a/cli/src/components/copy-icon-button.tsx b/cli/src/components/copy-icon-button.tsx
@@ -86,6 +86,7 @@ export const CopyIconButton: React.FC<CopyIconButtonProps> = ({
         suppressGlobalMessage: true,
       })
       setIsCopied(true)
+      hover.closeNow() // Close hover state so it doesn't persist after copied state ends
       setTimeout('reset-copied', () => setIsCopied(false), 2000)
     } catch (error) {
       // Error is already logged and displayed by copyTextToClipboard
diff --git a/cli/src/components/message-block.tsx b/cli/src/components/message-block.tsx
@@ -180,7 +180,7 @@ export const MessageBlock: React.FC<MessageBlockProps> = ({
         width: '100%',
       }}
     >
-      {/* User message timestamp with copy button and error indicator (non-bash commands) */}
+      {/* User message timestamp with error indicator (non-bash commands) */}
       {isUser && !bashCwd && (
         <box style={{ flexDirection: 'row', alignItems: 'center', gap: 1 }}>
           <text
@@ -193,8 +193,6 @@ export const MessageBlock: React.FC<MessageBlockProps> = ({
             {`[${timestamp}]`}
           </text>
 
-          <CopyIconButton textToCopy={content} />
-
           {validationErrors && validationErrors.length > 0 && (
             <Button
               onClick={() => setShowValidationPopover(!showValidationPopover)}
@@ -212,7 +210,7 @@ export const MessageBlock: React.FC<MessageBlockProps> = ({
         </box>
       )}
 
-      {/* Bash command metadata header (timestamp + copy button + cwd) */}
+      {/* Bash command metadata header (timestamp + cwd) - copy button moved inline */}
       {bashCwd && (
         <box style={{ flexDirection: 'row', alignItems: 'center', gap: 1 }}>
           <text
@@ -224,7 +222,6 @@ export const MessageBlock: React.FC<MessageBlockProps> = ({
           >
             {`[${timestamp}]`}
           </text>
-          <CopyIconButton textToCopy={content} />
           <text
             attributes={TextAttributes.DIM}
             style={{
@@ -285,9 +282,15 @@ export const MessageBlock: React.FC<MessageBlockProps> = ({
             onBuildMax={onBuildMax}
             isLastMessage={isLastMessage}
           />
+          {/* Copy button after user message content (for block-based messages) */}
+          {isUser && (
+            <box style={{ flexDirection: 'row', marginTop: 0 }}>
+              <CopyIconButton textToCopy={content} />
+            </box>
+          )}
         </box>
       ) : (
-        <SimpleContent
+        <UserContentWithCopyButton
           content={content}
           messageId={messageId}
           isLoading={isLoading}
@@ -296,6 +299,7 @@ export const MessageBlock: React.FC<MessageBlockProps> = ({
           textColor={resolvedTextColor}
           codeBlockWidth={markdownOptions.codeBlockWidth}
           palette={markdownOptions.palette}
+          showCopyButton={isUser}
         />
       )}
       {/* Show image attachments for user messages */}
@@ -815,7 +819,7 @@ const AgentBranchWrapper = memo(
   },
 )
 
-interface SimpleContentProps {
+interface UserContentWithCopyButtonProps {
   content: string
   messageId: string
   isLoading: boolean
@@ -824,9 +828,14 @@ interface SimpleContentProps {
   textColor: string
   codeBlockWidth: number
   palette: MarkdownPalette
+  showCopyButton: boolean
 }
 
-const SimpleContent = memo(
+/**
+ * Renders user content with an inline copy button that wraps together with the last word.
+ * Uses a wrapping flexbox so the copy button stays attached to the last word when line-breaking.
+ */
+const UserContentWithCopyButton = memo(
   ({
     content,
     messageId,
@@ -836,25 +845,99 @@ const SimpleContent = memo(
     textColor,
     codeBlockWidth,
     palette,
-  }: SimpleContentProps) => {
+    showCopyButton,
+  }: UserContentWithCopyButtonProps) => {
     const isStreamingMessage = isLoading || !isComplete
     const normalizedContent = isStreamingMessage
       ? trimTrailingNewlines(content)
       : content.trim()
 
+    if (!showCopyButton) {
+      return (
+        <text
+          key={`message-content-${messageId}`}
+          style={{ wrapMode: 'word', fg: textColor }}
+          attributes={isUser ? TextAttributes.ITALIC : undefined}
+        >
+          <ContentWithMarkdown
+            content={normalizedContent}
+            isStreaming={isStreamingMessage}
+            codeBlockWidth={codeBlockWidth}
+            palette={palette}
+          />
+        </text>
+      )
+    }
+
+    // For user messages with copy button, we need to handle the last word specially
+    // so the copy button wraps with it as a unit.
+    // Split by newlines first to handle multi-line content correctly.
+    const lines = normalizedContent.split('\n')
+    const lastLine = lines[lines.length - 1] || ''
+    const lastSpaceIndex = lastLine.lastIndexOf(' ')
+    
+    let contentBeforeLastWord: string
+    let lastWord: string
+    
+    if (lastSpaceIndex > 0) {
+      // Last line has multiple words - split on the last space
+      const beforeLastWordOnLastLine = lastLine.slice(0, lastSpaceIndex + 1)
+      lastWord = lastLine.slice(lastSpaceIndex + 1)
+      if (lines.length > 1) {
+        contentBeforeLastWord = lines.slice(0, -1).join('\n') + '\n' + beforeLastWordOnLastLine
+      } else {
+        contentBeforeLastWord = beforeLastWordOnLastLine
+      }
+    } else if (lines.length > 1) {
+      // Last line is a single word, but there are previous lines
+      contentBeforeLastWord = lines.slice(0, -1).join('\n') + '\n'
+      lastWord = lastLine
+    } else {
+      // Single word, single line
+      contentBeforeLastWord = ''
+      lastWord = normalizedContent
+    }
+
     return (
-      <text
+      <box
         key={`message-content-${messageId}`}
-        style={{ wrapMode: 'word', fg: textColor }}
-        attributes={isUser ? TextAttributes.ITALIC : undefined}
+        style={{
+          flexDirection: 'row',
+          flexWrap: 'wrap',
+          alignItems: 'baseline',
+        }}
       >
-        <ContentWithMarkdown
-          content={normalizedContent}
-          isStreaming={isStreamingMessage}
-          codeBlockWidth={codeBlockWidth}
-          palette={palette}
-        />
-      </text>
+        {contentBeforeLastWord && (
+          <text
+            style={{ wrapMode: 'word', fg: textColor }}
+            attributes={isUser ? TextAttributes.ITALIC : undefined}
+          >
+            <ContentWithMarkdown
+              content={contentBeforeLastWord}
+              isStreaming={isStreamingMessage}
+              codeBlockWidth={codeBlockWidth}
+              palette={palette}
+            />
+          </text>
+        )}
+        {/* Last word + copy button wrapped together so they line-break as a unit */}
+        <box
+          style={{
+            flexDirection: 'row',
+            alignItems: 'center',
+            flexWrap: 'no-wrap',
+            gap: 1,
+          }}
+        >
+          <text
+            style={{ wrapMode: 'none', fg: textColor }}
+            attributes={isUser ? TextAttributes.ITALIC : undefined}
+          >
+            {lastWord}
+          </text>
+          <CopyIconButton textToCopy={content} />
+        </box>
+      </box>
     )
   },
 )
