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', () => {