-
Notifications
You must be signed in to change notification settings - Fork 180
feat(FilesCard): 添加previewTeleported属性,支持将预览图片弹窗teleport到body下 #311
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
base: main
Are you sure you want to change the base?
feat(FilesCard): 添加previewTeleported属性,支持将预览图片弹窗teleport到body下 #311
Conversation
WalkthroughAdds a new optional boolean prop previewTeleported to FilesCard, wires it into el-image’s preview-teleported attribute, updates type definitions, Storybook argTypes, and English/Chinese docs with table formatting adjustments. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FC as FilesCard
participant EI as ElImage (preview)
participant DOM as DOM
User->>FC: Open image preview
FC->>EI: render with preview-teleported = previewTeleported
alt previewTeleported = true
EI->>DOM: Teleport preview overlay to <body>
else previewTeleported = false
EI->>DOM: Render preview overlay in local container
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/core/src/components/FilesCard/types.d.ts (2)
36-36: Typo breaks type-checking: CSSPropertiess → CSSProperties.This will fail TS declaration emit and consumers’ type-checking.
Apply this diff:
- hoverStyle?: CSSPropertiess; + hoverStyle?: CSSProperties;
49-49: Typo breaks type-checking: numbe → number.This prevents correct typing of percent.
Apply this diff:
- percent?: numbe; + percent?: number;packages/core/src/components/FilesCard/index.vue (1)
127-136: Fix computed ref usage and confirm correct preview methodPlease update the conditional in
handlePreviewActionto unwrap the computed ref and use the correctshowPreviewAPI. No fallback toshowVieweris needed—Element Plus v2.x exposesshowPreview().Files to update:
- packages/core/src/components/FilesCard/index.vue (around lines 127–136)
Minimal diff:
--- a/packages/core/src/components/FilesCard/index.vue +++ b/packages/core/src/components/FilesCard/index.vue @@ 127,136c127,136 - const imgRef = ref(); + const imgRef = ref<{ showPreview: () => void } | null>(null); function handlePreviewAction(type: 'self' | 'mask') { - if (props.imgPreview && imgRef.value && _previewImgUrl && type === 'mask') { - imgRef.value!.showPreview(); + if (props.imgPreview && imgRef.value && _previewImgUrl.value && type === 'mask') { + imgRef.value.showPreview(); } if (type === 'self') { emits('imagePreview', { ...props }); } }
🧹 Nitpick comments (4)
apps/docs/en/components/filesCard/index.md (1)
54-54: Polish previewTeleported description (grammar/clarity).Current: “whether to append image-viewer to body. A nested parent element attribute transform should have this attribute set to true.”
Suggested:
- “Whether to teleport the image viewer into document.body. If a parent element applies CSS transforms or creates a stacking context, set this to true.”
Apply this diff:
-| `previewTeleported` | `boolean` | No | `false` | whether to append image-viewer to body. A nested parent element attribute transform should have this attribute set to true. | +| `previewTeleported` | `boolean` | No | `false` | Whether to teleport the image viewer into `document.body`. If a parent element applies CSS transforms or creates a stacking context, set this to `true`. |apps/docs/zh/components/filesCard/index.md (1)
55-55: 优化描述,提升可读性与准确性。当前文案略显拗口:“image-viewer 是否插入至 body 元素上。 嵌套的父元素属性会发生修改时应该将此属性设置为 true”。
建议:
- “是否将图片预览(image-viewer)通过 teleport 挂载到 document.body。若父级元素使用了 transform 等属性造成定位/层级异常,建议设为 true。”
-| `previewTeleported` | `boolean` | 否 | `false` | image-viewer 是否插入至 body 元素上。 嵌套的父元素属性会发生修改时应该将此属性设置为 true | +| `previewTeleported` | `boolean` | 否 | `false` | 是否将图片预览(image-viewer)通过 teleport 挂载到 `document.body`。若父级元素使用了 `transform` 等属性造成定位/层级异常,建议设为 `true`。 |packages/core/src/components/FilesCard/index.vue (1)
104-121: Optional: Revoke object URLs to prevent memory leaks.
previewImagelikely creates an object URL. Revoke the previous one on change/unmount.Example:
let lastObjectUrl: string | undefined; watch( () => props.imgFile, async (newFile) => { if (newFile) { try { const url = await previewImage(newFile); if (lastObjectUrl && lastObjectUrl.startsWith('blob:')) URL.revokeObjectURL(lastObjectUrl); _previewImg.value = url; lastObjectUrl = url; } catch (error) { console.error('Preview failed:', error); } } else { if (lastObjectUrl && lastObjectUrl.startsWith('blob:')) URL.revokeObjectURL(lastObjectUrl); lastObjectUrl = undefined; _previewImg.value = undefined; } }, { deep: true, immediate: true } ); // also in onBeforeUnmount: onBeforeUnmount(() => { if (lastObjectUrl && lastObjectUrl.startsWith('blob:')) URL.revokeObjectURL(lastObjectUrl); });Note: import onBeforeUnmount if not auto-imported in this project.
packages/core/src/stories/FilesCard/FilesCard.stories.ts (1)
134-150: Optional: Add a story that demonstrates teleporting preview.Showcasing the prop helps validate behavior in iframes/portals.
You can add a variant:
export const TeleportedPreviewDemo: Story = { args: { ...FilesCardDemo.args, imgPreview: true, previewTeleported: true, fileType: 'image', url: 'https://picsum.photos/600/400' } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/docs/en/components/filesCard/index.md(1 hunks)apps/docs/zh/components/filesCard/index.md(1 hunks)packages/core/src/components/FilesCard/index.vue(2 hunks)packages/core/src/components/FilesCard/types.d.ts(1 hunks)packages/core/src/stories/FilesCard/FilesCard.stories.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/components/FilesCard/types.d.ts (1)
packages/core/src/components/Attachments/types.d.ts (1)
FileListProps(4-15)
🪛 LanguageTool
apps/docs/en/components/filesCard/index.md
[grammar] ~54-~54: There might be a mistake here.
Context: ...hould have this attribute set to true. | | status | `'uploading' | ...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ... | | percent | number ...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ... | | errorTip | string ...
(QB_NEW_EN)
🔇 Additional comments (7)
packages/core/src/components/FilesCard/types.d.ts (2)
41-42: Prop addition looks correct; name matches Element Plus’ viewer “teleport” concept.The new prop is well placed between imgPreview and imgPreviewMask and defaulted downstream. No functional concerns from types here.
22-22: Docs vs. types mismatch for uid “requiredness”.Interface marks uid as optional, while docs (both zh/en) list it as required. Please align one way or the other.
If uid is optional, update the tables to Required = No. If it’s truly required at runtime, change the type to
uid: string | number;.apps/docs/en/components/filesCard/index.md (1)
37-37: Make uid “Required = No” to match the type.Docs currently say Yes; the type is optional.
Apply this diff if uid is optional:
-| `uid` | `string \| number` | Yes | | File unique identifier | +| `uid` | `string \| number` | No | `undefined` | File unique identifier |apps/docs/zh/components/filesCard/index.md (1)
37-37: 文档与类型不一致:uid 是否必填。接口里 uid 为可选,这里标注为“是”。请统一。
如果可选,请改为“否”:
-| `uid` | `string \| number` | 是 | | 文件唯一标识符 | +| `uid` | `string \| number` | 否 | `undefined` | 文件唯一标识符 |packages/core/src/components/FilesCard/index.vue (2)
27-27: Default for previewTeleported = false is sensible.Keeps behavior unchanged for existing consumers while enabling opt-in teleporting.
194-199: Forwarding prop to el-image looks right; confirm Element Plus prop/event names.
- Attribute name
preview-teleportedaligns with Element Plus naming. Good.- Double-check
@showevent is the correct hook for image viewer opening in your Element Plus version.If your EL version uses a different event (e.g.,
@open, etc.), update accordingly to avoid missed telemetry.packages/core/src/stories/FilesCard/FilesCard.stories.ts (1)
105-109: Storybook control added correctly.ArgType wiring and description look good; exposes the new behavior to consumers.
| | `name` | `string` | No | `undefined` | File name (supports automatic suffix parsing to match icons) | | ||
| | `fileSize` | `number` | No | `undefined` | File size (unit: bytes, automatically converted to readable format) | | ||
| | `fileType` | `string` | No | `undefined` | File type (priority higher than `name` suffix parsing, e.g. `'image'`, `'document'`) | | ||
| | `description` | `string` | No | `undefined` | Description text (supports dynamic generation of file type and size info) | |
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.
Invalid example value for fileType (“document”).
The union in code does not include 'document'. Replace with valid options or remove the example to avoid misleading users.
Apply this diff (one option):
-| `fileType` | `string` | No | `undefined` | File type (priority higher than `name` suffix parsing, e.g. `'image'`, `'document'`) |
+| `fileType` | `string` | No | `undefined` | File type (priority higher than `name` suffix parsing; valid values include `'word'`, `'excel'`, `'ppt'`, `'pdf'`, `'txt'`, `'mark'`, `'image'`, `'audio'`, `'video'`, `'three'`, `'code'`, `'database'`, `'link'`, `'zip'`, `'file'`, `'unknown'`). |📝 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.
| | `description` | `string` | No | `undefined` | Description text (supports dynamic generation of file type and size info) | | |
| | `fileType` | `string` | No | `undefined` | File type (priority higher than `name` suffix parsing; valid values include `'word'`, `'excel'`, `'ppt'`, `'pdf'`, `'txt'`, `'mark'`, `'image'`, `'audio'`, `'video'`, `'three'`, `'code'`, `'database'`, `'link'`, `'zip'`, `'file'`, `'unknown'`). | | |
| | `description` | `string` | No | `undefined` | Description text (supports dynamic generation of file type and size info) | |
🤖 Prompt for AI Agents
In apps/docs/en/components/filesCard/index.md around line 41, the example value
"document" for fileType is invalid because the prop's union does not include
'document'; update the table to either use a valid union member (e.g., replace
"document" with one of the actual allowed values) or remove the example value
entirely so it doesn't mislead readers, and ensure any accompanying text
reflects the chosen valid option.
| | `name` | `string` | 否 | `undefined` | 文件名(支持自动解析后缀匹配图标) | | ||
| | `fileSize` | `number` | 否 | `undefined` | 文件大小(单位:字节,自动转换为易读格式) | | ||
| | `fileType` | `string` | 否 | `undefined` | 文件类型(优先级高于 `name` 后缀解析,如 `'image'`、`'document'`) | | ||
| | `description` | `string` | 否 | `undefined` | 描述文本(支持动态生成文件类型和大小信息) | |
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.
fileType 示例值不准确(含有不存在的 'document')。
类型联合中没有 'document'。请使用实际可用的取值,或移除该示例以免误导使用者。
-| `fileType` | `string` | 否 | `undefined` | 文件类型(优先级高于 `name` 后缀解析,如 `'image'`、`'document'`) |
+| `fileType` | `string` | 否 | `undefined` | 文件类型(优先级高于 `name` 后缀解析;可选:`'word'`、`'excel'`、`'ppt'`、`'pdf'`、`'txt'`、`'mark'`、`'image'`、`'audio'`、`'video'`、`'three'`、`'code'`、`'database'`、`'link'`、`'zip'`、`'file'`、`'unknown'`) |📝 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.
| | `description` | `string` | 否 | `undefined` | 描述文本(支持动态生成文件类型和大小信息) | | |
| | `fileType` | `string` | 否 | `undefined` | 文件类型(优先级高于 `name` 后缀解析;可选:`'word'`、`'excel'`、`'ppt'`、`'pdf'`、`'txt'`、`'mark'`、`'image'`、`'audio'`、`'video'`、`'three'`、`'code'`、`'database'`、`'link'`、`'zip'`、`'file'`、`'unknown'`) | |
🤖 Prompt for AI Agents
In apps/docs/zh/components/filesCard/index.md around line 41, the example value
'document' in the `fileType` description is incorrect because the union type
does not include 'document'; remove or replace it with an actual allowed value
from the component's `fileType` union (or simply omit the example) and update
the description text so it no longer references the nonexistent 'document'
value.
#299
1.FilesCard组件新增previewTeleported属性
2.修改中英文文档
3.添加storybook
Summary by CodeRabbit