Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes the Grid View component's image loading strategy to reduce backend server pressure by implementing lazy loading with viewport detection. The changes introduce a delayed loading mechanism that only loads images when they enter the viewport and have remained visible for 500ms, preventing unnecessary requests during fast scrolling.
Key Changes:
- Implemented IntersectionObserver-based lazy loading with 500ms delay mechanism
- Added visibility tracking signals (isVisible, loaded, canLoad) to control image loading behavior
- Extended ImageWithError component to support onLoad callback for tracking loaded state
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/pages/home/folder/GridItem.tsx | Adds IntersectionObserver setup with timeout-based delay, visibility signals, and conditional image rendering logic |
| src/components/ImageWithError.tsx | Extends component to accept optional onLoad callback prop for tracking when images finish loading |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (ref) { | ||
| const observer = new IntersectionObserver( | ||
| ([entry]) => { | ||
| if (entry.isIntersecting) { |
There was a problem hiding this comment.
The timeout is not cleared when a new intersection event occurs while a previous timeout is still pending. If the element rapidly enters and exits the viewport, multiple setTimeout callbacks could be queued, potentially causing canLoad to be set to true even after the element has left the viewport. Clear the existing timeout before setting a new one.
| if (entry.isIntersecting) { | |
| if (entry.isIntersecting) { | |
| if (loadTimeout) clearTimeout(loadTimeout) |
| const { openWithDoubleClick, toggleWithClick, restoreSelectionCache } = | ||
| useSelectWithMouse() | ||
|
|
||
| const [isVisible, setIsVisible] = createSignal(false) |
There was a problem hiding this comment.
The isVisible signal is set but never used in the component. This adds unnecessary state management overhead. Consider removing this signal if it's not needed for the lazy loading implementation.
| if (loadTimeout) clearTimeout(loadTimeout) | ||
| setCanLoad(false) | ||
| } | ||
| setIsVisible(entry.isIntersecting) |
There was a problem hiding this comment.
The isVisible signal is updated but never used anywhere in the component logic or rendering. This adds unnecessary overhead. Either remove this line or utilize the signal if it's intended for future use.
| <Show when={!err()} fallback={props.fallbackErr}> | ||
| <Image | ||
| {...props} | ||
| onLoad={props.onLoad} |
There was a problem hiding this comment.
The onLoad prop is redundantly specified both in the spread operator and explicitly. Since props are spread with {...props}, the explicit onLoad={props.onLoad} is unnecessary. Remove the explicit onLoad line or restructure to avoid redundancy.
| onLoad={props.onLoad} |
| const [loaded, setLoaded] = createSignal(false) | ||
| const [canLoad, setCanLoad] = createSignal(false) | ||
| let ref: HTMLDivElement | undefined | ||
| let loadTimeout: number | undefined |
There was a problem hiding this comment.
The loadTimeout variable is typed as 'number | undefined', but in some TypeScript configurations (particularly with Node.js types), window.setTimeout returns a NodeJS.Timeout object rather than a number. This can cause type errors. Use 'ReturnType' or cast the result to ensure type compatibility across different environments.
| let loadTimeout: number | undefined | |
| let loadTimeout: ReturnType<typeof setTimeout> | undefined |
| let loadTimeout: number | undefined | ||
|
|
||
| onMount(() => { | ||
| if (ref) { |
There was a problem hiding this comment.
This use of variable 'ref' always evaluates to false.
Description / 描述
优化 Grid View 组件的图片加载逻辑,实现以下改进:
这些优化显著减少了云盘服务器的请求压力,有效降低因频繁请求可能触发的风控机制风险。
Motivation and Context / 背景
当前 Grid View 组件在渲染大量图片时会一次性加载所有缩略图,存在以下问题:
通过实现懒加载和延迟加载策略,我们可以:
How Has This Been Tested? / 测试
测试环境
测试方法
测试结果
Checklist / 检查清单