-
Notifications
You must be signed in to change notification settings - Fork 354
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
Ospp 2024/feat graphic dialogue #791
Ospp 2024/feat graphic dialogue #791
Conversation
…timizing code formatting
…tiny/tiny-engine into ospp-2024/004-ai-multimodal
…tiny/tiny-engine into ospp-2024/004-ai-multimodal
WalkthroughThe changes introduce an image upload feature to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 3
Outside diff range, codebase verification and nitpick comments (1)
packages/plugins/robot/src/Main.vue (1)
556-619
: CSS for Image Display and AnimationThe CSS classes
.chat-message-image
and.preview-image
handle the styling and animation for displayed images. The use of absolute units (px) for dimensions and animations could be improved for responsiveness.Consider using relative units like
em
or%
for better responsiveness across different screen sizes:- width: 100px; - height: 70px; + width: 10em; + height: 7em;Also, review the keyframe animations for potential improvements in performance or visual effect.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/plugins/robot/src/Main.vue (19 hunks)
- packages/plugins/robot/src/js/robotSetting.js (1 hunks)
Files skipped from review due to trivial changes (1)
- packages/plugins/robot/src/js/robotSetting.js
@@ -72,6 +76,7 @@ | |||
<svg-icon name="chat-message" class="common-svg"></svg-icon> | |||
</template> | |||
<template #suffix> | |||
<icon-picture class="common-svg upload-image" @click="openFilePicker"></icon-picture> |
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.
Ensure Accessibility for Image Upload Icon
The image upload icon (icon-picture
) is clickable but lacks accessibility features such as aria-label
or role
attributes. This could hinder accessibility for users relying on screen readers.
Consider adding aria-label
to improve accessibility:
- <icon-picture class="common-svg upload-image" @click="openFilePicker"></icon-picture>
+ <icon-picture class="common-svg upload-image" aria-label="Upload Image" @click="openFilePicker"></icon-picture>
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.
<icon-picture class="common-svg upload-image" @click="openFilePicker"></icon-picture> | |
<icon-picture class="common-svg upload-image" aria-label="Upload Image" @click="openFilePicker"></icon-picture> |
}, 500) | ||
} | ||
|
||
const getMessage = (content, role) => ({ |
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.
Enhance getMessage
Function with Default Parameters
The getMessage
function now requires a role
parameter. To ensure backward compatibility and ease of use, consider providing a default value for the role
.
Add a default value for the role
parameter in getMessage
:
- const getMessage = (content, role) => ({
+ const getMessage = (content, role = 'user') => ({
role,
content,
name: 'John'
})
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.
const getMessage = (content, role) => ({ | |
const getMessage = (content, role = 'user') => ({ |
const fileInput = ref(null) | ||
const openFilePicker = () => { | ||
if (unref(fileInput)) { | ||
unref(fileInput)?.click() | ||
} | ||
} | ||
|
||
const imageUrl = ref('') | ||
const imageContent = ref() | ||
const uploadFile = (file) => { | ||
const formData = new FormData() | ||
const foundationModelData = JSON.stringify({ | ||
foundationModel: { | ||
manufacturer: currentModel.manufacturer, | ||
model: currentModel.value, | ||
token: localStorage.getItem(currentModel.modelKey) | ||
} | ||
}) | ||
formData.append('foundationModel', foundationModelData) | ||
formData.append('file', file) | ||
http | ||
.post('/app-center/api/ai/files', formData, { | ||
headers: { | ||
'Content-Type': 'multipart/form-data' | ||
}, | ||
timeout: 600000 | ||
}) | ||
.then((res) => { | ||
imageContent.value = res.originalResponse | ||
const reader = new FileReader() | ||
reader.readAsDataURL(file) | ||
reader.onload = () => { | ||
imageUrl.value = reader.result | ||
} | ||
}) | ||
.catch(() => { | ||
Notify({ | ||
type: 'error', | ||
message: '上传图片失败', | ||
position: 'top-right', | ||
duration: 5000 | ||
}) | ||
}) | ||
} | ||
|
||
const handleFileChange = (event) => { | ||
const files = event.target.files | ||
if (!files.length) { | ||
return | ||
} | ||
const file = files[0] | ||
const validImageTypes = ['image/jpeg', 'image/png', 'image/jpg'] | ||
if (!validImageTypes.includes(file.type)) { | ||
alert('请上传有效的图片文件(.jpeg, .png, .jpg)!') | ||
event.target.value = '' | ||
return | ||
} | ||
event.target.value = '' | ||
uploadFile(file) | ||
} | ||
|
||
const imageDeleting = ref(false) | ||
const handleDelete = () => { | ||
imageDeleting.value = true | ||
setTimeout(() => { | ||
imageUrl.value = '' | ||
imageContent.value = '' | ||
imageDeleting.value = false | ||
if (unref(fileInput)) { | ||
unref(fileInput).value = '' | ||
} | ||
}, 500) |
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.
Review Image Upload and Deletion Logic
The methods openFilePicker
, handleFileChange
, uploadFile
, and handleDelete
handle the core functionality of image uploading and deletion. Here are some observations and suggestions:
- Error Handling in
uploadFile
: The method catches errors but only displays a notification. It might be beneficial to also reset relevant states or perform additional cleanup. - File Type Validation: The validation for file types is done in
handleFileChange
. Consider extracting this to a separate method for better modularity and reusability. - Animation Handling in
handleDelete
: The deletion uses a timeout to manage animations. This could be replaced with more robust animation handling techniques using Vue's transition features.
Refactor the file type validation into a separate method and improve error handling in uploadFile
:
+ const isValidImageType = (fileType) => ['image/jpeg', 'image/png', 'image/jpg'].includes(fileType);
- if (!validImageTypes.includes(file.type)) {
+ if (!isValidImageType(file.type)) {
alert('请上传有效的图片文件(.jpeg, .png, .jpg)!');
event.target.value = '';
return;
}
Consider using Vue's transition for handling animations in handleDelete
instead of setTimeout
.
Committable suggestion was skipped due to low confidence.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/design-core/vite.config.js (2 hunks)
Additional comments not posted (2)
packages/design-core/vite.config.js (2)
17-17
: Verify the change in theorigin
constant.The
origin
constant has been updated to'http://localhost:7011/'
. Please confirm that this change aligns with the intended development environment settings and that all dependent services are available at this new port.
26-26
: Approved: Added blank line for readability.The addition of a blank line after the
alias
configuration improves the readability of the configuration file by visually separating different sections.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores