🙋 feat: Refine the Ask Question Card and Smooth Batched Question Steps - #16641
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 063719a0d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| selected: | ||
| question.multiSelect === true || value.length === 0 | ||
| value.trim().length === 0 | ||
| ? previous.selected | ||
| : { ...previous.selected, [question.id]: [] }, |
There was a problem hiding this comment.
This changes how the existing askQuestionsFormState is written when users type answers, so the atom is now part of the touched feature area, but it remains a Recoil atomFamily. Convert this atom and all of its readers/writers together to Jotai rather than extending behavior on the legacy store.
AGENTS.md reference: AGENTS.md:L161-L164
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1dc826e: the batch form state is now a Jotai atomFamily owned by useAskQuestionsForm, its only reader and writer. Popover and chat card share it, verified by scenario ask-batch-answers-survive-move-to-chat.
| <div className="absolute bottom-full z-10 mb-2 w-full"> | ||
| <div className="popover border-border-light bg-surface-secondary flex flex-col rounded-2xl border shadow-lg [view-transition-name:ask-question]"> |
There was a problem hiding this comment.
When a valid batch contains a long question or description, this absolutely positioned, bottom-anchored popover can grow upward beyond the viewport because the overall max-h-[70vh] cap was removed and only the answer area is scrollable. The backend permits up to 2,000 question characters plus a 4,000-character description, so the prompt can become unreadable above the viewport with no way to scroll to it; retain a viewport cap and put the prompt in a bounded scroll region.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1dc826e: the prompt block is capped at 25vh with its own scroll. Verified with a 2,000-character question plus 4,000-character description by scenario ask-batch-long-prompt-fits-viewport.
| className="flex flex-col" | ||
| /> | ||
| )} | ||
| <Input |
There was a problem hiding this comment.
In the composer popover, this single-line Input is nested inside ChatForm's outer <form>, so Enter performs implicit form submission rather than editing or confirming the batch answer. When the paused conversation already has composer text, the enabled during-run submit control receives that implicit submission and routes the unrelated composer draft through the queue/send path, while this answer is not registered with that form; prevent the outer submission or explicitly handle Enter for the batch field.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1dc826e: Enter in a batch answer field is handled there (next question, or submit on the last) and never reaches the chat form. Verified by scenario ask-batch-enter-confirms-answer, which asserts no new generation starts and the resume carries all three answers.
| option: | ||
| 'w-full select-none justify-start gap-2.5 whitespace-normal text-left font-normal text-text-primary transition-opacity enabled:duration-500 disabled:duration-500 disabled:transition-all hover:bg-surface-hover hover:active:bg-surface-pressed data-[selected=true]:bg-surface-active data-[selected=true]:hover:bg-surface-active', |
There was a problem hiding this comment.
When prefers-reduced-motion: reduce is enabled, locking or unlocking an option still runs the newly added 500 ms opacity/color transition because this variant has no motion-reduce override. Add a reduced-motion transition override so the shared option variant does not animate for users who requested less motion.
AGENTS.md reference: AGENTS.md:L150-L151
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1dc826e: the option variant adds motion-reduce:transition-none, including for disabled rows. Verified by scenario ask-option-rows-skip-fade-under-reduced-motion.
codegraph-librechat
Bot
added
the
🗺️ Agent Chat UI
063719a to
f34db3d
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
5c413df to
14cdb71
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14cdb71175
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div className="absolute bottom-full z-10 mb-2 w-full"> | ||
| {/* The prompt and answers each scroll within their own caps; this outer cap keeps | ||
| the whole card on screen when both are at their limit on a short viewport. */} | ||
| <div className="popover border-border-light bg-surface-secondary flex max-h-[70vh] flex-col overflow-y-auto rounded-2xl border shadow-lg [view-transition-name:ask-question]"> |
There was a problem hiding this comment.
For a valid batch where the prompt reaches its 25vh cap and several long options make the answer region reach 45vh, those regions plus the nonshrinking footer exceed this 70vh wrapper. The current AskUserQuestions child is allowed to shrink (min-h-0), and its AutoHeight child uses overflow-hidden, so flex layout clips the bottom of the answer region rather than creating overflow for this wrapper to scroll; the final options or text input can therefore remain unreachable. Fresh evidence beyond the earlier long-prompt thread is the current combination of this outer cap with the shrinking, clipping AutoHeight; make that content nonshrinking or put the scroll boundary outside the clipping element.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cb61b3a: the batch card is shrink-0 inside the 70vh popover, so the popover scrolls instead of AutoHeight clipping the answers. Scenario ask-batch-long-prompt-fits-viewport now uses six long options and asserts the answer field is reachable; it fails without the fix and passes with it.
| if (!isLastStep) { | ||
| if (!navLocked) { | ||
| goToStep(activeIndex + 1); |
There was a problem hiding this comment.
When a keyboard user presses Enter in a non-final answer field, changing the step replaces the keyed fieldset and unmounts the focused input. Fresh evidence after the earlier Enter fix is that this branch calls goToStep without setting refocusRef, unlike the single-select option path, so the step-change effect does not focus the new fieldset and focus falls back to the document body; the next Tab can restart at page chrome instead of the next answer. Set the refocus signal or explicitly focus the new answer before advancing.
AGENTS.md reference: AGENTS.md:L199-L204
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cb61b3a: Enter sets the refocus signal before advancing, so the next question's fieldset takes focus, the same as option selection. Covered by a unit test in AskUserQuestions.test.tsx.
| if (event.key !== 'Enter' || event.nativeEvent.isComposing) { | ||
| return; |
There was a problem hiding this comment.
On browsers where confirming an IME composition reports key === 'Enter' and keyCode === 229 while nativeEvent.isComposing is already false, this handler advances the step or submits the batch with incomplete CJK input. Fresh evidence after the earlier Enter thread is that the new handler omits the Safari/IME fallback already required by useAskAnswerMode and useTextarea; apply the same keyCode === 229 guard before treating Enter as confirmation.
AGENTS.md reference: AGENTS.md:L199-L204
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cb61b3a: the handler ignores keyCode 229 like useAskAnswerMode, so an Enter that confirms an IME composition neither advances nor submits. Covered by a unit test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb61b3ab96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…lder While Locked
… the Next Question
cb61b3a to
dbc0db8
Compare
Summary
Part 2 of the ui-refinments stack (#16640 onto dev, then #16641, #16642, #16643; merge from the bottom up).
The ask question card used its own colors and spacing, a "Ctrl+Shift+X to stop" composer hint, and an "Asking N questions" label above the question, and stepping through a batch of questions animated every title, option and input while the header and buttons slid with the height. The card now sits on
surface-secondary, puts the question title in the header row, uses the standardInputfor typed answers and the numbered option rows from the composer, and the stop hint is gone. Switching questions swaps the content instantly while only the card's outer height eases, with the header and footer pinned to the card edges.Option rows are a new
optionButton variant: instant hover, a slow fade when they lock, and a check mark on the chosen single-select answer. Typing an answer deselects and locks that question's options. While the question is inline in the thread, the locked composer drops the "Set your key" placeholder and its text cannot be selected; Back, Next and option labels cannot be text-selected either. The move-to-chat chevron rotates during the composer and chat morph. AdisclosureButton variant replaces the restyled header button inProgressText.Type of change
Testing
Tested environments/configuration:
Automated tests:
npx jest --findRelatedTestson the changed files, including newButton.spec.tsxcases for theoptionanddisclosurevariants and anAutoHeightspecnpx tsc --noEmit -p client/tsconfig.json, ESLint, Prettier,npm run static-checksScreenshots / recordings
Captured from the e2e mock harness at 1280x800 against the client built before and after this PR.
Risk / compatibility
Removes the
com_ui_composer_hint_stop,com_ui_question_number,com_ui_asking_questionsandcom_ui_asking_questions_oneEnglish keys, which nothing references after this change. The stop shortcut itself still works.Checklist