-
Notifications
You must be signed in to change notification settings - Fork 236
imprv: Hide summary mode switch in editor assistant mode #9897
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
imprv: Hide summary mode switch in editor assistant mode #9897
Conversation
|
|
|
||
| const clickQuickMenuHandler = async(quickMenu: string) => { | ||
| isQuickMenuReqRef.current = true; | ||
| await onSubmit({ input: quickMenu }); |
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.
onSubmit への引数に、markdown データ取得の種類 (送らない / ユーザーが選択中の箇所を送る / 全文を送る) を選択して付与できない?
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.
FB に基づいて改修しました
commit: 7b2d7b5
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.
Pull Request Overview
This PR hides the summary mode switch in editor assistant mode and refactors the related messaging functions to consistently use a FormData object rather than separate parameters. Key changes include updating service interfaces in both knowledge assistant and editor assistant, adding a new view to generate the summary mode switch for knowledge assistant, and conditionally rendering input controls in the sidebar based on the active assistant mode.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/app/src/features/openai/client/services/knowledge-assistant.tsx | Updated function signatures and added summary mode switch view |
| apps/app/src/features/openai/client/services/editor-assistant.tsx | Refactored postMessage and added form/resetForm support |
| apps/app/src/features/openai/client/components/AiAssistant/AiAssistantSidebar/AiAssistantSidebar.tsx | Adjusted sidebar form usage and conditional rendering to hide summary switch in editor mode |
Comments suppressed due to low confidence (1)
apps/app/src/features/openai/client/components/AiAssistant/AiAssistantSidebar/AiAssistantSidebar.tsx:102
- The identifier 'formForForKnowledgeAssistant' appears to have a repeated 'For' which may be a typo. It is recommended to rename it to 'formForKnowledgeAssistant' for better clarity.
const form = isEditorAssistant ? formForForEditorAssistant : formForForKnowledgeAssistant;
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.
Pull Request Overview
This PR improves the AI assistant functionality by hiding the summary mode switch for the editor assistant while maintaining it for the knowledge assistant. Key updates include changes to the form data structure and request payloads, conditionally rendering the summary mode switch, and unifying form handling in the sidebar.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/app/src/features/openai/client/services/knowledge-assistant.tsx | Updated form handling and request payload for knowledge assistant with summary mode switch view |
| apps/app/src/features/openai/client/services/editor-assistant.tsx | Revised form data to support markdown types and removed the summary mode control for editor assistant |
| apps/app/src/features/openai/client/components/AiAssistant/AiAssistantSidebar/AiAssistantSidebar.tsx | Unified form selection and conditionally hid the summary mode switch for the editor assistant |
Comments suppressed due to low confidence (1)
apps/app/src/features/openai/client/components/AiAssistant/AiAssistantSidebar/AiAssistantSidebar.tsx:102
- [nitpick] The variable name 'form' is ambiguous when it conditionally represents data for different assistants. Consider renaming it (e.g. 'activeForm') to improve code clarity.
const form = isEditorAssistant ? formForEditorAssistant : formForKnowledgeAssistant;
| // positionRef.current = 0; | ||
| } | ||
| }, [codeMirrorEditor, detectedDiff, selectedText, yDocs?.secondaryDoc]); | ||
| }, [detectedDiff]); |
Copilot
AI
Apr 28, 2025
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.
The dependency array for this effect has been reduced to only 'detectedDiff', but the effect body also references codeMirrorEditor, isTextSelected, and yDocs?.secondaryDoc. To avoid potential stale data issues, consider adding these dependencies.
| }, [detectedDiff]); | |
| }, [detectedDiff, setSelectedText, setDetectedDiff, lineRef]); |
|
This pull request has been removed from the queue for the following reason: The merge conditions cannot be satisfied due to failing checks:
You may have to fix your CI before adding the pull request to the queue again. |
Task