Skip to content

fix: prevent ListView overscroll in notification panel#1516

Open
qxp930712 wants to merge 1 commit intolinuxdeepin:masterfrom
qxp930712:master
Open

fix: prevent ListView overscroll in notification panel#1516
qxp930712 wants to merge 1 commit intolinuxdeepin:masterfrom
qxp930712:master

Conversation

@qxp930712
Copy link

@qxp930712 qxp930712 commented Mar 20, 2026

Added overscroll protection to the notification center ListView to prevent unwanted bounce-back behavior when scrolling beyond content boundaries. The fix includes three event handlers that monitor height, content height, and content position changes to ensure the scroll position stays within valid bounds.

Implemented bounds checking in onHeightChanged, onContentHeightChanged, and onContentYChanged handlers to clamp the contentY position when it exceeds the content boundaries. This prevents the ListView from showing empty space when scrolled beyond the actual content area, which was causing visual glitches and inconsistent scrolling behavior.

Influence:

  1. Test scrolling to the top and bottom of notification list
  2. Verify no empty space appears when reaching content boundaries
  3. Check that normal scrolling behavior remains smooth
  4. Test with various numbers of notifications (empty, few, many)
  5. Verify scroll position is maintained during content updates

PMS: BUG-284867

Summary by Sourcery

Bug Fixes:

  • Clamp the notification ListView scroll position to avoid showing empty space when scrolling beyond content limits.

Added overscroll protection to the notification center ListView to
prevent unwanted bounce-back behavior when scrolling beyond content
boundaries. The fix includes three event handlers that monitor height,
content height, and content position changes to ensure the scroll
position stays within valid bounds.

Implemented bounds checking in onHeightChanged, onContentHeightChanged,
and onContentYChanged handlers to clamp the contentY position when it
exceeds the content boundaries. This prevents the ListView from showing
empty space when scrolled beyond the actual content area, which was
causing visual glitches and inconsistent scrolling behavior.

Influence:
1. Test scrolling to the top and bottom of notification list
2. Verify no empty space appears when reaching content boundaries
3. Check that normal scrolling behavior remains smooth
4. Test with various numbers of notifications (empty, few, many)
5. Verify scroll position is maintained during content updates

PMS: BUG-284867
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qxp930712

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要针对 QML 中 ListView 的滚动位置(contentY)和内容高度(contentHeight)变化时的边界处理进行了逻辑修正,目的是解决"滚轮回弹"导致内容显示位置不正确的问题。

以下是对这段代码的详细审查意见:

1. 语法与逻辑审查

  • 逻辑一致性

    • 代码在 onHeightChangedonContentHeightChanged 中使用了相同的逻辑:if (contentHeight > height && contentY > contentHeight - height)。这表明当容器变高或内容变短时,如果当前滚动位置超出了新的底部边界,则强制吸附到底部。逻辑是正确的。
    • onContentYChanged 中,分别处理了顶部边界(< -1)和底部边界。这里使用了 -1+1 的容差值,这通常是为了防止浮点数精度误差导致的边界抖动,逻辑上也是合理的。
  • 潜在的性能问题(信号连锁)

    • 问题点:在 onContentYChanged 中直接修改 contentY 属性。
    • 分析:当手动设置 contentY 时,会再次触发 onContentYChanged 信号。虽然代码中有 if 判断作为保护,理论上在修正后(例如设为 0)再次触发时,if (contentY < -1) 将为假,从而停止递归。但在 QML 的绑定系统中,频繁修改属性可能会引起不必要的重绘或布局计算。
    • 建议:目前的写法在简单场景下通常是可以接受的,但如果发现滚动卡顿,可以考虑使用 return 语句确保逻辑分支清晰,或者检查是否可以通过 boundsBehavior 属性原生解决(见下文)。

2. 代码质量与规范

  • 注释

    • 注释 // ListView滚轮回弹行为是默认行为,需要手动修正 清晰地解释了添加这段代码的原因,这非常好。
    • 建议补充说明为什么使用 -1+1 的容差(例如:// 增加1像素容差以避免浮点数计算误差导致的边界抖动)。
  • 代码复用

    • onHeightChangedonContentHeightChanged 中的逻辑完全相同。虽然 QML 中不方便定义局部函数供 Signal Handler 调用,但这种重复是轻微的。

3. 改进意见与建议

建议 1:优先使用原生属性 boundsBehavior (最推荐)

QML 的 ListView 提供了 boundsBehavior 属性来控制边界行为。默认值通常是 ListView.DragAndOvershootBounds(允许拖动过界并回弹)。
如果目的是完全禁止回弹或过界滚动,直接设置属性比手动监听和修正 contentY 更高效且更符合框架设计。

修改方案:

ListView {
    // ...
    boundsBehavior: Flickable.StopAtBounds
    // ...
}
  • 优点:性能最好,代码最简洁,由 C++ 底层处理边界,无抖动。
  • 注意:如果业务逻辑确实需要"允许拖拽过界但松手后必须严格停在边界(不允许回弹动画)",那么现有的手动修正代码是必要的。但如果只是想防止内容"消失"或"卡在边界外",StopAtBounds 是最佳选择。

建议 2:优化现有代码(如果必须手动修正)

如果由于某些特殊原因(如复杂的动画交互)必须手动控制,建议对 onContentYChanged 做微调,增加 return 以提高可读性,并明确逻辑分支。

修改后的代码片段:

onContentYChanged: {
    // 顶部边界修正
    if (contentY < -1) {
        contentY = 0
        return // 明确返回,避免后续判断
    }
    
    // 底部边界修正
    // 仅当内容高度大于容器高度时才限制底部
    if (contentHeight > height) {
        const maxContentY = contentHeight - height
        if (contentY > maxContentY + 1) {
            contentY = maxContentY
        }
    }
}

建议 3:关于 onContentHeightChanged 的逻辑

onContentHeightChanged 中:

if (contentHeight > height && contentY > contentHeight - height)

contentHeight 减小(例如删除通知项)时,这个逻辑能正确地将视图吸附到底部。但如果 contentHeight 增大(例如新增通知项),且当前视图不在底部,通常希望保持 contentY 不变(即保持在顶部),目前的代码也能做到这一点(因为 contentY 不会大于 contentHeight - height)。逻辑是安全的。

总结

这段代码在逻辑上是正确的,能够解决 ListView 边界回弹的问题。

主要改进建议:

  1. 首选方案:检查是否可以使用 boundsBehavior: Flickable.StopAtBounds 替代所有手动修正代码,这样能显著提升性能和代码整洁度。
  2. 次选方案:如果必须保留手动修正,建议采纳上述"建议 2"中的代码结构,增加 return 和局部变量,提高代码可读性和鲁棒性。

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds overscroll protection to the notification center ListView by clamping contentY within valid bounds in response to height, content height, and scroll position changes, preventing bounce-back and empty-space glitches at the list edges.

File-Level Changes

Change Details Files
Clamp ListView scrolling to prevent overscroll and visual gaps at top/bottom of the notification list.
  • Add onHeightChanged handler to adjust contentY when the view height changes and the current scroll exceeds the bottom boundary.
  • Add onContentHeightChanged handler to re-clamp contentY when the content size changes, keeping the viewport within valid content bounds.
  • Add onContentYChanged handler to normalize minor negative overscroll at the top and cap overscroll beyond the bottom limit, ensuring no empty space is shown.
panels/notification/center/NotifyView.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The clamping logic for contentHeight > height && contentY > contentHeight - height is duplicated in onHeightChanged and onContentHeightChanged; consider extracting this into a small helper function to keep the behavior consistent and easier to maintain.
  • The magic offsets contentY < -1 and contentY > contentHeight - height + 1 in onContentYChanged could use either named constants or an inline comment explaining why a 1‑pixel tolerance is required here to avoid future confusion or accidental changes.
  • Since ListView/Flickable already expose boundsBehavior and related properties, it might be worth checking whether you can achieve the same non-overscroll behavior by configuring those instead of manually reassigning contentY in three handlers, which can be harder to reason about over time.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The clamping logic for `contentHeight > height && contentY > contentHeight - height` is duplicated in `onHeightChanged` and `onContentHeightChanged`; consider extracting this into a small helper function to keep the behavior consistent and easier to maintain.
- The magic offsets `contentY < -1` and `contentY > contentHeight - height + 1` in `onContentYChanged` could use either named constants or an inline comment explaining why a 1‑pixel tolerance is required here to avoid future confusion or accidental changes.
- Since ListView/Flickable already expose `boundsBehavior` and related properties, it might be worth checking whether you can achieve the same non-overscroll behavior by configuring those instead of manually reassigning `contentY` in three handlers, which can be harder to reason about over time.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.


// ListView滚轮回弹行为是默认行为,需要手动修正
onHeightChanged: {
if (contentHeight > height && contentY > contentHeight - height) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这三个是一样的逻辑吧,封装一下,

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.

4 participants