-
Notifications
You must be signed in to change notification settings - Fork 33
feat: 修改澄清模式继续按钮行为为填充输入框 #148
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
实现功能: - 点击澄清表单的"继续"按钮后,不再直接发送消息 - 而是将 Markdown 格式的回答内容填充到 ChatInput 组件中 - 用户可以继续编辑后再手动发送 技术实现: 1. 扩展 TaskContext,添加 appendToInput 方法和 inputRef 2. 修改 ChatInput 组件,支持接收 ref 参数 3. 修改 ChatArea 组件,注册输入框 setter 和 ref 到 Context 4. 修改 ClarificationForm,改变 handleSubmit 逻辑为调用 appendToInput 5. 添加国际化文案 text_filled 用户体验改进: - 澄清表单保持显示状态,不隐藏 - 支持多次点击继续按钮,内容累加到输入框 - 自动聚焦到输入框并平滑滚动 - 显示成功提示 toast
WalkthroughThis PR exposes and centralizes programmatic control of the chat input via TaskContext (adds Changes
Sequence Diagram(s)sequenceDiagram
participant ClarificationForm
participant TaskContext
participant ChatArea
participant ChatInput
participant Textarea
rect rgba(220,235,255,0.6)
Note over TaskContext: TaskContext new API (inputRef, registerInputSetter, appendToInput)
ChatArea->>TaskContext: registerInputSetter(setter)
TaskContext-->>ChatArea: store setter
ChatArea->>ChatInput: pass inputRef
ChatInput->>Textarea: ref={inputRef}
end
rect rgba(230,255,220,0.6)
ClarificationForm->>TaskContext: appendToInput(markdown)
TaskContext->>TaskContext: use setter to update input (format/append)
TaskContext->>Textarea: focus & scroll into view
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/src/features/tasks/components/ChatArea.tsx(4 hunks)frontend/src/features/tasks/components/ChatInput.tsx(3 hunks)frontend/src/features/tasks/components/ClarificationForm.tsx(3 hunks)frontend/src/features/tasks/contexts/taskContext.tsx(4 hunks)frontend/src/i18n/locales/en/chat.json(1 hunks)frontend/src/i18n/locales/zh-CN/chat.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/features/tasks/components/ClarificationForm.tsx (1)
frontend/src/features/tasks/contexts/taskContext.tsx (1)
useTaskContext(310-316)
🪛 Biome (2.1.2)
frontend/src/features/tasks/components/ChatArea.tsx
[error] 65-65: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Frontend
🔇 Additional comments (12)
frontend/src/i18n/locales/en/chat.json (1)
140-141: LGTM!The new localization entries are clear and properly formatted. The "text_filled" message appropriately communicates the new behavior where answers are filled into the input for editing rather than sent directly.
frontend/src/i18n/locales/zh-CN/chat.json (1)
138-139: LGTM!The Chinese translations are appropriate and consistent with the English version. The localization properly supports the new input-filling behavior.
frontend/src/features/tasks/components/ChatInput.tsx (2)
20-20: LGTM!The addition of the optional
inputRefprop to the interface is well-typed and enables external components to access the textarea DOM element for programmatic control.
72-72: LGTM!The ref is correctly forwarded to the underlying
TextareaAutosizecomponent, enabling the external control flow introduced in this PR.frontend/src/features/tasks/components/ClarificationForm.tsx (2)
30-30: LGTM!Correctly imports the new
appendToInputfunction from TaskContext to enable the new input-filling behavior.
223-229: LGTM!The refactored behavior appropriately fills the markdown answer into the input instead of sending it directly. The toast feedback clearly communicates the action to the user.
frontend/src/features/tasks/components/ChatArea.tsx (1)
396-396: LGTM!The
inputRefis correctly passed to bothChatInputinstances (in no-messages and messages views), enabling the centralized input control mechanism.Also applies to: 483-483
frontend/src/features/tasks/contexts/taskContext.tsx (5)
37-39: LGTM!The new context API properties are well-defined and properly typed, enabling centralized input management across the task components.
56-58: LGTM!The refs are appropriately created to hold the textarea element reference and the setter function for input state management.
251-270: LGTM!The
appendToInputimplementation is well-designed:
- Correctly uses the setState updater function pattern to safely append text
- Properly formats with double newline separation when existing content is present
- Includes appropriate focus and scroll behavior for good UX
- Gracefully handles the case when the setter isn't initialized
272-275: LGTM!The
registerInputSetterfunction correctly stores the setter reference for use byappendToInput.
298-300: LGTM!The new API methods are correctly exposed through the context provider value.
| // Register the input setter with TaskContext | ||
| useEffect(() => { | ||
| registerInputSetter(setTaskInputMessage); | ||
| }, [registerInputSetter]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Move this useEffect after destructuring registerInputSetter.
This useEffect references registerInputSetter before it's declared on line 78, causing a ReferenceError. JavaScript hoisting doesn't apply here since registerInputSetter is a const from destructuring.
Apply this fix by moving the destructuring before the useEffect:
const [taskInputMessage, setTaskInputMessage] = useState('');
const [isLoading, setIsLoading] = useState(false);
const [_error, setError] = useState('');
- // Register the input setter with TaskContext
- useEffect(() => {
- registerInputSetter(setTaskInputMessage);
- }, [registerInputSetter]);
-
const chatAreaRef = useRef<HTMLDivElement>(null);
const scrollContainerRef = useRef<HTMLDivElement>(null);
const isUserNearBottomRef = useRef(true);
const floatingInputRef = useRef<HTMLDivElement>(null);
const AUTO_SCROLL_THRESHOLD = 32;
const router = useRouter();
const searchParams = useSearchParams();
const [floatingMetrics, setFloatingMetrics] = useState({ width: 0, left: 0 });
const [inputHeight, setInputHeight] = useState(0);
// New: Get selectedTask to determine if there are messages
const { selectedTaskDetail, refreshTasks, refreshSelectedTaskDetail, setSelectedTask, inputRef, registerInputSetter } =
useTaskContext();
+
+ // Register the input setter with TaskContext
+ useEffect(() => {
+ registerInputSetter(setTaskInputMessage);
+ }, [registerInputSetter]);
+
const hasMessages = Boolean(selectedTaskDetail && selectedTaskDetail.id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Register the input setter with TaskContext | |
| useEffect(() => { | |
| registerInputSetter(setTaskInputMessage); | |
| }, [registerInputSetter]); | |
| const [taskInputMessage, setTaskInputMessage] = useState(''); | |
| const [isLoading, setIsLoading] = useState(false); | |
| const [_error, setError] = useState(''); | |
| const chatAreaRef = useRef<HTMLDivElement>(null); | |
| const scrollContainerRef = useRef<HTMLDivElement>(null); | |
| const isUserNearBottomRef = useRef(true); | |
| const floatingInputRef = useRef<HTMLDivElement>(null); | |
| const AUTO_SCROLL_THRESHOLD = 32; | |
| const router = useRouter(); | |
| const searchParams = useSearchParams(); | |
| const [floatingMetrics, setFloatingMetrics] = useState({ width: 0, left: 0 }); | |
| const [inputHeight, setInputHeight] = useState(0); | |
| // New: Get selectedTask to determine if there are messages | |
| const { selectedTaskDetail, refreshTasks, refreshSelectedTaskDetail, setSelectedTask, inputRef, registerInputSetter } = | |
| useTaskContext(); | |
| // Register the input setter with TaskContext | |
| useEffect(() => { | |
| registerInputSetter(setTaskInputMessage); | |
| }, [registerInputSetter]); | |
| const hasMessages = Boolean(selectedTaskDetail && selectedTaskDetail.id); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 65-65: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/ChatArea.tsx around lines 62-65
(current useEffect) and line ~78 (destructuring), the useEffect calls
registerInputSetter before registerInputSetter is created via destructuring,
causing a ReferenceError; move the destructuring that defines
registerInputSetter to above this useEffect so registerInputSetter is
initialized before being referenced, and keep the dependency array as
[registerInputSetter] (or update it if the variable name changes).
| {!isSubmitted && ( | ||
| <div className="flex justify-end pt-2"> | ||
| <Button variant="secondary" onClick={handleSubmit} disabled={isSubmitting} size="lg"> | ||
| <Button variant="secondary" onClick={handleSubmit} size="lg"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider preventing rapid clicking.
The submit button is no longer disabled during the append operation. While validation still runs, users could rapidly click the button to append the same content multiple times. Consider adding temporary disabled state or debouncing if this is not the intended behavior.
If multiple appends are intentional, you can safely ignore this comment. Otherwise, consider adding:
+ const [isAppending, setIsAppending] = useState(false);
const handleSubmit = async () => {
+ if (isAppending) return;
+ setIsAppending(true);
+
// ... validation logic ...
appendToInput(markdownAnswer);
toast({
title: t('clarification.text_filled') || 'Answer filled to input, you can continue editing',
});
+
+ setTimeout(() => setIsAppending(false), 500);
};
// ...
- <Button variant="secondary" onClick={handleSubmit} size="lg">
+ <Button variant="secondary" onClick={handleSubmit} size="lg" disabled={isAppending}>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/ClarificationForm.tsx around line 270,
the submit Button is clickable multiple times during the append operation; add a
temporary disabled state (e.g., isAppending or isSubmitting) and wire it into
the Button's disabled prop and any form controls, set that flag to true at the
start of the append handler and false when the async append completes (use
try/finally), and additionally guard the handler with an early return if the
flag is already true (or implement a short debounce) to prevent rapid double
submissions; optionally show a spinner/loader while disabled for UX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/features/tasks/contexts/taskContext.tsx (3)
38-40: Simplify the inputRef type.The
inputReftype is declared asReact.RefObject<HTMLTextAreaElement | null>, butRefObjectalready includesnullin its.currentproperty type. This is redundant.Apply this diff to simplify:
- inputRef: React.RefObject<HTMLTextAreaElement | null>; + inputRef: React.RefObject<HTMLTextAreaElement>;
264-264: Remove redundant type annotation.The type annotation
(prev: string)is unnecessary since TypeScript infers this from the setter's signature.Apply this diff:
- setTaskInputMessageRef.current((prev: string) => { + setTaskInputMessageRef.current((prev) => {
269-275: Consider cleanup and timing improvements for focus logic.The
setTimeoutlacks cleanup if the component unmounts during the delay. Additionally,scrollIntoViewwith smooth scrolling might interrupt the user's current focus or scroll position.Consider these improvements:
- Add timeout cleanup:
+ const timeoutId = setTimeout(() => { - setTimeout(() => { if (inputRef.current) { inputRef.current.focus(); inputRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' }); } }, 100); + return () => clearTimeout(timeoutId);However, since this function is not in a
useEffect, you'd need to track and clean up timeouts differently, or consider using a ref to track mounted state.
- Alternative: Use
requestAnimationFramefor more reliable timing:requestAnimationFrame(() => { if (inputRef.current) { inputRef.current.focus(); inputRef.current.scrollIntoView({ behavior: 'smooth', block: 'center' }); } });
- UX consideration: The
scrollIntoViewbehavior might be jarring. Consider making scroll optional or usingbehavior: 'auto'for instant scroll.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/features/tasks/contexts/taskContext.tsx(4 hunks)
🔇 Additional comments (3)
frontend/src/features/tasks/contexts/taskContext.tsx (3)
58-62: LGTM!The ref initialization pattern is correct. Using a ref to hold the setter function avoids re-renders while maintaining access to the latest setter.
278-281: LGTM!The
registerInputSetterimplementation is straightforward and correct.
305-307: LGTM!The new APIs are correctly exposed through the context provider and match the declared
TaskContextTypeinterface.
功能概述
在澄清模式下,修改"继续"按钮的行为:点击后不再直接发送消息,而是将 Markdown 格式的回答内容填充到当前任务的主输入框(ChatInput 组件)中,允许用户继续编辑后再手动发送。
技术实现
1. 扩展 TaskContext
appendToInput(text: string)方法,负责追加文本、聚焦输入框、滚动到输入框inputRef用于存储输入框的 refregisterInputSetter方法,用于注册输入框的 setter 函数2. 修改 ChatInput 组件
inputRef?: React.RefObject<HTMLTextAreaElement>3. 修改 ChatArea 组件
inputRef和registerInputSettersetTaskInputMessage到 ContextinputRef传递给两处 ChatInput 组件4. 修改 ClarificationForm 组件
appendToInput方法handleSubmit逻辑:移除消息发送代码,改为调用appendToInput(markdownAnswer)isSubmitting状态和相关禁用逻辑text_filled文案5. 国际化文案
text_filled: "回答已填充到输入框,您可以继续编辑"text_filled: "Answer filled to input, you can continue editing"用户体验改进
测试场景
代码更改
frontend/src/features/tasks/contexts/taskContext.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/ChatInput.tsxfrontend/src/features/tasks/components/ClarificationForm.tsxfrontend/src/i18n/locales/zh-CN/chat.jsonfrontend/src/i18n/locales/en/chat.jsonSummary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.