refactor(client): make one control move a question between chat and composer

The popover carried both a chevron and a close button. Both left the
question pending and both moved it into the chat, so their only real
difference was invisible: the chevron kept the composer wired up as the
answer box while the close button released it. Two icons for one visible
outcome, with the meaning hidden in a placeholder change somewhere else.

Collapsing now does both jobs at once. Moving a question to the chat hands
the composer back for normal messages, and the card's chevron re-arms it,
so the popover's visibility and the composer's role can never disagree.
Escape maps to the same path and the dismissed-ids state is gone. Both
chevrons get a tooltip and a matching label, since no glyph conveys this.

Also fixes two behaviours found along the way: the card's Skip was a
silent no-op once the question left the popover, because it routed through
a helper gated on answer mode instead of the live pause, and clicking a
multi-select row moved the keyboard highlight, leaving a row painted as
selected after it was unchecked.
This commit is contained in:
Marco Beretta 2026-07-31 18:59:21 +02:00
parent e71c471e48
commit 1a2b70aa84
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
3 changed files with 63 additions and 76 deletions

View file

@ -1,7 +1,7 @@
import { memo, useEffect, useRef } from 'react';
import { useWatch } from 'react-hook-form';
import { Button, TooltipAnchor } from '@librechat/client';
import { ChevronDown, CornerDownLeft, TriangleAlert, X } from 'lucide-react';
import { ChevronDown, CornerDownLeft, TriangleAlert } from 'lucide-react';
import AskUserQuestions from '~/components/Chat/Messages/Content/AskUserQuestions';
import useAskAnswerMode from '~/hooks/Input/useAskAnswerMode';
import AskOptions from '~/components/Chat/ask/options';
@ -43,7 +43,7 @@ function AskUserQuestionPopoverContent({
function AskUserQuestionsPopoverPanel({ ask }: { ask: ReturnType<typeof useAskAnswerMode> }) {
const localize = useLocalize();
const { liveAsk, collapse, dismiss } = ask;
const { liveAsk, collapse } = ask;
const questions = liveAsk?.questions;
if (liveAsk == null || questions == null || questions.length === 0) {
return null;
@ -59,24 +59,20 @@ function AskUserQuestionsPopoverPanel({ ask }: { ask: ReturnType<typeof useAskAn
{ 0: questions.length },
)}
</p>
<div className="flex items-center">
<button
type="button"
aria-label={localize('com_ui_collapse')}
className="rounded p-1 text-text-secondary hover:bg-surface-hover"
onClick={collapse}
>
<ChevronDown className="h-4 w-4" aria-hidden="true" />
</button>
<button
type="button"
aria-label={localize('com_ui_close')}
className="rounded p-1 text-text-secondary hover:bg-surface-hover"
onClick={dismiss}
>
<X className="h-4 w-4" aria-hidden="true" />
</button>
</div>
<TooltipAnchor
description={localize('com_ui_ask_move_to_chat')}
side="top"
render={
<button
type="button"
aria-label={localize('com_ui_ask_move_to_chat')}
className="rounded-md p-1 text-text-secondary transition-colors hover:bg-surface-hover hover:text-text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy"
onClick={collapse}
>
<ChevronDown className="size-4" aria-hidden="true" />
</button>
}
/>
</div>
<AskUserQuestions actionId={liveAsk.actionId} questions={questions} />
</div>

View file

@ -42,7 +42,6 @@ export default function AskUserQuestion({
questions={questions}
className="my-2 max-h-[70vh] w-full rounded-lg border border-border-light bg-surface-secondary"
onExpand={answerMode.collapsed && isLivePause ? answerMode.expand : undefined}
onDismiss={answerMode.collapsed && isLivePause ? answerMode.dismiss : undefined}
/>
);
}
@ -67,9 +66,9 @@ function AskUserQuestionSingle({
/**
* The composer popover is the primary answer surface while it's VISIBLE
* for this pause, rendering the card too duplicates the question. The card
* takes over when the popover is collapsed (answer mode stays live; the
* chevron re-expands it) or dismissed (and in contexts without a
* ChatContext, where the popover can't exist).
* takes over once the question is moved to the chat (the popover's chevron,
* which also releases the composer; this card's chevron moves it back), and
* in contexts without a ChatContext, where the popover can't exist.
*/
const { popoverVisible, collapsed, expand, liveAsk } = answerMode;
const isLivePause = liveAsk?.actionId === actionId;
@ -120,7 +119,7 @@ function AskUserQuestionSingle({
};
/** `answerMode.skip()` is gated on answer mode being ACTIVE, which a
* dismissed (×'d) question is not and that is precisely when this card
* question moved to the chat is not and that is precisely when this card
* is the only surface left. Decline through the answer path instead,
* which is gated on the live pause rather than on answer mode. */
const handleSkip = () => {

View file

@ -9,18 +9,18 @@ import {
findLiveAskUserQuestion,
splitOtherOption,
} from '~/utils/approval';
import { getAskAnswerDraftId, morphTransition } from '~/utils';
import { useGetMessagesByConvoId } from '~/data-provider';
import { useOptionalChatFormContext } from '~/Providers';
import { getAskAnswerDraftId } from '~/utils';
import store from '~/store';
/** Dismissed action ids — recoil so every consumer reacts. */
const dismissedAskActionsAtom = atom<string[]>({
key: 'askAnswerModeDismissedActions',
default: [],
});
/** Collapsed action ids: popover chrome hidden, answer mode still live. */
/**
* Action ids the user moved into the chat: the popover is hidden AND the
* composer is released (answer mode off), so the chat card is the question's
* only surface until the card's chevron moves it back. One state for one
* user-visible concept a hidden popover whose composer stayed armed was
* indistinguishable from one whose composer did not.
*/
const collapsedAskActionsAtom = atom<string[]>({
key: 'askAnswerModeCollapsedActions',
default: [],
@ -51,11 +51,10 @@ const askAnswerCheckedAtom = atom<number[]>({
* conversation draft is stashed on entry and restored once the question
* resolves.
*
* Two ways out short of answering: `collapse` hides the popover chrome but
* KEEPS the pause live (the question renders in the chat card, which still
* answers it), while the × `dismiss` exits answer mode entirely. `Skip`
* resumes the run with a canned decline notice. Collapsing always returns the
* composer to normal chat see `composerLocked`.
* One way out short of answering: `collapse` (the popover's chevron, or
* Escape) moves the question to the chat card AND releases the composer, so
* the user can type a normal message; the card's chevron moves it back.
* `Skip` resumes the run with a canned decline notice.
*
* `handleComposerKeyDown` only steers selection from the EMPTY composer and
* reports whether it consumed the key.
@ -71,7 +70,6 @@ export default function useAskAnswerMode(conversationId?: string | null) {
select: findLiveAskUserQuestion,
});
const liveAsk = enabled ? (liveAskData ?? null) : null;
const [dismissedIds, setDismissedIds] = useRecoilState(dismissedAskActionsAtom);
const [collapsedIds, setCollapsedIds] = useRecoilState(collapsedAskActionsAtom);
const [selected, setSelected] = useRecoilState(askAnswerSelectionAtom);
const [checked, setChecked] = useRecoilState(askAnswerCheckedAtom);
@ -111,7 +109,6 @@ export default function useAskAnswerMode(conversationId?: string | null) {
* no-op so a double-click or a stray Skip can't race a second resume. */
const status = liveAsk != null ? getAskStatus(liveAsk.actionId) : 'idle';
const locked = status === 'submitting' || status === 'submitted' || status === 'expired';
const dismissed = liveAsk != null && dismissedIds.includes(liveAsk.actionId);
/**
* An EXPIRED question can no longer be answered, so it drops out of answer
* mode entirely: the popover closes, the composer reverts to a normal
@ -127,22 +124,17 @@ export default function useAskAnswerMode(conversationId?: string | null) {
* submit whose store write couldn't run) holds the popover open over an
* answered question with every option greyed out.
*/
const active = liveAsk != null && !dismissed && status !== 'expired' && status !== 'submitted';
const collapsed = active && collapsedIds.includes(liveAsk.actionId);
/** The popover renders only while expanded; collapse keeps `active` but
* hands the question display to the chat card. */
const popoverVisible = active && !collapsed;
const batchMode = (liveAsk?.questions?.length ?? 0) > 0;
const answerable = liveAsk != null && status !== 'expired' && status !== 'submitted';
/** Moved to the chat: the card owns the question and the composer is free. */
const collapsed = answerable && collapsedIds.includes(liveAsk.actionId);
/**
* Which role the composer plays for this pause. A single question is
* answered IN the composer, so it stays live for as long as the pause does.
* A batch is answered in its own card, so the composer has nothing to
* contribute and locks but ONLY while the popover is up. Collapsing has to
* hand the composer back, because every other way out is gone once the
* popover closes: the stop button hides behind `composerAnswers` and Escape
* would reach a disabled textarea. Staying locked past collapse left no way
* to type, send, or stop the run short of reloading the page.
* Answer mode: the popover is up AND the composer is the free-form answer
* box. The two are deliberately the same condition the composer's answer
* role is only discoverable while the popover explains it.
*/
const active = answerable && !collapsed;
const popoverVisible = active;
const batchMode = (liveAsk?.questions?.length ?? 0) > 0;
const composerAnswers = active && !batchMode;
const composerLocked = popoverVisible && batchMode;
const multiSelect = !batchMode && liveAsk != null && liveAsk.question.multiSelect === true;
@ -164,36 +156,38 @@ export default function useAskAnswerMode(conversationId?: string | null) {
setChecked([]);
}, [liveAsk?.actionId, setSelected, setChecked]);
const dismiss = useCallback(() => {
if (liveAsk) {
setDismissedIds((prev) =>
prev.includes(liveAsk.actionId) ? prev : [...prev, liveAsk.actionId],
);
}
}, [liveAsk, setDismissedIds]);
/** Popover chat-card handoffs run inside a view transition: both
* surfaces carry the same `view-transition-name`, so the browser morphs
* one into the other instead of swapping. Both are user-event driven,
* which morphTransition's synchronous flush requires. */
const collapse = useCallback(() => {
if (liveAsk) {
setCollapsedIds((prev) =>
prev.includes(liveAsk.actionId) ? prev : [...prev, liveAsk.actionId],
morphTransition(() =>
setCollapsedIds((prev) =>
prev.includes(liveAsk.actionId) ? prev : [...prev, liveAsk.actionId],
),
);
}
}, [liveAsk, setCollapsedIds]);
const expand = useCallback(() => {
if (liveAsk) {
setCollapsedIds((prev) => prev.filter((id) => id !== liveAsk.actionId));
morphTransition(() =>
setCollapsedIds((prev) => prev.filter((id) => id !== liveAsk.actionId)),
);
}
}, [liveAsk, setCollapsedIds]);
/** Pure check toggle: the keyboard highlight is steered only by the
* composer's digit/arrow shortcuts, so a mouse toggle never leaves a
* row painted `selected` after it is unchecked. */
const toggleChecked = useCallback(
(index: number) => {
setSelected(index);
setChecked((prev) =>
prev.includes(index) ? prev.filter((i) => i !== index) : [...prev, index],
);
},
[setSelected, setChecked],
[setChecked],
);
const canSubmit =
@ -203,8 +197,8 @@ export default function useAskAnswerMode(conversationId?: string | null) {
/**
* Shared answer dispatch: sends the run's resume and clears the phase.
* Gated on the live pause (NOT `active` the chat card must still answer a
* dismissed question) and on `locked` (no duplicate resumes while one is in
* Gated on the live pause (NOT `active`, because the chat card must still
* answer a collapsed question) and on `locked` (no duplicate resumes while one is in
* flight).
*
* The selection/composer cleanup runs ONLY after the resume is accepted (in
@ -339,8 +333,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
/**
* Explicitly decline: resumes the run with a canned notice so the model
* knows the user chose not to answer. A client-side dismiss alone would
* leave the run paused until expiry a hung turn.
* knows the user chose not to answer.
*/
const skip = useCallback((): boolean => {
if (!active) {
@ -389,7 +382,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
*/
if (options.length === 0 || !popoverVisible) {
if (e.key === 'Escape') {
dismiss();
collapse();
return true;
}
return false;
@ -398,6 +391,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
if (!Number.isNaN(digit) && digit >= 1 && digit <= Math.min(options.length, 9)) {
e.preventDefault();
if (multiSelect) {
setSelected(digit - 1);
toggleChecked(digit - 1);
} else {
setSelected(digit - 1);
@ -420,7 +414,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
return true;
}
if (e.key === 'Escape') {
dismiss();
collapse();
return true;
}
return false;
@ -436,7 +430,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
submit,
submitText,
toggleChecked,
dismiss,
collapse,
setSelected,
],
);
@ -475,8 +469,6 @@ export default function useAskAnswerMode(conversationId?: string | null) {
batchMode,
liveAsk,
options,
dismissed,
dismiss,
collapsed,
collapse,
expand,