Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/components/image-viewer/image-viewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,16 @@ export const MultiImageViewer = forwardRef<
slidesRef.current?.swipeTo(index, immediate)
},
}))

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;
})();
Comment on lines +119 to +128

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This logic for determining viewportWidth has a bug and can be improved for robustness.

  1. Bug: The expression props.getContainer && props.getContainer() is incorrect. The getContainer prop can be an HTMLElement object. In that case, props.getContainer() will throw a TypeError. This error is silently caught, causing the logic to incorrectly fall back to using window.innerWidth, which defeats the purpose of this change.
  2. 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.
  3. Redundancy: The condition w && w > 0 is redundant. w > 0 is sufficient, as 0 is 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;
  })();

Comment on lines +119 to +128
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

修复: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.

Suggested change
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.

const onSlideChange = useCallback(
(newIndex: number) => {
if (newIndex === index) return
Expand Down Expand Up @@ -150,6 +159,7 @@ export const MultiImageViewer = forwardRef<
onTap={props.onClose}
maxZoom={props.maxZoom}
imageRender={props.imageRender}
viewportWidth={viewportWidth}
/>
)}
</div>
Expand Down
7 changes: 6 additions & 1 deletion src/components/image-viewer/slides.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@ export type SlidesType = {
image: string,
{ ref, index }: { ref: RefObject<HTMLImageElement>; index: number }
) => ReactNode
viewportWidth: number
}
Comment on lines +26 to 27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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" -nC3

Length 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 src

Length 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 src

Length 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.

export type SlidesRef = {
swipeTo: (index: number, immediate?: boolean) => void
}

export const Slides = forwardRef<SlidesRef, SlidesType>((props, ref) => {
const slideWidth = window.innerWidth + convertPx(16)
const baseWidth = props.viewportWidth

const slideWidth = baseWidth + convertPx(16)

const [{ x }, api] = useSpring(() => ({
x: props.defaultIndex * slideWidth,
config: { tension: 250, clamp: true },
Expand All @@ -51,6 +55,7 @@ export const Slides = forwardRef<SlidesRef, SlidesType>((props, ref) => {
}))

const dragLockRef = useRef(false)

const bind = useDrag(
state => {
if (dragLockRef.current) return
Expand Down
Loading