diff --git a/client/src/components/Chat/Messages/HoverButtons.tsx b/client/src/components/Chat/Messages/HoverButtons.tsx index 49ae50c4cd..b1a627953c 100644 --- a/client/src/components/Chat/Messages/HoverButtons.tsx +++ b/client/src/components/Chat/Messages/HoverButtons.tsx @@ -98,7 +98,9 @@ const HoverButton = memo( !isLast && isVisible && 'group-hover:opacity-100 group-focus-within:opacity-100 [@media(hover:hover)]:opacity-0', - !isVisible && 'pointer-events-none opacity-0', + /** `!` is load-bearing: the shared Button sets `disabled:opacity-50`, which outranks a + * plain `opacity-0` and would leave a dimmed ghost of the hidden action on screen. */ + !isVisible && 'pointer-events-none !opacity-0', 'focus-visible:ring-2 focus-visible:ring-text-primary focus-visible:outline-none', isActive && isVisible && 'active text-text-primary bg-surface-hover', className, diff --git a/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx b/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx index e19d7eb40e..4a6ab73c4e 100644 --- a/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx +++ b/client/src/components/Chat/Messages/__tests__/HoverButtons.spec.tsx @@ -62,9 +62,13 @@ describe('HoverButtons edit affordance', () => { const editButton = renderHoverButtons(true); expect(editButton).toBeDisabled(); - expect(editButton).toHaveClass('opacity-0', 'pointer-events-none'); + expect(editButton).toHaveClass('pointer-events-none'); expect(editButton.className).not.toMatch(/group-hover:opacity-100/); expect(editButton.className).not.toMatch(/group-focus-within:opacity-100/); + /** Must outrank the shared Button's `disabled:opacity-50`; a bare `opacity-0` loses to it, + * and jsdom applies no stylesheet, so the important modifier is what we can assert here. */ + expect(editButton).toHaveClass('!opacity-0'); + expect(editButton).not.toHaveClass('opacity-0'); }); it('reveals on row hover once the generation settles', () => { @@ -72,6 +76,6 @@ describe('HoverButtons edit affordance', () => { expect(editButton).toBeEnabled(); expect(editButton).toHaveClass('group-hover:opacity-100'); - expect(editButton).not.toHaveClass('pointer-events-none', 'opacity-0'); + expect(editButton).not.toHaveClass('pointer-events-none', 'opacity-0', '!opacity-0'); }); }); diff --git a/e2e/specs/mock/hover-actions.spec.ts b/e2e/specs/mock/hover-actions.spec.ts new file mode 100644 index 0000000000..a55e7915ae --- /dev/null +++ b/e2e/specs/mock/hover-actions.spec.ts @@ -0,0 +1,66 @@ +import { expect, test } from '@playwright/test'; +import type { Page } from '@playwright/test'; +import { + MOCK_ENDPOINTS, + NEW_CHAT_PATH, + messagesView, + selectMockEndpoint, + sendMessage, +} from './helpers'; + +/** + * Regression guard for the edit action leaking through mid-stream. + * + * The unit spec can only assert class names: jsdom applies no stylesheet, so it + * cannot see that the shared Button's `disabled:opacity-50` (specificity 0,2,0) + * outranks a plain `opacity-0` (0,1,0) and repaints the hidden pencil at half + * opacity. Only a real browser resolves that cascade, which is why this lives + * here rather than in Jest. + */ + +const uniqueLabel = (prefix: string) => + `${prefix}-${Date.now()}-${Math.floor(Math.random() * 1e4)}`; + +const userTurn = (page: Page) => + messagesView(page) + .locator('.message-render') + .filter({ has: page.locator('.user-turn') }) + .last(); + +const stopButton = (page: Page) => page.getByRole('button', { name: 'Stop generating' }); + +test.describe('message hover actions', () => { + test('keeps the edit action fully hidden while a generation streams', async ({ page }) => { + test.setTimeout(120000); + const label = uniqueLabel('hover-edit'); + + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); + + const run = await sendMessage(page, `E2E_SLOW_REPLY:${label}`); + expect(run.ok()).toBeTruthy(); + await expect(messagesView(page).getByText('chunk-010')).toBeVisible({ timeout: 15000 }); + + const row = userTurn(page); + const editButton = row.locator('button[id^="edit-"]'); + const copyButton = row.getByRole('button', { name: 'Copy to clipboard' }); + + await row.hover(); + + /** Pin the window: if the stream already settled, the edit assertion below + * would be checking the wrong state and pass for the wrong reason. */ + await expect(stopButton(page)).toBeVisible(); + + /** The sibling action proves the row is genuinely hovered — without it a + * broken hover would make the edit assertion pass for the wrong reason. */ + await expect(copyButton).toHaveCSS('opacity', '1'); + await expect(editButton).toHaveCSS('opacity', '0'); + await expect(editButton).toBeDisabled(); + + /** ...and the affordance must come back, or "hidden" would just be "gone". */ + await expect(stopButton(page)).toBeHidden({ timeout: 60000 }); + await row.hover(); + await expect(editButton).toBeEnabled(); + await expect(editButton).toHaveCSS('opacity', '1'); + }); +});