mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
🔙 refactor: Keep Pasted Text Return Action Visible (#15586)
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.
This commit is contained in:
parent
a9ce3d2b33
commit
fde46a62f9
2 changed files with 38 additions and 80 deletions
|
|
@ -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 = ({
|
|||
<button
|
||||
type="button"
|
||||
onClick={subtitleAction.onClick}
|
||||
aria-label={subtitleAction.label}
|
||||
className={cn(
|
||||
/** Reads as the subtitle it replaces, so no colour of its own; the underline is
|
||||
* the only thing marking it as clickable, and it lands once the pointer is on
|
||||
* the label rather than merely on the chip. 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 hover:underline',
|
||||
/** The chip's only cue that its text can be brought back, so it states itself
|
||||
* at rest instead of replacing the file type on hover: an affordance that only
|
||||
* appears under a pointer is one most people never find, and touch has no
|
||||
* pointer to find it with. Underlined for the same reason — as a permanent
|
||||
* 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',
|
||||
focusRing,
|
||||
)}
|
||||
>
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className="group-focus-within:hidden group-hover:hidden [@media(hover:none)]:hidden"
|
||||
>
|
||||
{fileType.title}
|
||||
</span>
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className="hidden group-focus-within:inline group-hover:inline [@media(hover:none)]:inline"
|
||||
>
|
||||
{subtitleAction.label}
|
||||
</span>
|
||||
{subtitleAction.label}
|
||||
</button>
|
||||
) : (
|
||||
(subtitle ?? (
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
/** 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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
/** 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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
/** 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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
/** 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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
/** 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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
|
|
@ -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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
/** 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(
|
||||
<FileContainer file={baseFile()} onClick={jest.fn()} subtitleAction={subtitleAction()} />,
|
||||
);
|
||||
|
||||
/** 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', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue