diff --git a/client/src/components/Chat/Messages/MinimalHoverButtons.tsx b/client/src/components/Chat/Messages/MinimalHoverButtons.tsx index 6cca34cbb4..b80e3f94fe 100644 --- a/client/src/components/Chat/Messages/MinimalHoverButtons.tsx +++ b/client/src/components/Chat/Messages/MinimalHoverButtons.tsx @@ -2,6 +2,8 @@ import { useState, useMemo } from 'react'; import { Button, Clipboard, CheckMark, TooltipAnchor } from '@librechat/client'; import type { TMessage, SearchResultData } from 'librechat-data-provider'; import { useLocalize, useCopyToClipboard, hasCopyableText } from '~/hooks'; +import { revealOnRowHoverClasses } from './styles'; +import { cn } from '~/utils'; type THoverButtons = { message: TMessage; @@ -36,7 +38,12 @@ export default function MinimalHoverButtons({ message, searchResults }: THoverBu ? localize('com_ui_copied_to_clipboard') : localize('com_ui_copy_to_clipboard') } - className="ml-0 flex size-auto items-center gap-1.5 rounded-lg p-1.5 text-xs text-text-secondary-alt transition-colors duration-200 hover:bg-surface-hover hover:text-text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-text-primary group-focus-within:opacity-100 group-hover:opacity-100 [@media(hover:hover)]:opacity-0" + className={cn( + 'ml-0 flex size-auto items-center gap-1.5 rounded-lg p-1.5 text-xs text-text-secondary-alt', + 'hover:bg-surface-hover hover:text-text-primary', + 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-text-primary', + revealOnRowHoverClasses, + )} disabled={!canCopy} onClick={() => copyToClipboard(setIsCopied)} > diff --git a/client/src/components/Chat/Messages/SiblingSwitch.tsx b/client/src/components/Chat/Messages/SiblingSwitch.tsx index 4e566fb1c5..c647346b78 100644 --- a/client/src/components/Chat/Messages/SiblingSwitch.tsx +++ b/client/src/components/Chat/Messages/SiblingSwitch.tsx @@ -33,7 +33,7 @@ export default function SiblingSwitch({ const buttonStyle = cn( 'hover-button h-auto rounded-lg p-1.5 text-text-secondary-alt', 'hover:text-text-primary hover:bg-surface-hover', - 'group-hover:visible group-focus-within:visible group-[.final-completion]:visible', + 'group-hover:visible group-focus-visible:visible group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:visible group-[.final-completion]:visible', 'focus-visible:ring-2 focus-visible:ring-text-primary focus-visible:outline-none', ); diff --git a/client/src/components/Chat/Messages/__tests__/styles.spec.ts b/client/src/components/Chat/Messages/__tests__/styles.spec.ts index 87653176d0..922761539c 100644 --- a/client/src/components/Chat/Messages/__tests__/styles.spec.ts +++ b/client/src/components/Chat/Messages/__tests__/styles.spec.ts @@ -1,13 +1,70 @@ -import { hoverButtonClasses, messageFooterClasses } from '../styles'; +import { hoverButtonClasses, messageFooterClasses, revealOnRowHoverClasses } from '../styles'; const FADE = '[@media(hover:hover)]:opacity-0'; +/** The row-wide keyboard-focus condition, as two variants. It cannot be one + * `group-[&:is(...)]`: that form makes Tailwind emit a bare `.group$` rule + * that lightningcss rejects, which fails the production CSS build. */ +const FOCUS_SELF = 'group-focus-visible'; +const FOCUS_DESCENDANT = 'group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]'; + +describe('revealOnRowHoverClasses', () => { + it('reveals on row hover and on keyboard focus', () => { + expect(revealOnRowHoverClasses).toContain(FADE); + expect(revealOnRowHoverClasses).toContain('group-hover:opacity-100'); + expect(revealOnRowHoverClasses).toContain(`${FOCUS_DESCENDANT}:opacity-100`); + }); + + /* Clicking a tool card in the message body parks focus there. `:focus-within` + would hold the footer open with the pointer nowhere near the row. */ + it('does not reveal on plain focus-within', () => { + expect(revealOnRowHoverClasses).not.toContain('group-focus-within:'); + }); + + /* MessageNav moves the reader by focusing the row itself through + tabindex="-1", and :has() never matches its own subject. */ + it('reveals when the row element itself is focus-visible', () => { + expect(revealOnRowHoverClasses).toContain(`${FOCUS_SELF}:opacity-100`); + }); + + /* Text-entry controls match :focus-visible even on a mouse click, and + ToolApproval and AskUserQuestion both render textareas inside a row. */ + it('ignores focus that landed in a text-entry control', () => { + expect(revealOnRowHoverClasses).toContain(':not(:is(input,textarea,[contenteditable]))'); + }); + + /* A bare `.group$` rule in the emitted CSS fails the lightningcss minify + step, so the variant form itself is worth pinning. */ + it('does not use a group-[&:...] variant', () => { + expect(revealOnRowHoverClasses).not.toContain('group-[&'); + }); + + it('fades on the shared motion role rather than snapping', () => { + expect(revealOnRowHoverClasses).toContain('duration-theme-normal'); + expect(revealOnRowHoverClasses).toContain('ease-out'); + expect(revealOnRowHoverClasses).toContain('motion-reduce:transition-none'); + }); + + /* `cn` merges the whole `transition-*` group, so a bare `transition-opacity` + would replace the `transition-colors` a `Button` contributes and the hover + tint would snap. The colour properties have to ride along explicitly. */ + it('names the colour properties alongside opacity', () => { + expect(revealOnRowHoverClasses).toContain('transition-[opacity,color,background-color]'); + expect(revealOnRowHoverClasses).not.toContain('transition-opacity'); + }); +}); + describe('hoverButtonClasses', () => { it('fades an idle action out until the row is hovered', () => { expect(hoverButtonClasses()).toContain(FADE); expect(hoverButtonClasses()).toContain('group-hover:opacity-100'); }); + it('reveals an idle action on keyboard focus, not on a click parking focus in the row', () => { + expect(hoverButtonClasses()).toContain(`${FOCUS_DESCENDANT}:opacity-100`); + expect(hoverButtonClasses()).not.toContain('group-focus-within:'); + }); + it('marks an active action so the toolbar can key off it', () => { expect(hoverButtonClasses({ isActive: true })).toContain('hover-button-active'); expect(hoverButtonClasses({ isActive: true })).toContain('active'); diff --git a/client/src/components/Chat/Messages/styles.ts b/client/src/components/Chat/Messages/styles.ts index 4ed8962b39..e51efff1c9 100644 --- a/client/src/components/Chat/Messages/styles.ts +++ b/client/src/components/Chat/Messages/styles.ts @@ -3,11 +3,40 @@ import { cn } from '~/utils'; /** * Reveal-on-hover for a control that shares the message footer with the hover actions. * - * Pointer devices fade it out until the row is hovered or something inside it takes - * focus. Touch devices, which cannot hover, keep it visible. + * Pointer devices fade it out until the row is hovered or keyboard focus lands inside + * it. Touch devices, which cannot hover, keep it visible. + * + * The focus half is `:focus-visible`, not `:focus-within`: clicking a tool card or an + * expand toggle in the message body leaves focus parked there, and `:focus-within` + * would hold the whole footer open with the pointer nowhere near the row. An action + * that opens a surface keeps the toolbar up through `hover-button-active` instead. + * + * That focus half reads + * `:is(:focus-visible, :has(:focus-visible:not(:is(input, textarea, [contenteditable]))))` + * on the row, and both halves earn their keep: + * + * - The row itself has to be tested, not only its descendants. `MessageNav` moves the + * reader by setting `tabindex="-1"` on the row and focusing it, and `:has()` never + * matches its own subject, so a plain `:has(:focus-visible)` leaves a focused row + * showing its focus ring with its metadata and its actions still hidden. + * - Text-entry controls are excluded from the descendant half, because they match + * `:focus-visible` even when a mouse clicks them. `ToolApproval` and + * `AskUserQuestion` both render textareas inside a row, and without the exclusion + * clicking one pins the row open with the pointer somewhere else entirely. Every + * toolbar action is a button, so a keyboard user still never focuses a hidden one. + * + * The two halves stay two variants on purpose. Folding them into a single + * `group-[&:is(...)]` makes Tailwind emit a bare `.group$ { opacity: 1 }` rule, which + * lightningcss rejects and which fails the production CSS build while leaving `jest` + * and `tsc` perfectly green. + * + * The transition names `color` and `background-color` alongside `opacity` rather than + * naming opacity alone: `cn` merges the whole `transition-*` group, so a bare + * `transition-opacity` here would replace the `transition-colors` a `Button` brings and + * the hover tint would snap instead of fading. */ export const revealOnRowHoverClasses = - 'group-focus-within:opacity-100 group-hover:opacity-100 [@media(hover:hover)]:opacity-0'; + 'transition-[opacity,color,background-color] duration-theme-normal ease-out group-hover:opacity-100 group-focus-visible:opacity-100 group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:opacity-100 motion-reduce:transition-none [@media(hover:hover)]:opacity-0'; /** * The message footer, holding the height of its action row. @@ -48,7 +77,7 @@ export const hoverButtonClasses = ({ cn( 'hover-button size-auto rounded-lg p-1.5 text-text-secondary-alt', 'hover:text-text-primary hover:bg-surface-hover', - 'group-hover:visible group-focus-within:visible group-[.final-completion]:visible', + 'group-hover:visible group-focus-visible:visible group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:visible group-[.final-completion]:visible', !isLast && revealOnRowHoverClasses, 'group-has-[.hover-button-active]:visible group-has-[.hover-button-active]:opacity-100', 'focus-visible:ring-2 focus-visible:ring-text-primary focus-visible:outline-none', diff --git a/client/src/components/Chat/Messages/ui/HeaderLabel.tsx b/client/src/components/Chat/Messages/ui/HeaderLabel.tsx index ed9c59ecbe..60a9bc550a 100644 --- a/client/src/components/Chat/Messages/ui/HeaderLabel.tsx +++ b/client/src/components/Chat/Messages/ui/HeaderLabel.tsx @@ -24,15 +24,24 @@ export function getHeaderModelName( } /** Both names occupy one grid cell so the slot is sized by the longer of the - * two and neither reflows the header as they cross over. */ + * two and neither reflows the header as they cross over. + * + * Timed off the same motion role as the timestamp and the footer actions, so a + * keyboard focus that reveals all three lands them together instead of staggering + * across the card-resize spring this used to borrow. */ const labelSlot = - '[grid-area:1/1] truncate transition-[opacity,transform,filter] [transition-duration:var(--resize-dur)] [transition-timing-function:var(--resize-ease)] motion-reduce:transition-none motion-reduce:transform-none motion-reduce:blur-none'; + '[grid-area:1/1] truncate transition-[opacity,transform,filter] duration-theme-normal ease-out motion-reduce:transition-none motion-reduce:transform-none motion-reduce:blur-none'; /** Provider name that crossfades to the model name. A pointer swaps it on the - * label itself; focusing anything in the message row swaps it too, so a + * label itself; keyboard focus landing on the message row swaps it too, so a * sighted keyboard user reaches the model the same way they reach the * timestamp. The model is additionally carried in text that never hides, for - * screen readers that never move the visual focus at all. */ + * screen readers that never move the visual focus at all. + * + * The focus condition is the row-wide one documented on + * `revealOnRowHoverClasses` in `../styles`: the row itself or a descendant + * that is not a text-entry control. Both halves matter here for the same + * reasons they matter to the timestamp and the footer actions. */ export default function HeaderLabel({ label, hoverLabel }: HeaderLabelProps) { const localize = useLocalize(); @@ -46,7 +55,9 @@ export default function HeaderLabel({ label, hoverLabel }: HeaderLabelProps) { className={cn( labelSlot, 'group-hover/label:-translate-y-1 group-hover/label:opacity-0 group-hover/label:blur-[2px]', - 'group-focus-within:-translate-y-1 group-focus-within:opacity-0 group-focus-within:blur-[2px]', + 'group-focus-visible:-translate-y-1 group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:-translate-y-1', + 'group-focus-visible:opacity-0 group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:opacity-0', + 'group-focus-visible:blur-[2px] group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:blur-[2px]', )} > {label} @@ -57,7 +68,9 @@ export default function HeaderLabel({ label, hoverLabel }: HeaderLabelProps) { labelSlot, 'translate-y-1 opacity-0 blur-[2px]', 'group-hover/label:translate-y-0 group-hover/label:opacity-100 group-hover/label:blur-0', - 'group-focus-within:translate-y-0 group-focus-within:opacity-100 group-focus-within:blur-0', + 'group-focus-visible:translate-y-0 group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:translate-y-0', + 'group-focus-visible:opacity-100 group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:opacity-100', + 'group-focus-visible:blur-0 group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]:blur-0', )} > {hoverLabel} diff --git a/client/src/components/Chat/Messages/ui/MessageTimestamp.tsx b/client/src/components/Chat/Messages/ui/MessageTimestamp.tsx index 8be7ad4b2c..54679046e9 100644 --- a/client/src/components/Chat/Messages/ui/MessageTimestamp.tsx +++ b/client/src/components/Chat/Messages/ui/MessageTimestamp.tsx @@ -19,7 +19,8 @@ function TimestampText({ title={timestamp.isRecent ? timestamp.absolute : undefined} className={cn( 'message-timestamp text-xs font-normal text-text-secondary', - revealOnHover && 'ml-2 transition-opacity duration-200', + revealOnHover && + 'ml-2 transition-opacity duration-theme-normal ease-out motion-reduce:transition-none', className, )} > diff --git a/client/src/components/Chat/Messages/ui/__tests__/HeaderLabel.spec.tsx b/client/src/components/Chat/Messages/ui/__tests__/HeaderLabel.spec.tsx index 27f2112d12..2af04d85ab 100644 --- a/client/src/components/Chat/Messages/ui/__tests__/HeaderLabel.spec.tsx +++ b/client/src/components/Chat/Messages/ui/__tests__/HeaderLabel.spec.tsx @@ -38,11 +38,28 @@ describe('HeaderLabel', () => { /* A pointer swap alone would strand a sighted keyboard user, who reaches the row by focus. `.message-render` carries the `group` this keys off. */ - it('also swaps the model in when the message row takes focus', () => { + it('also swaps the model in when the message row takes keyboard focus', () => { render(); - expect(screen.getByText('Ollama')).toHaveClass('group-focus-within:opacity-0'); - expect(screen.getByText('gemma4:12b-it-qat')).toHaveClass('group-focus-within:opacity-100'); + const self = 'group-focus-visible'; + const descendant = 'group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]'; + + const provider = screen.getByText('Ollama').className; + const model = screen.getByText('gemma4:12b-it-qat').className; + + expect(provider).toContain(`${self}:opacity-0`); + expect(provider).toContain(`${descendant}:opacity-0`); + expect(model).toContain(`${self}:opacity-100`); + expect(model).toContain(`${descendant}:opacity-100`); + }); + + /* Clicking a tool card inside the row leaves focus on it. `:focus-within` + would hold the model in place with the pointer nowhere near the row. */ + it('does not key the swap off plain focus-within', () => { + render(); + + expect(screen.getByText('Ollama').className).not.toContain('group-focus-within:'); + expect(screen.getByText('gemma4:12b-it-qat').className).not.toContain('group-focus-within:'); }); /* Screen readers never move the visual focus, so the model also has to reach diff --git a/client/src/style.css b/client/src/style.css index 842d795243..909bfa1f79 100644 --- a/client/src/style.css +++ b/client/src/style.css @@ -582,14 +582,27 @@ pre { } /* Show the message time when the row is hovered. Tailwind group-hover - loses to `[@media(hover:hover)]:opacity-0` (same specificity, :where()). */ + loses to `[@media(hover:hover)]:opacity-0` (same specificity, :where()). + + Keyboard focus reveals it too, but only `:focus-visible`: a pointer that + clicks a tool card or an expand toggle inside the row leaves focus sitting + there, and plain `:focus-within` would pin the time open long after the + pointer moved away. + + The condition matches the row itself as well as its descendants, because + `MessageNav` focuses the row through `tabindex="-1"` and `:has()` never + matches its own subject. Text-entry controls are excluded, because they + match `:focus-visible` on a plain mouse click. See `revealOnRowHoverClasses` + in `components/Chat/Messages/styles.ts` for the full reasoning. */ @media (hover: hover) { .message-render .message-timestamp { opacity: 0; } .message-render:hover .message-timestamp, - .message-render:focus-within .message-timestamp { + .message-render:focus-visible .message-timestamp, + .message-render:has(:focus-visible:not(:is(input, textarea, [contenteditable]))) + .message-timestamp { opacity: 1; } }