mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-29 05:20:49 +00:00
🖱️ fix: Reveal Message Metadata on Hover, Not on Click (#14900)
* fix: reveal message metadata on hover, not on click
The message timestamp, the provider/model label crossfade, and the hover
action toolbar all revealed on `:focus-within` over the message row. A mouse
click sets focus, so clicking a tool card, an expand toggle, or a code block
button parked focus inside the row and pinned all three open long after the
pointer had left.
Key the focus half of each reveal on `:focus-visible` instead. A pointer
click no longer counts, while keyboard focus still does, so a sighted
keyboard user still reaches the model name and the timestamp by tabbing. An
action that opens a surface keeps the toolbar up through `hover-button-active`
as before.
* fix: fade the message row reveal instead of snapping it
The footer actions carried no opacity transition at all, so they arrived in a
single frame while the timestamp eased in behind them over 200ms and the
provider/model crossfade ran on the 300ms card-resize spring it had borrowed.
One hover, three different arrivals.
Put all three on the shared `duration-theme-normal` motion role with a common
ease-out, and add the reduced-motion guard the timestamp and the footer were
missing. `MinimalHoverButtons` now composes the shared reveal helper rather
than repeating its classes inline.
The reveal transition names `color` and `background-color` alongside `opacity`
because `cn` merges the whole `transition-*` group: a bare `transition-opacity`
would replace the `transition-colors` a `Button` contributes and the hover tint
would snap.
* fix: widen the message row keyboard-focus test
Two gaps in the `:focus-visible` reveal, both raised in review.
`:has()` never matches its own subject, so keying the reveal on
`:has(:focus-visible)` missed the row element itself. `MessageNav` moves the
reader by setting `tabindex="-1"` on the row and focusing it, which left a
focused row showing its focus ring while its timestamp, its model name and its
actions all stayed hidden.
Text-entry controls match `:focus-visible` even when a mouse clicks them, so a
click into the textareas `ToolApproval` and `AskUserQuestion` render inside a
row still pinned that row's metadata open with the pointer somewhere else. They
are excluded from the descendant half of the test. Every toolbar action is a
button, so a keyboard user still never focuses a hidden one.
Both halves are now one condition,
`:is(:focus-visible, :has(:focus-visible:not(:is(input, textarea, [contenteditable]))))`,
applied to the timestamp, the header label and the footer actions alike.
* fix: split the row focus test into two variants
Folding the row-itself and descendant halves into a single
`group-[&:is(...)]` made Tailwind emit a bare `.group$ { opacity: 1 }`, which
lightningcss refuses to minify. That failed the client CSS build and every job
downstream of it while jest and tsc stayed green, because neither ever builds
the stylesheet.
The condition is unchanged in behaviour, expressed as `group-focus-visible`
plus `group-has-[:focus-visible:not(:is(input,textarea,[contenteditable]))]`.
The plain CSS in style.css keeps the `:is()` form, which is valid there.
The stale string also had to come out of the specs: tailwind scans
`src/**/*.{ts,tsx}`, so a class literal in a test file reaches the production
stylesheet.
* fix: split the timestamp focus selector too
`:has()` nested inside `:is()` made postcss log "Failed to parse selector" on
every client build. The rule survived intact, but the warning was noise coming
from this change, and splitting it matches how the Tailwind side now expresses
the same condition.
Behaviour is unchanged: hover, a focused row and a mouse-clicked textarea all
measure the same as before.
This commit is contained in:
parent
7d850c308a
commit
df5abbb377
8 changed files with 156 additions and 19 deletions
|
|
@ -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)}
|
||||
>
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)}
|
||||
>
|
||||
|
|
|
|||
|
|
@ -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(<HeaderLabel label="Ollama" hoverLabel="gemma4:12b-it-qat" />);
|
||||
|
||||
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(<HeaderLabel label="Ollama" hoverLabel="gemma4:12b-it-qat" />);
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue