Skip to content

fix: iOS ScrollView insets growing continuously without reset logic#1197

Closed
RoeKun wants to merge 9 commits intoTencent-TDS:mainfrom
RoeKun:bugfix/valo_iOS_scrollviewInsets_reset
Closed

fix: iOS ScrollView insets growing continuously without reset logic#1197
RoeKun wants to merge 9 commits intoTencent-TDS:mainfrom
RoeKun:bugfix/valo_iOS_scrollviewInsets_reset

Conversation

@RoeKun
Copy link
Copy Markdown
Collaborator

@RoeKun RoeKun commented Apr 1, 2026

fix: iOS ScrollView insets growing continuously without reset logic

@RoeKun RoeKun requested a review from wwwcg as a code owner April 1, 2026 09:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses an iOS overscroll behavior where KRScrollView’s contentInset could continuously increase because inset expansion logic was applied during non-drag (programmatic/animated) scrolling.

Changes:

  • Gate contentInset expansion in maxEdgeInsetsWithContentOffset: so it only happens while the user is actively dragging (_isCurrentlyDragging).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return UIEdgeInsetsMake(-contentOffset.y, self.contentInset.left, self.contentInset.bottom, self.contentInset.right);
// 只有是业务主动的拖拽,才会出现边界变化的情况;
// 如果是滑动的话,那么是不会超越边界的
if (_isCurrentlyDragging) {
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

maxEdgeInsetsWithContentOffset: now relies on _isCurrentlyDragging, but that flag is only toggled in scrollViewWillBeginDragging:/scrollViewDidEndDragging:. If the view is reused/removed mid-drag (e.g., Compose reuse via css_prepareForComposeReuse) without receiving scrollViewDidEndDragging:, _isCurrentlyDragging can stay YES and this method will keep mutating contentInset during programmatic contentOffset updates—reintroducing the “inset keeps growing” behavior. Consider defensively resetting _isCurrentlyDragging = NO in reuse/cancel paths (at least css_prepareForComposeReuse, and possibly css_abortContentOffsetAnimate / lifecycle teardown).

Suggested change
if (_isCurrentlyDragging) {
// 这里同时参考系统的拖拽状态,避免 _isCurrentlyDragging 异常未重置时误判为拖拽
BOOL shouldAdjustInsets = (_isCurrentlyDragging || self.isTracking || self.isDragging) && !self.isDecelerating;
if (shouldAdjustInsets) {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@luoyibu luoyibu left a comment

Choose a reason for hiding this comment

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

Review 分析

修改方向正确,fling/回弹期间禁止扩大 inset 能有效解决原始问题。以下是几个需要关注的风险点:

风险 1(中风险):下拉刷新的程序化 setContentOffset 会失效

maxEdgeInsetsWithContentOffset: 的原始设计目的是:当 Kotlin 侧程序化调用 setContentOffset 到负值位置时(比如程序化触发下拉刷新动画),自动扩大 inset 使 scrollView 能停留在该位置。

加了 _isCurrentlyDragging 限制后,非拖拽状态下的程序化 setContentOffset 到负值将不再自动扩大 inset。如果业务有"代码触发下拉刷新"或"程序化滚动到负偏移位置"的场景,contentOffset 设置后会被 UIScrollView 弹回,因为 inset 没有被扩大。

建议:确认是否存在非拖拽状态下 Kotlin 侧主动调用 contentOffset 方法将 scrollView 滚动到负偏移位置的业务场景。如果有,需要额外的标记来区分"scroll 事件回调中的 setContentOffset"和"业务主动的程序化 setContentOffset"。

风险 2(低风险):_isCurrentlyDragging 状态残留

虽然已在 css_prepareForComposeReuse 中做了防御性重置,但还有其他路径可能导致状态残留:

  • css_abortContentOffsetAnimate:中止动画时没有重置 _isCurrentlyDragging。虽然理论上中止动画不应该影响拖拽状态,但如果在拖拽过程中被调用(比如 Kotlin 侧在 scroll 事件中调用 abort),scrollViewDidEndDragging: 可能不会被正常触发。
  • 视图从 window 移除:如果 scrollView 在拖拽过程中被移除(非 Compose reuse 路径),scrollViewDidEndDragging: 不会被调用,_isCurrentlyDragging 残留为 YES。

不过这个风险较低,因为残留 YES 只会导致回到旧的行为(允许扩大 inset),不会引入新 bug。

风险 3(低风险):!self.isDecelerating 条件冗余但无害

_isCurrentlyDragging == YES 时(scrollViewWillBeginDragging: 已触发),self.isDecelerating 理论上应该是 NO(用户手指在屏幕上,不可能同时在减速)。这个条件是双重保险,不会造成问题,但逻辑上是冗余的。

风险 4(建议):缺少 inset 恢复机制

当前修改只是"不再扩大 inset",但没有处理已经被扩大的 inset 何时恢复的问题。如果用户拖拽期间 inset 被扩大了,拖拽结束后这个扩大的 inset 会一直保留。原始代码也有这个问题,不是本 PR 引入的,但值得关注。

@RoeKun RoeKun requested a review from luoyibu April 7, 2026 13:19
@RoeKun RoeKun closed this Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants