feat: infiniteloading default scroll #3010
Conversation
Walkthrough本次变更为 Changes
Sequence Diagram(s)sequenceDiagram
participant IL as InfiniteLoading 组件
participant Hook as useEffect 钩子
participant DOM as 滚动元素 (scrollEl)
IL->>Hook: 接收 defaultScrollTop 属性
Hook->>DOM: 计算子元素高度
Hook->>DOM: 在超时后更新 scrollTop 为 defaultScrollTop
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/infiniteloading/doc.md(1 hunks)src/packages/infiniteloading/infiniteloading.tsx(3 hunks)
🧰 Additional context used
🪛 ESLint
src/packages/infiniteloading/infiniteloading.tsx
[error] 84-84: Replace ·(getRefreshTop().firstElementChild·as·HTMLElement).offsetHeight·||·0; with ⏎········(getRefreshTop().firstElementChild·as·HTMLElement).offsetHeight·||·0
(prettier/prettier)
[error] 85-85: Delete ;
(prettier/prettier)
[error] 86-86: Insert ·
(prettier/prettier)
[error] 90-90: Delete ;
(prettier/prettier)
🔇 Additional comments (3)
src/packages/infiniteloading/infiniteloading.tsx (2)
26-26: 新属性定义正确且清晰！
defaultScrollTop属性的类型定义恰当，使用可选参数符合最佳实践。
64-64: 属性解构实现正确！
defaultScrollTop属性的解构位置恰当，保持了代码的一致性。src/packages/infiniteloading/doc.md (1)
71-71: 文档更新准确完整！新属性
defaultScrollTop的文档说明清晰，与实现保持一致。
| useEffect(() => { | ||
| if (defaultScrollTop) { | ||
| const childHeight = (getRefreshTop().firstElementChild as HTMLElement).offsetHeight || 0; | ||
| refreshMaxH.current = Math.floor(childHeight * 1 + 10); | ||
| setTimeout(() =>{ | ||
| scrollEl.current.scrollTop = defaultScrollTop | ||
| }, 10) | ||
| } | ||
| }, [defaultScrollTop]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议优化 useEffect 实现
当前实现存在以下需要改进的地方：
- 缺少清理函数以处理组件卸载情况
- setTimeout 使用了魔法数字 (10ms)
- 没有处理可能的空值情况
- 可能与 DOM 更新产生竞态条件
建议按照以下方式重构：
useEffect(() => {
if (defaultScrollTop) {
+ const element = getRefreshTop();
+ if (!element?.firstElementChild) return;
+
const childHeight = (getRefreshTop().firstElementChild as HTMLElement).offsetHeight || 0;
refreshMaxH.current = Math.floor(childHeight * 1 + 10);
- setTimeout(() =>{
- scrollEl.current.scrollTop = defaultScrollTop
- }, 10)
+ const timeoutId = setTimeout(() => {
+ if (scrollEl.current) {
+ scrollEl.current.scrollTop = defaultScrollTop;
+ }
+ }, 100); // 使用更合理的延迟时间
+
+ return () => clearTimeout(timeoutId);
}
}, [defaultScrollTop]);📝 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.
| useEffect(() => { | |
| if (defaultScrollTop) { | |
| const childHeight = (getRefreshTop().firstElementChild as HTMLElement).offsetHeight || 0; | |
| refreshMaxH.current = Math.floor(childHeight * 1 + 10); | |
| setTimeout(() =>{ | |
| scrollEl.current.scrollTop = defaultScrollTop | |
| }, 10) | |
| } | |
| }, [defaultScrollTop]); | |
| useEffect(() => { | |
| if (defaultScrollTop) { | |
| + const element = getRefreshTop(); | |
| + if (!element?.firstElementChild) return; | |
| + | |
| const childHeight = (getRefreshTop().firstElementChild as HTMLElement).offsetHeight || 0; | |
| refreshMaxH.current = Math.floor(childHeight * 1 + 10); | |
| - setTimeout(() =>{ | |
| - scrollEl.current.scrollTop = defaultScrollTop | |
| - }, 10) | |
| + const timeoutId = setTimeout(() => { | |
| + if (scrollEl.current) { | |
| + scrollEl.current.scrollTop = defaultScrollTop; | |
| + } | |
| + }, 100); // 使用更合理的延迟时间 | |
| + | |
| + return () => clearTimeout(timeoutId); | |
| } | |
| }, [defaultScrollTop]); |
🧰 Tools
🪛 ESLint
[error] 84-84: Replace ·(getRefreshTop().firstElementChild·as·HTMLElement).offsetHeight·||·0; with ⏎········(getRefreshTop().firstElementChild·as·HTMLElement).offsetHeight·||·0
(prettier/prettier)
[error] 85-85: Delete ;
(prettier/prettier)
[error] 86-86: Insert ·
(prettier/prettier)
[error] 90-90: Delete ;
(prettier/prettier)
github-actions
bot
added
the
action:review
oasis-cloud
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #3010 +/- ##
==========================================
- Coverage 85.56% 85.54% -0.02%
==========================================
Files 267 267
Lines 18003 18013 +10
Branches 2726 2729 +3
==========================================
+ Hits 15405 15410 +5
- Misses 2595 2600 +5
Partials 3 3 ☔ View full report in Codecov by Sentry. |
|
ci 问题可以先修复一下 |
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
提供设置默认滚动距离的能力
☑️ 请求合并前的自查清单
Summary by CodeRabbit
defaultScrollTop，允许用户设置加载时的默认滚动位置，从而改善滚动体验。defaultScrollTop属性的类型和用途。