From fde46a62f91a35275310f7dd6d534a16d21966f9 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 3 Sep 2026 23:41:25 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=99=20refactor:=20Keep=20Pasted=20Text?= =?UTF-8?q?=20Return=20Action=20Visible=20(#15586)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chip generated from a long paste offers returning its text to the composer on the subtitle line, but the label only swapped in on `group-hover` / `group-focus-within`. At rest the chip read as an inert "Plain Text" subtitle, so the affordance went unfound — and touch devices have no pointer to find it with, which the `[@media(hover:none)]` override was already conceding. The label is now the subtitle: stated at rest, underlined so a permanent line of secondary text reads as a control rather than as a caption. The `aria-label` that shadowed the visible text goes with the swap it existed for, leaving the accessible name equal to what the label says. --- .../Chat/Input/Files/FileContainer.tsx | 32 +++---- .../Files/__tests__/FileContainer.spec.tsx | 86 ++++++------------- 2 files changed, 38 insertions(+), 80 deletions(-) diff --git a/client/src/components/Chat/Input/Files/FileContainer.tsx b/client/src/components/Chat/Input/Files/FileContainer.tsx index 9dfd53bf2e..e914be08d2 100644 --- a/client/src/components/Chat/Input/Files/FileContainer.tsx +++ b/client/src/components/Chat/Input/Files/FileContainer.tsx @@ -5,7 +5,7 @@ import { getFileType, cn } from '~/utils'; import FilePreview from './FilePreview'; import RemoveFile from './RemoveFile'; -/** A second action offered in place of the subtitle, revealed when the chip is hovered or holds focus. */ +/** A second action offered in place of the subtitle, stated on the chip rather than revealed on hover. */ export type SubtitleAction = { label: string; onClick: () => void; @@ -87,30 +87,20 @@ const FileContainer = ({ ) : ( (subtitle ?? ( diff --git a/client/src/components/Chat/Input/Files/__tests__/FileContainer.spec.tsx b/client/src/components/Chat/Input/Files/__tests__/FileContainer.spec.tsx index f072042387..6c44251613 100644 --- a/client/src/components/Chat/Input/Files/__tests__/FileContainer.spec.tsx +++ b/client/src/components/Chat/Input/Files/__tests__/FileContainer.spec.tsx @@ -60,25 +60,40 @@ describe('FileContainer chip label', () => { describe('FileContainer subtitle action', () => { const subtitleAction = (onClick = jest.fn()) => ({ label: 'Move back into message', onClick }); - it('shows the file type until the chip is hovered', () => { + it('states the action on the chip instead of the file type', () => { render( , ); - /** Both labels are in the DOM; which one shows is a `group-hover` swap CSS owns. */ - expect(screen.getByText('Plain')).toBeInTheDocument(); + /** The action is the subtitle now, not something the file type is swapped out for. */ expect(screen.getByText('Move back into message')).toBeInTheDocument(); + expect(screen.queryByText('Plain')).not.toBeInTheDocument(); }); - it('names the subtitle control for screen readers regardless of the visible label', () => { + it('offers the action with no pointer or focus needed to reveal it', () => { render( , ); + /** The affordance used to live behind a `group-hover` swap, which left the chip reading as + * an inert "Plain Text" until pointed at — so nobody found it, and touch had no pointer to + * find it with. Nothing about the label may be conditional on hover or focus again. */ const control = screen.getByRole('button', { name: 'Move back into message' }); - expect(control).toBeInTheDocument(); - /** The swapped spans are decorative; the accessible name comes from the label. */ - expect(screen.getByText('Plain')).toHaveAttribute('aria-hidden', 'true'); + expect(control.className).not.toContain('group-hover'); + expect(control.className).not.toContain('group-focus-within'); + expect(control.className).not.toContain('hover:none'); + expect(control.textContent).toBe('Move back into message'); + }); + + it('names the subtitle control from its own visible label', () => { + render( + , + ); + + /** Visible text and accessible name are the same string, so voice control can act on what + * the label says without an `aria-label` shadowing it. */ + const control = screen.getByRole('button', { name: 'Move back into message' }); + expect(control).not.toHaveAttribute('aria-label'); }); it('runs the subtitle action without also opening the chip', async () => { @@ -124,43 +139,6 @@ describe('FileContainer subtitle action', () => { expect(container.querySelector('button button')).toBeNull(); }); - it('reveals the action from the whole chip rather than the label alone', () => { - render( - , - ); - - /** The swap keys off the chip's own `group`, so pointing anywhere on the chip offers it. */ - const resting = screen.getByText('Plain'); - const revealed = screen.getByText('Move back into message'); - expect(resting.className).toContain('group-hover:hidden'); - expect(revealed.className).toContain('group-hover:inline'); - expect(`${resting.className} ${revealed.className}`).not.toContain('group-hover/'); - }); - - it('reveals the action for keyboard users too', () => { - render( - , - ); - - expect(screen.getByText('Plain').className).toContain('group-focus-within:hidden'); - expect(screen.getByText('Move back into message').className).toContain( - 'group-focus-within:inline', - ); - }); - - it('keeps the action revealed on devices with no hover to reveal it', () => { - render( - , - ); - - /** Touch has no hover or focus to announce the swap with, so the label itself must show: - * an inert-looking "Plain Text" subtitle that detaches on tap is a trap, not an affordance. */ - expect(screen.getByText('Plain').className).toContain('[@media(hover:none)]:hidden'); - expect(screen.getByText('Move back into message').className).toContain( - '[@media(hover:none)]:inline', - ); - }); - it('keeps the full-chip focus indicator inside the clipped surface', () => { render( , @@ -173,27 +151,17 @@ describe('FileContainer subtitle action', () => { expect(chipControl.className).not.toContain('ring-offset'); }); - it('styles the action exactly like the subtitle it replaces', () => { + it('reads as a control rather than as a description', () => { render( , ); + /** It keeps the subtitle's weight, but a permanent line of secondary text with no marking + * would read as a caption; the underline is what says it can be clicked. */ const control = screen.getByRole('button', { name: 'Move back into message' }); expect(control.className).toContain('text-text-secondary'); - /** No colour of its own: it should read as the subtitle, not as a link. */ - expect(control.className).not.toContain('hover:text-text-primary'); - }); - - it('underlines the action once the pointer is on the label itself', () => { - render( - , - ); - - /** Keyed to the control's own hover, not the chip's, so the underline only shows up - * under a pointer that is actually on the label. */ - const control = screen.getByRole('button', { name: 'Move back into message' }); - expect(control.className).toContain('hover:underline'); - expect(control.className).not.toContain('group-hover:underline'); + expect(control.className).toContain('underline'); + expect(control.className).not.toContain('hover:underline'); }); it('leaves the plain subtitle alone when no action is offered', () => {