👓 fix: Match Pasted Text Focus Feedback (#15588)

Review follow-up. The label's colour shift answered the pointer only. The focus
ring already announces focus, so this is parity rather than a missing indicator
— but two input modes getting different feedback for the same state was a
difference with nothing behind it.

The hover-gate guard keeps asserting class tokens, brittle as that is. jsdom
loads no stylesheet, so a hover gate has no effect on the DOM under test and a
presence check cannot see one: the swap this replaced kept both labels mounted,
which is why the spec it replaced asserted both were in the document at once.
Recorded the reasoning where the next reader will meet it.
This commit is contained in:
Danny Avila 2026-09-04 00:07:08 -04:00 • committed by GitHub
parent 1754297038
commit bd83da552d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 23 additions and 4 deletions

View file

@ -95,8 +95,10 @@ const FileContainer = ({
* subtitle it would otherwise read as a description rather than a control. The
* hit area is the label and nothing more: `inline-block` keeps it off the rest
* of the row, and `leading-4` keeps it inside its 20px line so the chip's own
* center still opens the editor. */
'pointer-events-auto relative z-10 inline-block max-w-full truncate rounded text-left align-middle leading-4 text-text-secondary underline underline-offset-2 hover:text-text-primary',
* center still opens the editor. The colour shift answers focus as well as
* hover: the ring already announces focus, but leaving the two input modes
* with different feedback is a difference with no reason behind it. */
'pointer-events-auto relative z-10 inline-block max-w-full truncate rounded text-left align-middle leading-4 text-text-secondary underline underline-offset-2 hover:text-text-primary focus-visible:text-text-primary',
focusRing,
)}
>

View file

@ -77,12 +77,17 @@ describe('FileContainer subtitle action', () => {
/** 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. */
* find it with. Nothing about the label may be conditional on hover or focus again.
*
* Asserted on class tokens deliberately, brittle as that is. jsdom loads no stylesheet, so
* a hover gate has no effect on the DOM here and a presence check cannot see one: the swap
* this replaced kept BOTH labels mounted, which is why the spec it replaced asserted they
* were both in the document at once. Only the tokens distinguish the two markups. */
const control = screen.getByRole('button', { name: 'Move back into message' });
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');
expect(control).toHaveTextContent('Move back into message');
});
it('names the subtitle control from its own visible label', () => {
@ -164,6 +169,18 @@ describe('FileContainer subtitle action', () => {
expect(control.className).not.toContain('hover:underline');
});
it('gives keyboard focus the same feedback the pointer gets', () => {
render(
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
);
/** The focus ring already announces focus, so this is parity rather than a missing
* indicator: whatever the colour shift signals to a pointer, it signals to a keyboard. */
const control = screen.getByRole('button', { name: 'Move back into message' });
expect(control.className).toContain('hover:text-text-primary');
expect(control.className).toContain('focus-visible:text-text-primary');
});
it('leaves the plain subtitle alone when no action is offered', () => {
render(<FileContainer file={baseFile()} onClick={jest.fn()} />);