-
-
Notifications
You must be signed in to change notification settings - Fork 110
feat: video preview type #450
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: master
Are you sure you want to change the base?
Conversation
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough本次变更在预览系统中引入媒体类型支持,扩展从仅图片到图片与视频两种类型。Image 将 preview.type 传递给 Preview;Preview 根据 type 分支渲染 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant I as Image
participant PG as PreviewGroup
participant P as Preview
participant M as MediaRenderer
rect rgba(238,246,255,0.7)
note over I: 构建注册数据(包含 src 与 type)
U->>I: 点击缩略图
I->>P: open({ src, type })
end
alt 单图或分组打开
P->>M: render(type)
end
par 渲染分支
alt type == 'image'
M->>M: 渲染 <img class="...-img">
P-->>U: 显示 Footer/工具栏
else type == 'video'
M->>M: 渲染 <video class="...-video" controls autoplay>
P-->>U: 不渲染 Footer
end
end
note over P,M: 样式规则对 .-img 与 .-video 应用相同尺寸约束
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md. Please share your feedback with us on this Discord post. ✨ 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. Comment |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Summary of Changes
Hello @guoyunhe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
此拉取请求旨在扩展现有 Image
组件的功能,使其能够支持视频预览。这使得在 Web 应用中处理图片和视频混合内容时更加灵活,用户可以像在社交媒体平台(如 Twitter/X)上一样,以简化的界面预览视频内容,从而提升用户体验。
Highlights
- 新增视频预览类型: 在
PreviewProps
中引入type
属性,支持 'image' 和 'video' 两种预览类型,默认为 'image'。 - 支持视频属性: 为视频预览添加了
videoCommonProps
,允许传入标准的 HTML<video>
标签属性。 - 动态渲染媒体: 根据
type
属性动态渲染<img>
或<video>
元素,实现不同媒体类型的预览。 - 简化视频工具栏: 视频预览模式下,移除了图像操作工具栏(如缩放、旋转等),仅保留翻页和关闭功能,符合描述中“视频预览没有工具栏”的要求。
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
您好,感谢您为 rc-image 贡献代码。这个 PR 增加了视频预览功能,这是一个很棒的增强。我主要有两个反馈点:
- 新添加的
type
和videoCommonProps
属性应该放在InternalPreviewConfig
接口中,以确保它们可以通过Image
组件的preview
prop 正确传递。这是一个关键问题,否则新功能将无法使用。 - 渲染的
<video>
元素没有使用videoCommonProps
,并且缺少默认的播放控件,导致用户无法与视频交互。
具体的修改建议请看我的评论。修复这些问题后,这个功能就更完善了。
src/Preview/index.tsx
Outdated
type?: 'image' | 'video'; | ||
imgCommonProps?: React.ImgHTMLAttributes<HTMLImageElement>; | ||
videoCommonProps?: React.VideoHTMLAttributes<HTMLVideoElement>; |
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.
src/Preview/index.tsx
Outdated
...(groupContext ? { current } : {}), | ||
}) | ||
: imgNode)} | ||
{type === 'video' && <video src={src} width={props.width} height={props.height} />} |
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.
您好,这里新增的 <video>
元素有两点可以改进:
- 没有使用在
PreviewProps
接口中新增的videoCommonProps
属性。为了让使用者可以自定义视频播放器的行为(例如自动播放、循环播放等),建议将videoCommonProps
应用到<video>
元素上。 - 默认情况下视频没有播放控件,用户无法播放、暂停或调整音量。建议默认添加
controls
属性,以便提供基本的播放控制功能。
{type === 'video' && <video src={src} width={props.width} height={props.height} />} | |
{type === 'video' && <video {...props.videoCommonProps} src={src} width={props.width} height={props.height} controls />} |
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
🧹 Nitpick comments (3)
src/Preview/index.tsx (3)
333-350
: 避免抢占视频原生左右键:视频聚焦时放行键盘事件左右方向键在
<video>
上通常用于跳转/快退。当前全局keydown
会在分组预览时拦截左右键,影响视频可用性。建议在视频模式下、当事件目标位于<video>
内时直接 return:const onKeyDown = useEvent((event: KeyboardEvent) => { if (open) { const { keyCode } = event; + // Video mode: respect native arrow key behavior when focus is within <video> + if ( + type === 'video' && + (event.target as Element | null)?.closest?.('video') + ) { + return; + } if (keyCode === KeyCode.ESC) { onClose?.(); }
205-231
: 视频模式下仍初始化图片的交互 Hook(移动/缩放/触摸),可按类型降载这些 Hook 在视频模式没有意义,还会注册监听与产生计算。可用布尔卫语句降载(不违背 Hooks 规则):
- const { transform, resetTransform, updateTransform, dispatchZoomChange } = useImageTransform( + const { transform, resetTransform, updateTransform, dispatchZoomChange } = useImageTransform( imgRef, minScale, maxScale, onTransform, ); - const { isMoving, onMouseDown, onWheel } = useMouseEvent( + const isImage = type === 'image'; + const openForImage = open && isImage; + const movableForImage = movable && isImage; + const { isMoving, onMouseDown, onWheel } = useMouseEvent( imgRef, - movable, - open, + movableForImage, + openForImage, scaleStep, transform, updateTransform, dispatchZoomChange, ); - const { isTouching, onTouchStart, onTouchMove, onTouchEnd } = useTouchEvent( + const { isTouching, onTouchStart, onTouchMove, onTouchEnd } = useTouchEvent( imgRef, - movable, - open, + movableForImage, + openForImage, minScale, transform, updateTransform, dispatchZoomChange, );
462-493
: 视频模式下完全移除 Footer 会失去“X / Y”进度显示;请确认产品预期PR 说明称“无工具栏,仅分页与关闭”。若仍希望在视频模式展示进度(不显示操作按钮),可重用 Footer 并将
actionsRender={() => null}
,保留翻页与进度:- {type === 'image' && ( + {(type === 'image' || showOperationsProgress) && ( <Footer prefixCls={prefixCls} showProgress={showOperationsProgress} current={current} count={count} showSwitch={showLeftOrRightSwitches} // Style classNames={classNames} styles={styles} // Render image={image} transform={transform} icons={icons} countRender={countRender} - actionsRender={actionsRender} + actionsRender={type === 'video' ? () => null : actionsRender} // Scale scale={scale} minScale={minScale} maxScale={maxScale} // Actions onActive={onActive} onFlipY={onFlipY} onFlipX={onFlipX} onRotateLeft={onRotateLeft} onRotateRight={onRotateRight} onZoomOut={onZoomOut} onZoomIn={onZoomIn} onClose={onClose} onReset={onReset} /> )}若确实不需要进度,当前实现也可接受。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Preview/index.tsx
(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Preview/index.tsx (2)
docs/examples/imageRender.tsx (1)
imageRender
(6-27)src/Preview/Footer.tsx (1)
Footer
(46-212)
⏰ 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: WIP
🔇 Additional comments (1)
src/Preview/index.tsx (1)
170-170
: 默认值保持向后兼容,LGTM
type = 'image'
可确保旧用法无感升级。
src/Preview/index.tsx
Outdated
// Preview media | ||
type?: 'image' | 'video'; | ||
imgCommonProps?: React.ImgHTMLAttributes<HTMLImageElement>; | ||
videoCommonProps?: React.VideoHTMLAttributes<HTMLVideoElement>; | ||
width?: string | number; |
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.
新增视频相关 Props 的接线不完整,risk:对外暴露但无效
- 定义了
type?: 'image' | 'video'
与videoCommonProps?
,但渲染分支未透传videoCommonProps
,导致无法开启controls
、poster
等关键交互属性。请在视频渲染处使用它(见后文具体修复)。 src
目前被复用为图片与视频的统一来源;若支持videoCommonProps.src
,需约定优先级(建议以videoCommonProps.src ?? src
合并)。- 文档/示例注意:应使用字符串字面量
type: 'video'
,而不是type: video
。
另:从类型安全角度,建议将 PreviewProps
改为“可辨识联合”,避免 imgCommonProps
与 videoCommonProps
同时暴露造成歧义(示例见下方可选重构)。
可选重构(类型层面,示意):
type ImagePreviewProps = { type?: 'image'; imgCommonProps?: React.ImgHTMLAttributes<HTMLImageElement> };
type VideoPreviewProps = { type: 'video'; videoCommonProps?: React.VideoHTMLAttributes<HTMLVideoElement> };
export type PreviewProps = InternalPreviewConfig & {
prefixCls: string;
classNames?: Partial<Record<PreviewSemanticName, string>>;
styles?: Partial<Record<PreviewSemanticName, React.CSSProperties>>;
imageInfo?: { width: number | string; height: number | string };
fallback?: string;
width?: string | number;
height?: string | number;
current?: number;
count?: number;
onChange?: (current: number, prev: number) => void;
onClose?: () => void;
mousePosition: null | { x: number; y: number };
} & (ImagePreviewProps | VideoPreviewProps);
🤖 Prompt for AI Agents
In src/Preview/index.tsx around lines 124-128, the component defines type?:
'image' | 'video' and videoCommonProps? but the video render branch does not
forward videoCommonProps, so controls/poster/src etc. are ineffective; fix by
spreading videoCommonProps onto the rendered <video> element, and resolve source
precedence by using videoCommonProps.src ?? src (or equivalent prop) for the
video src; optionally, consider converting PreviewProps into a discriminated
union (image vs video) so imgCommonProps and videoCommonProps cannot be
ambiguous.
src/Preview/index.tsx
Outdated
{type === 'image' && | ||
(imageRender | ||
? imageRender(imgNode, { | ||
transform, | ||
image, | ||
...(groupContext ? { current } : {}), | ||
}) | ||
: imgNode)} | ||
{type === 'video' && <video src={src} width={props.width} height={props.height} />} |
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.
视频不可交互(缺少 controls)且未透传 videoCommonProps;请修复
当前 <video>
未加 controls
/playsInline
/preload
,用户无法播放、暂停或在 iOS 内联播放;同时没有接入 videoCommonProps
。建议如下改动(包含默认值与可覆盖能力):
- {type === 'video' && <video src={src} width={props.width} height={props.height} />}
+ {type === 'video' && (
+ <video
+ className={`${prefixCls}-video`}
+ {...props.videoCommonProps}
+ aria-label={alt}
+ src={props.videoCommonProps?.src ?? src}
+ width={props.width}
+ height={props.height}
+ playsInline={props.videoCommonProps?.playsInline ?? true}
+ controls={props.videoCommonProps?.controls ?? true}
+ preload={props.videoCommonProps?.preload ?? 'metadata'}
+ style={{
+ maxWidth: '100%',
+ maxHeight: '100%',
+ objectFit: 'contain',
+ ...(props.videoCommonProps?.style || {}),
+ }}
+ />
+ )}
说明:
- 透传
videoCommonProps
(事件、属性、src
优先级:videoCommonProps.src ?? src
)。 - 默认启用
controls
与playsInline
,并使用preload="metadata"
以平衡首屏与带宽。 - 提供
className
以便样式对齐图片预览(例如居中、缩放适配)。
📝 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.
{type === 'image' && | |
(imageRender | |
? imageRender(imgNode, { | |
transform, | |
image, | |
...(groupContext ? { current } : {}), | |
}) | |
: imgNode)} | |
{type === 'video' && <video src={src} width={props.width} height={props.height} />} | |
{type === 'image' && | |
(imageRender | |
? imageRender(imgNode, { | |
transform, | |
image, | |
...(groupContext ? { current } : {}), | |
}) | |
: imgNode)} | |
{type === 'video' && ( | |
<video | |
className={`${prefixCls}-video`} | |
{...props.videoCommonProps} | |
aria-label={alt} | |
src={props.videoCommonProps?.src ?? src} | |
width={props.width} | |
height={props.height} | |
playsInline={props.videoCommonProps?.playsInline ?? true} | |
controls={props.videoCommonProps?.controls ?? true} | |
preload={props.videoCommonProps?.preload ?? 'metadata'} | |
style={{ | |
maxWidth: '100%', | |
maxHeight: '100%', | |
objectFit: 'contain', | |
...(props.videoCommonProps?.style || {}), | |
}} | |
/> | |
)} |
🤖 Prompt for AI Agents
In src/Preview/index.tsx around lines 431 to 439, the <video> element is missing
controls, playsInline, preload and is not receiving videoCommonProps; update the
JSX to merge videoCommonProps with explicit defaults: compute finalSrc =
videoCommonProps?.src ?? src, set controls={videoCommonProps?.controls ?? true},
playsInline={videoCommonProps?.playsInline ?? true},
preload={videoCommonProps?.preload ?? 'metadata'}, include className (e.g.
videoCommonProps?.className ?? 'preview-media') and pass width/height from
props, and spread the rest of videoCommonProps so event handlers and attributes
are forwarded, allowing explicit videoCommonProps to override defaults where
provided.
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 (1)
src/Image.tsx (1)
178-185
: 避免将imgCommonProps
误声明为含有type
的类型,导致 TS 报错/歧义这里
registerData
需要{ type }
没问题;但上游imgCommonProps
的构造使用了const obj: ImageElementProps = {}
(包含必填type
),这会与新增的type
约束冲突,且imgCommonProps
仅用于<img>
,不应含type
。建议:
- 将
imgCommonProps
的对象类型改为React.ImgHTMLAttributes<HTMLImageElement>
或Omit<ImageElementProps, 'type'>
;registerData
再在其基础上补上type: previewType
。变更位于“imgCommonProps”构造处(文件行 163-175,非本次标记范围之外的辅助修改):
- const imgCommonProps = useMemo( - () => { - const obj: ImageElementProps = {}; + const imgCommonProps = useMemo( + () => { + const obj: React.ImgHTMLAttributes<HTMLImageElement> = {}; COMMON_PROPS.forEach((prop: any) => { if (props[prop] !== undefined) { obj[prop] = props[prop]; } }); return obj; }, COMMON_PROPS.map(prop => props[prop]), );
registerData
维持现状即可(已补上type: previewType
)。
♻️ Duplicate comments (1)
src/Preview/index.tsx (1)
125-128
:videoCommonProps
放错层级,导致对外不可用(与过往评论一致)目前
videoCommonProps
仅定义在PreviewProps
,外部通过Image
的preview
无法合法传入(类型不匹配),功能形同虚设。应与imgCommonProps
一样接入可传递的内部配置层。最小修复(把
videoCommonProps
上提到InternalPreviewConfig
,并从PreviewProps
移除重复定义):export interface InternalPreviewConfig { // Semantic /** Better to use `classNames.root` instead */ rootClassName?: string; // Media type?: 'image' | 'video'; src?: string; alt?: string; + // Video only + videoCommonProps?: React.VideoHTMLAttributes<HTMLVideoElement>; ... } export interface PreviewProps extends InternalPreviewConfig { // Misc prefixCls: string; ... - // Preview media - imgCommonProps?: React.ImgHTMLAttributes<HTMLImageElement>; - videoCommonProps?: React.VideoHTMLAttributes<HTMLVideoElement>; + // Preview media + imgCommonProps?: React.ImgHTMLAttributes<HTMLImageElement>; ... }同时在
src/Image.tsx
的PreviewConfig
(扩展自InternalPreviewConfig
)即可自然透传,无需额外改动。
🧹 Nitpick comments (4)
assets/preview.less (1)
28-32
: 为视频增加“等比缩放”以避免拉伸变形当前仅限制了 max-width/max-height。视频预览在不同宽高比下可能被拉伸,建议为
&-video
增加object-fit: contain
(不影响图片分支)。&-img, &-video { max-width: 100%; max-height: 70%; } + + // 确保视频在容器内等比缩放不变形 + &-video { + object-fit: contain; + }docs/demo/previewvideo.md (1)
1-8
: 补充示例说明与标题优化(微调)建议补一句话说明“视频预览不带工具栏,仅保留分页与关闭按钮”,并将标题改为更可读的 “Preview Video”。
---- -title: previewvideo +title: Preview Video nav: title: Demo path: /demo ---- + +本示例展示 `Image` 组件的「视频预览」能力:仅显示分页与关闭按钮,不含旋转/缩放工具栏。docs/examples/previewvideo.tsx (2)
10-17
: 示例中建议显式传入videoCommonProps
,展示可定制性演示默认行为之上,建议补充
controls/playsInline/preload/poster
等配置,帮助用户理解可配置项。<Image src="https://zos.alipayobjects.com/rmsportal/jkjgkEfvpUPVyRjUImniVslZfWPnJuuZ.png?x-oss-process=image/auto-orient,1/resize,p_10/quality,q_10" preview={{ icons: defaultIcons, type: 'video', - src: 'https://gw.alipayobjects.com/os/rmsportal/NTMlQdLIkPjOACXsdRrq.mp4', + src: 'https://gw.alipayobjects.com/os/rmsportal/NTMlQdLIkPjOACXsdRrq.mp4', + videoCommonProps: { + controls: true, + playsInline: true, + preload: 'metadata', + poster: 'https://dummyimage.com/800x450/000/fff.jpg&text=Video', + }, }} width={200} />
31-38
: 组内示例同样补充videoCommonProps
(可选)与单体示例保持一致,突出可配置点。
<Image key={2} src="https://zos.alipayobjects.com/rmsportal/jkjgkEfvpUPVyRjUImniVslZfWPnJuuZ.png" preview={{ type: 'video', - src: 'https://gw.alipayobjects.com/os/rmsportal/NTMlQdLIkPjOACXsdRrq.mp4', + src: 'https://gw.alipayobjects.com/os/rmsportal/NTMlQdLIkPjOACXsdRrq.mp4', + videoCommonProps: { controls: true, playsInline: true, preload: 'metadata' }, }} width={200} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
assets/preview.less
(1 hunks)docs/demo/previewvideo.md
(1 hunks)docs/examples/previewvideo.tsx
(1 hunks)src/Image.tsx
(3 hunks)src/Preview/index.tsx
(5 hunks)src/PreviewGroup.tsx
(2 hunks)src/interface.ts
(1 hunks)
⏰ 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: WIP
🔇 Additional comments (6)
src/PreviewGroup.tsx (1)
70-71
: 正确透传媒体类型到 Preview:点赞从
mergedItems[current]?.data
解构type
并传入<Preview type={type} />
的接线合理,便于组内图片/视频混排。若
items
中仍存在历史对象未带type
(或为string
),请确认usePreviewItems
生成的数据在缺省时能让type
为undefined
并在Preview
内回落到'image'
(当前实现看起来是可以的)。Also applies to: 139-140
src/Image.tsx (2)
120-127
: 默认'image'
的回落策略合适
preview.type
缺省时回落'image'
,与向后兼容目标一致。
271-272
: 向 Preview 传入type
:OK独立预览模式下已正确透传
type={previewType}
。src/Preview/index.tsx (3)
75-77
: 在 InternalPreviewConfig 中新增type
:方向正确
type
放在内部配置里,保证Image
的preview
能够类型安全地传入该字段。
170-170
: 默认type = 'image'
:符合回退预期与未升级调用方兼容。
471-503
: 仅为图片渲染 Footer 工具栏:设计合理视频不支持旋转/缩放的决策清晰,避免误导。
{type === 'image' && | ||
(imageRender | ||
? imageRender(imgNode, { | ||
transform, | ||
image, | ||
...(groupContext ? { current } : {}), | ||
}) | ||
: imgNode)} | ||
{type === 'video' && ( | ||
<video | ||
className={`${prefixCls}-video`} | ||
src={src} | ||
width={props.width} | ||
height={props.height} | ||
controls | ||
autoPlay | ||
/> | ||
)} |
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
视频渲染未透传 videoCommonProps
且默认 autoPlay
有 UX/兼容隐患
- 未使用已暴露的
videoCommonProps
,用户无法配置controls/poster/preload/事件
等; - 默认
autoPlay
在多数浏览器会被静音门槛或策略阻断,建议默认不自动播放(或仅在muted
时启用); - 可读性与无障碍:建议使用
aria-label={alt}
; - 建议允许
videoCommonProps.src
覆盖顶层src
。
- {type === 'video' && (
- <video
- className={`${prefixCls}-video`}
- src={src}
- width={props.width}
- height={props.height}
- controls
- autoPlay
- />
- )}
+ {type === 'video' && (
+ <video
+ className={`${prefixCls}-video`}
+ aria-label={alt}
+ // 允许调用方在 videoCommonProps 中覆盖 src
+ src={props.videoCommonProps?.src ?? src}
+ width={props.width}
+ height={props.height}
+ // 默认开启基础控件与内联播放,可被调用方覆盖
+ controls={props.videoCommonProps?.controls ?? true}
+ playsInline={props.videoCommonProps?.playsInline ?? true}
+ preload={props.videoCommonProps?.preload ?? 'metadata'}
+ // 默认不自动播放,交由调用方显式开启
+ autoPlay={props.videoCommonProps?.autoPlay}
+ {...props.videoCommonProps}
+ />
+ )}
📝 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.
{type === 'image' && | |
(imageRender | |
? imageRender(imgNode, { | |
transform, | |
image, | |
...(groupContext ? { current } : {}), | |
}) | |
: imgNode)} | |
{type === 'video' && ( | |
<video | |
className={`${prefixCls}-video`} | |
src={src} | |
width={props.width} | |
height={props.height} | |
controls | |
autoPlay | |
/> | |
)} | |
{type === 'image' && | |
(imageRender | |
? imageRender(imgNode, { | |
transform, | |
image, | |
...(groupContext ? { current } : {}), | |
}) | |
: imgNode)} | |
{type === 'video' && ( | |
<video | |
className={`${prefixCls}-video`} | |
aria-label={alt} | |
// 允许调用方在 videoCommonProps 中覆盖 src | |
src={props.videoCommonProps?.src ?? src} | |
width={props.width} | |
height={props.height} | |
// 默认开启基础控件与内联播放,可被调用方覆盖 | |
controls={props.videoCommonProps?.controls ?? true} | |
playsInline={props.videoCommonProps?.playsInline ?? true} | |
preload={props.videoCommonProps?.preload ?? 'metadata'} | |
// 默认不自动播放,交由调用方显式开启 | |
autoPlay={props.videoCommonProps?.autoPlay} | |
{...props.videoCommonProps} | |
/> | |
)} |
🤖 Prompt for AI Agents
In src/Preview/index.tsx around lines 431 to 448, the <video> element does not
forward the exposed videoCommonProps, forces autoPlay by default, lacks
aria-label for accessibility, and doesn't let videoCommonProps.src override the
top-level src; fix by spreading videoCommonProps onto the video element (after
explicitly setting className, width, height so caller props can override), use
videoCommonProps.src ?? src for the source, remove unconditional autoPlay and
only set autoPlay when videoCommonProps.autoPlay is true or when muted is true
(e.g., autoPlay: videoCommonProps.autoPlay || videoCommonProps.muted), ensure
controls/poster/preload and event handlers from videoCommonProps are supported,
and add aria-label={alt} for accessibility.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Preview/index.tsx (1)
260-283
: alt 丢失风险:后置的alt={alt}
会覆盖imgCommonProps.alt
当
alt
未在 props 明确传入、而仅存在于imgCommonProps.alt
时,当前写法会把 alt 覆盖为undefined
,影响无障碍与可测试性。建议合并 alt 值后再传入。- const imgNode = ( + const mergedAlt = alt ?? imgCommonProps?.alt; + const imgNode = ( <PreviewImage {...imgCommonProps} width={props.width} height={props.height} imgRef={imgRef} className={`${prefixCls}-img`} - alt={alt} + alt={mergedAlt} style={{ transform: `translate3d(${transform.x}px, ${transform.y}px, 0) scale3d(${transform.flipX ? '-' : ''}${scale}, ${transform.flipY ? '-' : ''}${scale}, 1) rotate(${rotate}deg)`, transitionDuration: (!enableTransition || isTouching) && '0s', }} fallback={fallback} src={src}src/PreviewGroup.tsx (1)
132-148
: 向 Preview 透传 alt 与 videoCommonProps补齐属性传递链,确保无障碍与视频行为可配置。
<Preview aria-hidden={!isShowPreview} open={isShowPreview} prefixCls={previewPrefixCls} onClose={onPreviewClose} mousePosition={mousePosition} imgCommonProps={imgCommonProps} src={src} + alt={alt} type={type} + videoCommonProps={videoCommonProps} fallback={fallback} icons={icons} current={current} count={mergedItems.length} onChange={onInternalChange}
♻️ Duplicate comments (2)
src/Preview/index.tsx (2)
75-76
: 缺少 videoCommonProps:应加入 InternalPreviewConfig 以完成对外接线当前仅新增了
type?: 'image' | 'video'
,但未提供videoCommonProps
,导致通过preview={{ type:'video', videoCommonProps: {...} }}
无法生效。建议在InternalPreviewConfig
中补充该属性(与过往评论一致)。export interface InternalPreviewConfig { // Semantic /** Better to use `classNames.root` instead */ rootClassName?: string; // Media type?: 'image' | 'video'; + /** 供 <video> 透传的原生属性与事件 */ + videoCommonProps?: React.VideoHTMLAttributes<HTMLVideoElement>; src?: string; alt?: string;
438-447
: 视频渲染未透传属性,且默认 autoPlay 可能被策略拦截
- 未透传
videoCommonProps
,用户无法自定义controls/poster/preload/事件
等;- 建议默认开启
controls
、playsInline
,preload='metadata'
;移除强制autoPlay
(由调用方决定),并提供aria-label
以提升无障碍;- 允许
videoCommonProps.src
覆盖顶层src
。- {type === 'video' && ( - <video - className={`${prefixCls}-video`} - src={src} - width={props.width} - height={props.height} - controls - autoPlay - /> - )} + {type === 'video' && ( + <video + className={`${prefixCls}-video`} + aria-label={alt ?? imgCommonProps?.alt} + src={props.videoCommonProps?.src ?? src} + width={props.width} + height={props.height} + controls={props.videoCommonProps?.controls ?? true} + playsInline={props.videoCommonProps?.playsInline ?? true} + preload={props.videoCommonProps?.preload ?? 'metadata'} + style={{ + maxWidth: '100%', + maxHeight: '100%', + objectFit: 'contain', + ...(props.videoCommonProps?.style || {}), + }} + {...props.videoCommonProps} + /> + )}同时在组件入参解构处补充
videoCommonProps
(紧邻imgCommonProps
):imageRender, - imgCommonProps, + imgCommonProps, + videoCommonProps, actionsRender,
🧹 Nitpick comments (2)
src/Preview/index.tsx (1)
125-129
: imgCommonProps 对外暴露 OK,建议与视频对称
imgCommonProps
的暴露方式合理;为保持对称与一致性,视频侧请补充videoCommonProps
(见上条)。src/PreviewGroup.tsx (1)
12-21
: 补充 videoCommonProps 并基于 type 使用可辨识联合重构预览配置
- src/interface.ts:16 – 在
ImageElementProps
中添加videoCommonProps
- src/PreviewGroup.tsx (12–21) – 将
GroupPreviewConfig
及相关 PreviewConfig 改为以type
字段判别的联合类型,避免同时暴露 img/video Props 导致歧义
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
assets/preview.less
(1 hunks)docs/demo/previewvideo.md
(1 hunks)docs/examples/previewvideo.tsx
(1 hunks)src/Image.tsx
(3 hunks)src/Preview/index.tsx
(5 hunks)src/PreviewGroup.tsx
(2 hunks)src/interface.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/demo/previewvideo.md
🚧 Files skipped from review as they are similar to previous changes (4)
- assets/preview.less
- src/interface.ts
- src/Image.tsx
- docs/examples/previewvideo.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/Preview/index.tsx (2)
docs/examples/imageRender.tsx (1)
imageRender
(6-27)src/Preview/Footer.tsx (1)
Footer
(46-212)
🔇 Additional comments (2)
src/Preview/index.tsx (2)
169-169
: 为 type 提供默认值 'image' 👍默认分支清晰,可避免未设置时的行为歧义。
470-501
: 仅图片显示 Footer 的策略与需求一致视频预览不显示工具栏,符合 PR 说明(仅保留翻页与关闭)。
@@ -67,7 +67,7 @@ const Group: React.FC<PreviewGroupProps> = ({ | |||
const [keepOpenIndex, setKeepOpenIndex] = useState(false); | |||
|
|||
// >>> Image | |||
const { src, ...imgCommonProps } = mergedItems[current]?.data || {}; | |||
const { src, type, ...imgCommonProps } = mergedItems[current]?.data || {}; |
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
从数据中额外取出 alt 与 videoCommonProps,避免属性丢失并完成视频接线
当前将除 src/type
外的字段全部并入 imgCommonProps
,会让 alt
被当作 img 属性下发且在预览端被覆盖;同时未传递 videoCommonProps
。建议如下解构:
- const { src, type, ...imgCommonProps } = mergedItems[current]?.data || {};
+ const { src, type, alt, videoCommonProps, ...imgCommonProps } =
+ mergedItems[current]?.data || {};
📝 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 { src, type, ...imgCommonProps } = mergedItems[current]?.data || {}; | |
const { src, type, alt, videoCommonProps, ...imgCommonProps } = | |
mergedItems[current]?.data || {}; |
🤖 Prompt for AI Agents
In src/PreviewGroup.tsx around line 70, the current destructure pulls all
non-src/type fields into imgCommonProps causing alt to be treated as an img prop
and dropped/overwritten and videoCommonProps to be omitted; change the
destructure to pull out alt and videoCommonProps explicitly (e.g. const { src,
type, alt, videoCommonProps, ...imgCommonProps } = mergedItems[current]?.data ||
{}), then use alt for the img element and pass videoCommonProps into the video
element when rendering so both alt and video-specific props are preserved.
https://ant-design.antgroup.com/components/image-cn#toolbarrenderinfotype 可以自行定义,Image 组件还是不太适合提供 video type,很多属性语义就不对了。考虑一下 ConfigProvider 支持全局配置? |
近些年的 Web 应用中,图片和视频混排的情况非常多,因此希望在 Image 组件中支持视频预览,效果类似于 Twitter/X 的多图帖子。视频预览没有工具栏,仅有翻页和关闭按钮。
Summary by CodeRabbit
Summary by CodeRabbit