-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Enhance Attached File Display (增强附件显示) #250
base: main
Are you sure you want to change the base?
Conversation
添加了 nextjs 文档的直接链接
Added direct link to next docs
📝 Walkthrough📝 WalkthroughWalkthrough此次更改涉及 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: 1
🧹 Outside diff range and nitpick comments (1)
docs/playground/independent.tsx (1)
311-330
: 建议优化样式管理方式当前实现存在以下问题:
- 内联样式不利于维护和复用
- 样式值使用魔法数字
- 颜色值未使用主题变量
建议将样式迁移到
useStyle
中:const useStyle = createStyles(({ token, css }) => { return { + attachmentContainer: css` + display: flex; + align-items: center; + `, + attachmentName: css` + margin-left: ${token.marginXS}px; + padding: ${token.paddingXS}px ${token.padding}px; + border-radius: ${token.borderRadiusLG}px; + background-color: ${token.colorPrimaryBg}; + border: 1px solid ${token.colorPrimaryBorder}; + display: inline-flex; + align-items: center; + justify-content: center; + font-weight: ${token.fontWeightStrong}; + `, // ... 其他样式 }; });然后更新组件实现:
- <div style={{ display: 'flex', alignItems: 'center' }}> + <div className={styles.attachmentContainer}> <Button type="text" icon={<PaperClipOutlined />} /> {attachedFileName && ( - <span - style={{ - marginLeft: '8px', - padding: '4px 12px', - borderRadius: '20px', - backgroundColor: '#cbdcfc', - border: '1px solid #a0c1e6', - display: 'inline-flex', - alignItems: 'center', - justifyContent: 'center', - fontWeight: 'bold', - }} - > + <span className={styles.attachmentName}> {attachedFileName} </span> )} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/playground/independent.tsx
(4 hunks)
🔇 Additional comments (2)
docs/playground/independent.tsx (2)
20-21
: 导入更改看起来不错!
新增的图标和类型导入符合需求,且放置位置合适。
Also applies to: 27-27
204-205
: 状态管理实现合理!
使用 useState 管理附件文件名状态,实现简单清晰。
docs/playground/independent.tsx
Outdated
const handleFileChange = (info: UploadChangeParam) => { | ||
const { fileList } = info; | ||
if (fileList.length > 0) { | ||
setAttachedFileName(fileList[0].name); | ||
} else { | ||
setAttachedFileName(''); | ||
} | ||
}; |
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.
🛠️ Refactor suggestion
建议增强文件处理的健壮性
当前实现较为基础,建议添加以下安全检查:
- 文件大小限制
- 文件类型验证
- 错误处理机制
建议参考以下实现:
const handleFileChange = (info: UploadChangeParam) => {
const { fileList } = info;
if (fileList.length > 0) {
+ const file = fileList[0];
+
+ // 检查文件大小(例如:10MB限制)
+ const isLargeFile = file.size > 10 * 1024 * 1024;
+ if (isLargeFile) {
+ message.error('文件大小不能超过10MB');
+ return;
+ }
+
+ // 检查文件类型
+ const allowedTypes = ['image/jpeg', 'image/png', 'application/pdf'];
+ if (!allowedTypes.includes(file.type)) {
+ message.error('不支持的文件类型');
+ return;
+ }
+
setAttachedFileName(fileList[0].name);
} else {
setAttachedFileName('');
}
};
Committable suggestion skipped: line range outside the PR's diff.
First of all, thank you very much for your contribution! I reviewed the Preview page and noticed a few issues: the current interaction doesn't quite align with our RICH visual guidelines. Here is an example of the recommended interaction: If possible, could you adjust the interaction in this PR according to this example? Thank you very much! 首先非常感谢您的贡献! 我查看了 Preview 页面,有一些问题:现在的交互不太符合我们的 RICH 视觉规范。 推荐的交互示例:https://x.ant.design/components/attachments-cn?theme=dark#attachments-demo-with-sender 如果可以,您可以按这个交互完成此 PR,非常感谢! |
Bundle ReportBundle size has no change ✅ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #250 +/- ##
=======================================
Coverage 91.13% 91.13%
=======================================
Files 66 66
Lines 1421 1421
Branches 364 364
=======================================
Hits 1295 1295
Misses 126 126 ☔ View full report in Codecov by Sentry. |
Thanks for the feedback! I’ll adjust the interaction to align with the visual guidelines you provided, and make another PR shortly. |
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
🧹 Outside diff range and nitpick comments (2)
docs/playground/independent.tsx (2)
29-33
: 建议为接口添加文档注释和类型验证建议添加以下改进:
- 为接口添加 JSDoc 文档注释,说明每个字段的用途
- 考虑添加可选的 type 字段用于文件类型验证
+/** + * 附件文件的接口定义 + * @interface AttachedFile + * @property {string} uid - 文件唯一标识符 + * @property {string} name - 文件名称 + * @property {number} size - 文件大小(字节) + * @property {string} [type] - 文件MIME类型 + */ interface AttachedFile { uid: string; name: string; size: number; + type?: string; }
322-329
: 建议提升附件显示的可访问性当前实现可以通过以下方面改进用户体验:
- 为附件按钮添加
aria-label
属性- 添加文件删除功能
- 显示文件大小的人性化格式
-<Button type="text" icon={<PaperClipOutlined />} /> +<Button + type="text" + icon={<PaperClipOutlined />} + aria-label="添加附件" +/> <Space direction="vertical" style={{ marginTop: 8 }}> {attachedFiles.map((file) => ( - <Attachments.FileCard key={file.uid} item={file} /> + <Attachments.FileCard + key={file.uid} + item={{ + ...file, + size: formatFileSize(file.size) + }} + onDelete={() => handleDeleteFile(file.uid)} + /> ))} </Space>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/playground/independent.tsx
(4 hunks)
🔇 Additional comments (1)
docs/playground/independent.tsx (1)
298-310
:
需要增强文件处理的安全性
当前实现缺少必要的安全检查,建议:
- 添加文件大小限制
- 实现文件类型白名单验证
- 添加重复文件检查
@YumoImer I've made the updates in the current push: Let me know if it satisfies the guidelines. 我已经进行了更新。如果符合指南要求,请告诉我。 |
Thank you very much! I have reviewed the preview page, and the attachment style now conforms to the RICH specification. Regarding the attachment upload interaction, I have recorded an example from the official website Attachment - Combination which can be implemented as shown. The final UI should look like this: If possible, could you adjust the interaction in this PR? Thank you very much! |
Pull Request: Enhance Attached File Display
The new feature adds a visually enhanced display for attached files, presenting the file name within a styled circular border to improve user experience and clarity.
新功能为文件附件显示添加了视觉增强效果,文件名现在以圆形边框和按钮样式呈现,以提高用户体验和可识别性。
Overview:
This PR enhances the file attachment feature by styling the attached file name for better visibility and user experience.
Key Changes:
Impact:
This enhancement makes it clearer for users when files are attached, contributing to a more intuitive chat interface.
Testing:
The feature has been tested across various browsers to ensure consistent functionality and appearance.
Summary by CodeRabbit
PaperClipOutlined
。AttachedFile
,定义附加文件的结构,包括uid
、name
和size
属性。attachedFiles
,用于管理已附加文件的数组。