fix: error 6945, antd-mobile slide calculation error #6946
Conversation
dosubot
bot
added
the
size:S
📝 WalkthroughWalkthrough为 Slides 组件新增必需的 viewportWidth 属性，由父级 MultiImageViewer 在渲染时计算容器宽度（容错到 window.innerWidth 或 375）并传递下去；Slides 内部以该宽度作为基准计算单页宽度，其余索引与动画逻辑不变。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant MIV as MultiImageViewer
participant C as Container (props.getContainer)
participant Slides as Slides
App->>MIV: 渲染 MultiImageViewer(props)
rect rgba(200,220,255,0.25)
note right of MIV: 计算 viewportWidth
MIV->>C: getContainer()?.getBoundingClientRect()
C-->>MIV: { width } 或失败
MIV->>MIV: 选择 width 或 window.innerWidth 或 375
end
MIV->>Slides: 渲染 Slides({ viewportWidth, ... })
rect rgba(220,255,220,0.25)
note right of Slides: 基于 viewportWidth 计算 slideWidth<br/>保持原有索引/动画逻辑
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
✨ 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 |
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @yuetanS2024, 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!
This pull request resolves a bug in the antd-mobile image viewer component where slide animations were misaligned due to an incorrect width calculation. The fix introduces a more robust method for determining the viewport width, allowing the component to correctly size itself based on its parent container, thereby improving its adaptability and enabling more flexible integration into various layouts.
Highlights
- Fix for antd-mobile slide calculation error (6945): Corrected an issue where the image viewer's slide width was incorrectly calculated using window.innerWidth, leading to display errors, especially when the component was rendered within a custom container.
- Dynamic viewport width calculation: Implemented a new viewportWidth calculation that first attempts to use the width of a specified container (via getContainer prop) and falls back to window.innerWidth or a default value if no container is provided or its width is invalid.
- Enhanced customization for image viewer: The change allows users to customize the rendering layer of the image viewer by ensuring slide animations correctly adapt to the parent container's dimensions, rather than being fixed to the browser window's width.
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.
Code Review
This pull request aims to fix a slide calculation error by using the container's width instead of the window's width. The overall approach is correct, but the implementation for retrieving the container's width contains a bug. It doesn't correctly handle cases where the getContainer prop is an HTML element rather than a function. I've provided a specific comment with a code suggestion to address this issue. The rest of the changes logically follow from this and appear correct.
| const viewportWidth = (function () { | ||
| try { | ||
| const host = props.getContainer && props.getContainer(); | ||
| if (host && host.getBoundingClientRect) { | ||
| const w = host.getBoundingClientRect().width; | ||
| if (w && w > 0) return w; | ||
| } | ||
| } catch (e) {} | ||
| return typeof window !== 'undefined' ? window.innerWidth : 375; | ||
| })(); |
There was a problem hiding this comment.
This logic for determining viewportWidth has a bug and can be improved for robustness.
- Bug: The expression
props.getContainer && props.getContainer()is incorrect. ThegetContainerprop can be anHTMLElementobject. In that case,props.getContainer()will throw aTypeError. This error is silently caught, causing the logic to incorrectly fall back to usingwindow.innerWidth, which defeats the purpose of this change. - Error Handling: The empty
catch (e) {}block swallows all errors, which can make debugging very difficult. It's better practice to at least log errors during development. - Redundancy: The condition
w && w > 0is redundant.w > 0is sufficient, as0is falsy.
I've provided a suggestion that fixes the bug while keeping the structure of your code.
const viewportWidth = (function () {
try {
const container = props.getContainer;
const host = container && (typeof container === 'function' ? container() : container);
if (host && host.getBoundingClientRect) {
const w = host.getBoundingClientRect().width;
if (w > 0) return w;
}
} catch (e) {}
return typeof window !== 'undefined' ? window.innerWidth : 375;
})();
coderabbitai
bot
added
lgtm
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/image-viewer/slides.tsx (2)
33-36: 容错：当 viewportWidth ≤ 0 时回退默认值，避免 slideWidth 异常尽管上层已尽量返回正值，这里建议再做一次兜底，提升组件健壮性。
- const baseWidth = props.viewportWidth - const slideWidth = baseWidth + convertPx(16) + const baseWidth = props.viewportWidth > 0 ? props.viewportWidth : 375 + const slideWidth = baseWidth + convertPx(16)
33-41: 可选：当容器宽度变化时，同步纠正 spring 偏移，避免指示器与实际页错位若上层按建议引入 ResizeObserver/resize 监听并改变
viewportWidth，当前useSpring初始位移不会随之更新。建议在宽度变更时基于当前索引重设x。补充示例（需引入
useEffect）：const prevSlideWidthRef = useRef(slideWidth); useEffect(() => { if (prevSlideWidthRef.current !== slideWidth) { const currentIndex = bound(Math.round(x.get() / prevSlideWidthRef.current), 0, count - 1); api.start({ x: currentIndex * slideWidth, immediate: true }); prevSlideWidthRef.current = slideWidth; } }, [slideWidth, count, api, x]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/image-viewer/image-viewer.tsx(2 hunks)src/components/image-viewer/slides.tsx(2 hunks)
🔇 Additional comments (1)
src/components/image-viewer/image-viewer.tsx (1)
162-162: 无需修改：所有<Slides>用法已包含viewportWidth
已验证唯一出现的<Slides>用法（位于src/components/image-viewer/image-viewer.tsx）已正确透传该属性，无遗漏。
| const viewportWidth = (function () { | ||
| try { | ||
| const host = props.getContainer && props.getContainer(); | ||
| if (host && host.getBoundingClientRect) { | ||
| const w = host.getBoundingClientRect().width; | ||
| if (w && w > 0) return w; | ||
| } | ||
| } catch (e) {} | ||
| return typeof window !== 'undefined' ? window.innerWidth : 375; | ||
| })(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
修复：GetContainer 形态未全覆盖，HTMLElement 会被误当函数调用，导致宽度始终退回 window
当前写法在 getContainer 为 HTMLElement 时会抛 TypeError（被 try/catch 吃掉），从而永远走到 window.innerWidth 分支，违背“按容器宽度计算”的目标。建议同时支持函数与元素两种形态，并保持 SSR 安全。
可在原位置直接替换为：
- const viewportWidth = (function () {
- try {
- const host = props.getContainer && props.getContainer();
- if (host && host.getBoundingClientRect) {
- const w = host.getBoundingClientRect().width;
- if (w && w > 0) return w;
- }
- } catch (e) {}
- return typeof window !== 'undefined' ? window.innerWidth : 375;
- })();
+ const viewportWidth = (function () {
+ try {
+ const container = props.getContainer;
+ const host = typeof container === 'function' ? container() : container;
+ const w =
+ host && typeof (host as any).getBoundingClientRect === 'function'
+ ? (host as any).getBoundingClientRect().width
+ : 0;
+ if (w > 0) return w;
+ } catch {}
+ return typeof window !== 'undefined' ? window.innerWidth : 375;
+ })();可选增强（推荐后续跟进）：当容器尺寸或方向变化时动态重算宽度，避免横竖屏切换后滑动距离与指示器错位。
附示例（需在文件顶部引入 useLayoutEffect）：
- const viewportWidth = (function () { /* 立即求值 */ })();
+ const [viewportWidth, setViewportWidth] = useState(375);
+ const measure = useCallback(() => {
+ try {
+ const container = props.getContainer;
+ const host = typeof container === 'function' ? container() : container;
+ const w =
+ host && typeof (host as any).getBoundingClientRect === 'function'
+ ? (host as any).getBoundingClientRect().width
+ : 0;
+ return w > 0 ? w : (typeof window !== 'undefined' ? window.innerWidth : 375);
+ } catch { return typeof window !== 'undefined' ? window.innerWidth : 375; }
+ }, [props.getContainer]);
+ useLayoutEffect(() => {
+ const update = () => setViewportWidth(measure());
+ update();
+ if (typeof window !== 'undefined') {
+ window.addEventListener('resize', update);
+ let ro: ResizeObserver | null = null;
+ const container = props.getContainer;
+ const host = typeof container === 'function' ? container() : container;
+ if (host && 'ResizeObserver' in window) {
+ ro = new ResizeObserver(update); ro.observe(host as Element);
+ }
+ return () => { window.removeEventListener('resize', update); ro?.disconnect(); };
+ }
+ }, [measure, props.getContainer]);📝 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 viewportWidth = (function () { | |
| try { | |
| const host = props.getContainer && props.getContainer(); | |
| if (host && host.getBoundingClientRect) { | |
| const w = host.getBoundingClientRect().width; | |
| if (w && w > 0) return w; | |
| } | |
| } catch (e) {} | |
| return typeof window !== 'undefined' ? window.innerWidth : 375; | |
| })(); | |
| const viewportWidth = (function () { | |
| try { | |
| const container = props.getContainer; | |
| const host = typeof container === 'function' ? container() : container; | |
| const w = | |
| host && typeof (host as any).getBoundingClientRect === 'function' | |
| ? (host as any).getBoundingClientRect().width | |
| : 0; | |
| if (w > 0) return w; | |
| } catch {} | |
| return typeof window !== 'undefined' ? window.innerWidth : 375; | |
| })(); |
🤖 Prompt for AI Agents
In src/components/image-viewer/image-viewer.tsx around lines 119-128, the
current IIFE assumes props.getContainer is a function and swallows a TypeError
when getContainer is actually an HTMLElement, causing viewportWidth to fall back
to window.innerWidth; change the logic to first check typeof props.getContainer
=== 'function' and call it, then else-if check if props.getContainer is an
HTMLElement (or has getBoundingClientRect) and use that element directly, obtain
its width via getBoundingClientRect().width when available, keep the SSR
fallback to window.innerWidth or a default (375), and ensure any exceptions are
handled minimally so valid element cases aren’t masked; optionally follow up by
moving this into a layout effect to recalc on resize/orientation changes.
| viewportWidth: number | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
类型一致性：maxZoom 在父组件可为 'auto'，此处仅为 number
MultiImageViewerProps 继承了 ImageViewerProps，其中 maxZoom?: number | 'auto'；当前 SlidesType.maxZoom: number 可能在传入 'auto' 时造成类型/运行期不一致。建议放宽类型或在父层归一化。
可选其一：
- maxZoom: number
+ maxZoom: number | 'auto'或在父组件传参前将 'auto' 归一为具体数值（如根据图片尺寸计算）。
🏁 Script executed:
#!/bin/bash
# 查找 ImageViewerProps 定义及其 maxZoom 类型
rg -nP --type=ts "interface\s+ImageViewerProps" -nC3
# 查找 MultiImageViewerProps 定义及其继承关系
rg -nP --type=ts "interface\s+MultiImageViewerProps" -nC3
# 查找 SlidesType 定义，确认 maxZoom 类型
rg -nP --type=ts "type\s+SlidesType" -nC3Length of output: 1268
🏁 Script executed:
#!/bin/bash
# 查找 slides.tsx 文件路径
fd slides.tsx -t f
# 打印 SlidesType 定义及周围上下文
rg -n 'type SlidesType' -C5 src
# 查找 ImageViewerProps 定义及 maxZoom 类型
rg -n 'interface ImageViewerProps' -C5 src
# 查找 MultiImageViewerProps 定义及继承关系
rg -n 'interface MultiImageViewerProps' -C5 srcLength of output: 774
🏁 Script executed:
#!/bin/bash
# 列出 image-viewer 组件目录下所有文件
fd -t f src/components/image-viewer
# 搜索 MultiImageViewer 组件定义
rg -nP 'export (function|const) MultiImageViewer' -C5 src
# 搜索 MultiImageViewerProps 定义或引用
rg -nP 'MultiImageViewerProps' -C5 src
# 搜索 ImageViewerProps 定义或引用
rg -nP 'ImageViewerProps' -C5 srcLength of output: 12252
同步 SlidesType.maxZoom 与父组件 Prop 类型
maxZoom 在 ImageViewerProps/MultiImageViewerProps 中可为 number | 'auto'，但 SlidesType 定义为 number，若传入 'auto' 会造成类型或运行期错误。建议二选一：
- 将 SlidesType.maxZoom 类型放宽为
number | 'auto' - 或在 MultiImageViewer 渲染 Slides 前，将
'auto'归一为具体数值（如基于图片尺寸或默认值计算）
🤖 Prompt for AI Agents
In src/components/image-viewer/slides.tsx around lines 26-27, SlidesType
currently types maxZoom as number but parent props
(ImageViewerProps/MultiImageViewerProps) allow number | 'auto'; update to keep
types consistent by either widening SlidesType.maxZoom to number | 'auto' OR
normalize the prop before passing to Slides (convert 'auto' to a concrete number
based on image/default sizing). Implement one of these fixes: (A) change the
SlidesType definition to accept number | 'auto' and propagate that type through
usages, or (B) in MultiImageViewer compute a numeric maxZoom when maxZoom ===
'auto' and pass only a number into Slides, ensuring no runtime type mismatch.
|
Pls help to provide reproduce in #6945. We need confirm it first. |
set animation calculation width to container width instead of window width, so that user can customize the layer of the rendering.
Summary by CodeRabbit