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 ef251a481d
commit 5bada1fab6
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
2 changed files with 45 additions and 41 deletions

View file

@ -34,9 +34,9 @@ export default function AskUserQuestion({
/**
* 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 conversationId = useContext(ChatContext)?.conversation?.conversationId;
const answerMode = useAskAnswerMode(conversationId);
@ -89,7 +89,7 @@ export default function AskUserQuestion({
};
/** `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,10 +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 answer mode live (the question renders in the chat card; the composer
* still answers), while the × `dismiss` exits answer mode entirely. `Skip`
* resumes the run with a canned decline notice.
* 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.
@ -70,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);
@ -87,7 +86,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
@ -103,11 +101,16 @@ 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` (and the
* composer's answer role) but hands the question display to the chat card. */
const popoverVisible = active && !collapsed;
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);
/**
* 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 multiSelect = liveAsk != null && liveAsk.question.multiSelect === true;
/** Answer-phase draft key: handed to useAutoSave so the composer drafts
* under the question's own key while answer mode is live, leaving the
@ -126,36 +129,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 =
@ -317,7 +322,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
*/
if (options.length === 0 || !popoverVisible) {
if (e.key === 'Escape') {
dismiss();
collapse();
return true;
}
return false;
@ -326,6 +331,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);
@ -348,7 +354,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
return true;
}
if (e.key === 'Escape') {
dismiss();
collapse();
return true;
}
return false;
@ -363,7 +369,7 @@ export default function useAskAnswerMode(conversationId?: string | null) {
submit,
submitText,
toggleChecked,
dismiss,
collapse,
setSelected,
],
);
@ -401,8 +407,6 @@ export default function useAskAnswerMode(conversationId?: string | null) {
active,
liveAsk,
options,
dismissed,
dismiss,
collapsed,
collapse,
expand,