将蒙层点击与图片点击事件分离 #6937
将蒙层点击与图片点击事件分离 #6937
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
|
|
dosubot
bot
added
the
size:XS
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6937 +/- ##
=======================================
Coverage 92.91% 92.92%
=======================================
Files 336 336
Lines 7342 7346 +4
Branches 1829 1859 +30
=======================================
+ Hits 6822 6826 +4
+ Misses 512 484 -28
- Partials 8 36 +28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthrough将 ImageViewer 的顶级 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Methods as showImageViewer / showMultiImageViewer
participant IV as ImageViewer / MultiImageViewer
participant Mask as Mask
User->>Methods: 调用 showImageViewer(props...)
Methods->>IV: renderImperatively(mask={{ ...props.mask, afterClose: wrapper }})
IV->>Mask: 渲染 Mask（afterClose=mask?.afterClose, onMaskClick=mask?.onClick, className=classNames?.mask）
Mask-->>IV: 触发 onClick / afterClose
IV-->>Methods: 调用包装后的 afterClose（清理 handler 并透传原始 mask.afterClose）
IV-->>User: 执行用户提供的 mask 回调
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
dosubot
bot
added
size:S
dosubot
bot
added
the
size:M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/image-viewer/index.en.md (3)
18-18: 措辞微调，提升可读性；同时补充版本信息占位
- 英文表述更自然的写法建议为 “Triggered after the viewer is fully displayed”。
- 建议在 Version 列补充该属性引入的版本号（与中文文档保持一致），否则和表头的 5 列结构不完全对齐的感觉会略显突兀。
可参考以下 diff（不包含版本号，需按真实发布版本补全）：
-| afterShow | Triggered after fully displayed | `() => void` | - | +| afterShow | Triggered after the viewer is fully displayed | `() => void` | - |
24-24: 澄清 onMaskClick 触发语义并顺带优化英文措辞
- 静态检查提示该行可能有问题；主要是描述略显含糊。建议明确“只在点击蒙层（不包含图片区域）时触发”，以体现与图片点击事件分离的 PR 目的。
- 若默认点击蒙层会关闭弹窗（或不会），也建议在文档中点明默认行为。
建议改为：
-| onMaskClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | +| onMaskClick | Triggered when clicking the mask (outside the image content) | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - |补充确认：
- onMaskClick 是否会默认调用 onClose/关闭？若无默认关闭行为，建议在说明中附一句 “This callback does not close the viewer by default.”
- 英文/中文文档两处描述需保持一致。
50-57: “Mask”小节适用范围不够清晰；用语与上表不完全一致
- 该小节位于 Multi 的 “Ref” 之后，读者可能误以为仅适用于 ImageViewer.Multi。建议显式说明该小节同样适用于单图与多图两种模式（如果事实如此）。
- 三个描述与上文主表的用语略有差异，建议统一（“completely closed”“fully displayed” 两处可与上文一致）。
- 可在该小节增加一行注释，明确 onMaskClick 不会响应图片元素的点击，避免误解。
可参考以下 diff（仅优化措辞并增加说明，不改变表结构；请按实际情况确认）：
### Mask + +The following mask-related props apply to both ImageViewer and ImageViewer.Multi. +Note: onMaskClick only fires when clicking on the mask area; image clicks are handled separately. -| Name | Description | Type | Default | -| --- | --- | --- | --- | -| afterClose | Triggered when completely closed | `() => void` | - | -| afterShow | Triggered after fully displayed | `() => void` | - | -| onMaskClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | +| Name | Description | Type | Default | +| --- | --- | --- | --- | +| afterClose | Triggered when it is completely closed | `() => void` | - | +| afterShow | Triggered after the viewer is fully displayed | `() => void` | - | +| onMaskClick | Triggered when clicking the mask (outside the image content) | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - |
📜 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 (3)
src/components/image-viewer/image-viewer.tsx(3 hunks)src/components/image-viewer/index.en.md(2 hunks)src/components/image-viewer/index.zh.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/image-viewer/image-viewer.tsx
- src/components/image-viewer/index.zh.md
🧰 Additional context used
🪛 LanguageTool
src/components/image-viewer/index.en.md
[grammar] ~24-~24: There might be a mistake here.
Context: ...LDivElement, MouseEvent>) => void` | - | | renderFooter | Render extra content on...
(QB_NEW_EN)
⏰ 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). (4)
- GitHub Check: build preview
- GitHub Check: size
- GitHub Check: compressed-size
- GitHub Check: check
🔇 Additional comments (1)
src/components/image-viewer/index.en.md (1)
15-26: 为新增属性补齐版本信息，确保中英文文档与代码一致
- 主表中的 afterShow、onMaskClick 当前 Version 列为空；Mask 小表没有 Version 列。建议：
- 若本项目约定所有新增对外 API 都记录引入版本，则在主表补齐版本；如需要，也可在 Mask 小表增加 Version 列并补齐。
- 确认中文文档（index.zh.md）相同位置的版本号与英文一致。
- 同步核对代码实现：单图与多图模式、以及 ImageViewer.show / ImageViewer.Multi.show 的命令式 API 是否都透传 afterShow 与 onMaskClick。
如果决定在 Mask 小表也展示版本号，可按最终版本补齐（此处不提供具体 diff 以避免误导版本号）。
Also applies to: 50-57
| | --- | --- | --- | --- | | ||
| | afterClose | 完全关闭后触发 | `() => void` | - | | ||
| | afterShow | 完全展示后触发 | `() => void` | - | | ||
| | onMaskClick | 点击蒙层自身触发 | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | |
There was a problem hiding this comment.
直接 onClick 就行了，mask.onMaskClick 会有点多余。
coderabbitai
bot
added
the
size:L
dosubot
bot
added
size:M
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/image-viewer/index.zh.md (1)
39-46: 命名与语义需统一；补充“蒙层点击不触发图片点击”的说明
- 与英文文档不一致：英文在顶层 props 使用
onMaskClick，而此处在 Mask 小节使用onClick。请确认代码中实际 API，并在中英文文档统一命名（建议沿用先前讨论的 Mask.onClick，避免混用）。- 建议在本小节补一句“蒙层点击只触发蒙层回调，不会触发图片点击事件”，以明确“蒙层点击与图片点击事件分离”的改动意图。
- 若单图 ImageViewer 同样支持 Mask 回调，请在单图部分也新增同名 Mask 小节，保持单/多图一致。
可参考如下最小补充（仅示意文案）：
### Mask @@ | onClick | 点击蒙层自身触发 | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | +说明：点击蒙层仅会触发本小节中的回调（如 onClick），不会触发图片区域的点击事件。
🧹 Nitpick comments (3)
src/components/image-viewer/index.en.md (3)
18-18: 修复表格列数错误（缺少 Version 单元格）该行与表头列数不一致（5 列），markdownlint 已报错。请补齐 Version 列。
-| afterShow | Triggered after fully displayed | `() => void` | - | +| afterShow | Triggered after fully displayed | `() => void` | - | |
24-24: 修复表格列数错误；同时确认事件命名是否应为 onClick
- 同样缺少 Version 列单元格。
- 英文顶层使用
onMaskClick，而 Mask 小节使用onClick，与中文文档不一致。请确认最终命名并在两种语言中保持一致（建议统一为 Mask.onClick，或统一为 onMaskClick，两者择一）。-| onMaskClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | +| onMaskClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | |如若决定统一为 Mask.onClick，请删除本顶层行，并在 ImageViewer（单图）部分新增 Mask 小节对齐 Multi 的写法。
50-57: 命名不一致：此处为onClick，而顶层为onMaskClick为了降低用户理解成本，请两处保持一致命名（并与中文文档一致）。建议选择其一：
- 方案 A：统一使用 Mask 小节的
onClick（符合先前评审建议“直接 onClick 即可”）；- 方案 B：统一为
onMaskClick，则本表也应改为onMaskClick。示例（方案 A，若采用 Mask.onClick 策略）：
-| onClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | +| onClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - |示例（方案 B，若坚持顶层 onMaskClick 策略）：
-| onClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | +| onMaskClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - |
📜 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 (3)
src/components/image-viewer/image-viewer.tsx(3 hunks)src/components/image-viewer/index.en.md(2 hunks)src/components/image-viewer/index.zh.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/image-viewer/image-viewer.tsx
🧰 Additional context used
🪛 LanguageTool
src/components/image-viewer/index.en.md
[grammar] ~24-~24: There might be a mistake here.
Context: ...LDivElement, MouseEvent>) => void` | - | | renderFooter | Render extra content on...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
src/components/image-viewer/index.en.md
18-18: Table column count
Expected: 5; Actual: 4; Too few cells, row will be missing data
(MD056, table-column-count)
24-24: Table column count
Expected: 5; Actual: 4; Too few cells, row will be missing data
(MD056, table-column-count)
⏰ 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). (4)
- GitHub Check: build preview
- GitHub Check: compressed-size
- GitHub Check: size
- GitHub Check: check
| | classNames | Semantic structure class | `{ mask?:string,body?:string }` | - | 5.33.1 | | ||
| | getContainer | To get the specified mounted HTML node, the default is `null` rendered to the current node | `HTMLElement \| () => HTMLElement \| null` | `null` | | | ||
| | image | The `url` of the image resource | `string` | - | | | ||
| | maxZoom | The maximum zoom ratio | `number \| 'auto'` | `3` | | | ||
| | onClose | Triggered when it is closed | `() => void` | - | | | ||
| | onMaskClick | Triggered when the mask is clicked | `(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void` | - | |
There was a problem hiding this comment.
中英文文档不一致，另外上面的代码也没有变。mask 抽出来是这么消费的:
<ImageViewer
mask={{ onClick: handleMaskClick }}
/>There was a problem hiding this comment.
我单独提出来一个 mask 属性写法改成： props.mask.onClick
使用：
<ImageViewer
mask={{ onClick: handleMaskClick }}
/>
There was a problem hiding this comment.
然后我在文档说明的地方增加了
| mask | 遮罩层的属性 | `{afterClose?:()=>void,onClick?:(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void}` | - | |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/components/image-viewer/index.zh.md (1)
40-46: 命名与上轮评审建议一致（mask.onClick 而非 mask.onMaskClick）与此前评审建议保持一致性，已赞同该命名。
src/components/image-viewer/index.en.md (1)
25-25: 中英文文档已对齐（mask.onClick 命名与结构一致）与前次评审关于中英文一致性的反馈一致，变更已对齐。
Also applies to: 48-54
🧹 Nitpick comments (7)
src/components/image-viewer/index.zh.md (2)
25-25: mask 属性文档已补充，建议补上版本号并统一类型书写格式
- 建议在“版本”列补上引入版本（或标注为“即将发布”），便于用户识别破坏性变更。
- 类型字符串建议加空格与分号，保持与其他文档风格一致。
如需仅做排版微调，可直接应用此行内改动：
-| mask | 遮罩层的属性 | `{afterClose?:()=>void,onClick?:(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void}` | - | | +| mask | 遮罩层的属性 | `{ afterClose?: () => void; onClick?: (event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void }` | - | |
40-46: 补充用法说明：mask.onClick 不会自动关闭，请在回调中自行关闭为避免误解，建议在表格下追加一句说明，并给出最小示例，指明需要在 onClick 里手动触发关闭逻辑（如设置 visible=false 或调用 onClose）：
示例（建议追加到本小节下方）：
// 点击蒙层关闭弹窗 <ImageViewer visible={visible} onClose={() => setVisible(false)} mask={{ onClick: () => setVisible(false) }} />src/components/image-viewer/image-viewer.tsx (3)
37-40: 轻微类型收敛建议：去掉第二泛型以减少对 DOM lib 的显式依赖通常写法用
React.MouseEvent<HTMLDivElement>即可，避免显式引用原生MouseEvent。mask?: { afterClose?: () => void - onClick?: (event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void + onClick?: (event: React.MouseEvent<HTMLDivElement>) => void }
55-57: 可选：蒙层点击默认同时触发关闭，且保留用户回调当前仅透传
mask.onClick，若产品期望“默认点击蒙层即可关闭”，可在不提供mask.onClick时或无条件地在回调后追加触发onClose。同时把props?.classNames?.mask简化为props.classNames?.mask。请确认是否需要“默认点击蒙层关闭”的产品行为；若需要，可应用：
<Mask visible={props.visible} afterClose={props.mask?.afterClose} - onMaskClick={props.mask?.onClick} - className={props?.classNames?.mask} + onMaskClick={(e) => { + props.mask?.onClick?.(e) + props.onClose?.() + }} + className={props.classNames?.mask} disableBodyScroll={false} opacity='thick' destroyOnClose >
136-139: Multi 分支与单图分支保持一致的蒙层行为同上，若决定采用“点击蒙层默认关闭”，建议同步调整 Multi 分支；同时移除不必要的可选链。
<Mask visible={props.visible} afterClose={props.mask?.afterClose} - className={props?.classNames?.mask} - onMaskClick={props.mask?.onClick} + className={props.classNames?.mask} + onMaskClick={(e) => { + props.mask?.onClick?.(e) + props.onClose?.() + }} disableBodyScroll={false} opacity='thick' destroyOnClose >src/components/image-viewer/index.en.md (2)
25-25: 术语与类型格式小修：更自然的表述与统一写法将“Attributes of Mask Layer”简化为“Mask props”，并统一类型空格/分号；同时可去掉
MouseEvent第二泛型。-| mask | Attributes of Mask Layer | `{afterClose?:()=>void,onClick?:(event: React.MouseEvent<HTMLDivElement, MouseEvent>) => void}` | - | | +| mask | Mask props | `{ afterClose?: () => void; onClick?: (event: React.MouseEvent<HTMLDivElement>) => void }` | - | |
48-54: 补充说明与示例：mask.onClick 仅回调不自动关闭建议在本表下增加一句说明和最小示例，便于英文用户快速上手。
示例（建议追加到本小节下方）：
<ImageViewer visible={visible} onClose={() => setVisible(false)} mask={{ onClick: () => setVisible(false) }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/image-viewer/image-viewer.tsx(3 hunks)src/components/image-viewer/index.en.md(2 hunks)src/components/image-viewer/index.zh.md(2 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). (2)
- GitHub Check: build preview
- GitHub Check: check
🔇 Additional comments (1)
src/components/image-viewer/image-viewer.tsx (1)
52-84: 确认 onMaskClick 仅在蒙层空白区域触发 Mask 组件根节点 onClick 已通过e.target === e.currentTarget判断，避免内容区点击冒泡触发，且 aria-button 单独绑定关闭事件；无需调整。
|
afterClose 去掉后，代码里的部分也要调整回来。CI 都挂了 |
coderabbitai
bot
added
the
size:L
dosubot
bot
added
size:M
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/image-viewer/methods.tsx (2)
41-46: 与上方相同的清理逻辑建议 + 去重以避免重复代码
- 同步采纳 try/catch 包装用户回调的建议，保持单一行为。
- 两处
mask={{ ...props.mask, afterClose: ... }}逻辑重复，建议抽成小工具函数，便于后续演进时保持一致。可抽取（示例）：
function withCleanupMask( mask: ImageViewerProps['mask'], handler: ImageViewerShowHandler ): NonNullable<ImageViewerProps['mask']> { return { ...mask, afterClose: () => { handlerSet.delete(handler) try { mask?.afterClose?.() } catch (e) { console.error(e) } }, } } // 使用：mask={withCleanupMask(props.mask, handler)}请确认 Multi 的所有关闭交互（包括左右滑动切换、缩放恢复、手势退出等）同样最终触发到 Mask 的
afterClose，以确保 handler 不泄漏。
21-26: 可选：为 mask.afterClose 的用户回调加防御性 try/catch，并可选包装 handler.close 作为兜底
- 验证要点：src/components/image-viewer/methods.tsx 中已先执行 handlerSet.delete(handler) 再调用 props.mask?.afterClose?.()，因此用户回调抛错不会导致 handler 未删除。
- 额外保障：src/utils/render-imperatively.tsx 在正常关闭路径和 unmount 回退路径都会调用 element.props.afterClose，且 clearImageViewer 在触发 close 后会立即 handlerSet.clear()，未发现会绕过 Mask.afterClose 导致残留 handler 的路径。
- 结论与建议：关于“遗留 handler”的主要风险已由现有实现覆盖；但为避免用户回调异常冒泡影响动画/组件，建议可选性地将用户回调包在 try/catch（下方 diff），以及可选地在添加 handler 前包装 handler.close 作为额外兜底。
可选最小改动（示例 diff）：
mask={{ ...props.mask, - afterClose: () => { - handlerSet.delete(handler) - props.mask?.afterClose?.() - }, + afterClose: () => { + handlerSet.delete(handler) + try { + props.mask?.afterClose?.() + } catch (e) { + // 避免用户回调异常影响清理流程 + console.error(e) + } + }, }}可选备份包装（示例）：
// 在 handlerSet.add(handler) 之前 const rawClose = handler.close handler.close = () => { handlerSet.delete(handler) // 先行清理，重复 delete 无副作用 rawClose() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/image-viewer/methods.tsx(2 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). (4)
- GitHub Check: build preview
- GitHub Check: compressed-size
- GitHub Check: check
- GitHub Check: size
支持将蒙层点击与图片点击事件分离，允许通过蒙层关闭弹窗
Summary by CodeRabbit