Skip to content

fix: use positionViewAtBeginning instead of set contentY#748

Merged
BLumia merged 1 commit into
linuxdeepin:masterfrom
BLumia:pms-bug-356795
Apr 15, 2026
Merged

fix: use positionViewAtBeginning instead of set contentY#748
BLumia merged 1 commit into
linuxdeepin:masterfrom
BLumia:pms-bug-356795

Conversation

@BLumia
Copy link
Copy Markdown
Member

@BLumia BLumia commented Apr 15, 2026

It is not recommended to use contentX or contentY to position the view at a particular index. This is unreliable since removing items from the start of the list does not cause all other items to be repositioned, and because the actual start of the view can vary based on the size of the delegates.

PMS: BUG-356795

Summary by Sourcery

Bug Fixes:

  • Use positionViewAtBeginning() to reliably scroll the app list to the top instead of setting contentY to 0.

It is not recommended to use contentX or contentY to position the
view at a particular index. This is unreliable since removing items
from the start of the list does not cause all other items to be
repositioned, and because the actual start of the view can vary based
on the size of the delegates.

PMS: BUG-356795
Log:
@BLumia BLumia requested review from robertkill and wjyrich April 15, 2026 09:47
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 15, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Replaces direct manipulation of ListView scroll position via contentY with the recommended positionViewAtBeginning() API when resetting the app list view to the start, ensuring robust behavior when items are added/removed or delegate sizes vary.

Sequence diagram for resetting the app list view to the beginning

sequenceDiagram
    actor User
    participant AppListView
    participant ListView
    participant LauncherController

    User->>AppListView: triggerReset()
    AppListView->>ListView: set currentIndex(0)
    AppListView->>ListView: set highlightFollowsCurrentItem(false)
    AppListView->>ListView: positionViewAtBeginning()
    AppListView->>ListView: restore highlightFollowsCurrentItem(wasFollowing)
    AppListView->>LauncherController: check visible
    alt LauncherController not visible
        AppListView->>AppListView: alphabetCategoryPopup.close()
    end
Loading

File-Level Changes

Change Details Files
Use ListView.positionViewAtBeginning() instead of manually setting contentY when resetting the app list to the top.
  • Capture and temporarily disable highlightFollowsCurrentItem while resetting the ListView position to avoid highlight side effects
  • Set currentIndex to 0 before repositioning the view so the beginning item is current
  • Replace listView.contentY = 0 with listView.positionViewAtBeginning() to correctly align the view at the start according to ListView semantics
  • Restore highlightFollowsCurrentItem to its previous value after repositioning
qml/windowed/AppListView.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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码修改主要涉及 QML 中 ListView 的滚动行为控制。以下是对该 git diff 的详细审查意见:

1. 语法逻辑审查

  • 修改内容:将 listView.contentY = 0 替换为 listView.positionViewAtBeginning()
  • 分析
    • 原代码 (contentY = 0):这是一种强制设置视图内容 Y 轴偏移量的做法。它直接操作内部属性,假设视图的起始位置就是 0。这在大多数情况下有效,但如果列表有 header(表头)或 topMargin(顶部边距),直接设置 contentY 为 0 可能会导致视图定位不准确(例如表头被遮挡或显示不完整)。
    • 新代码 (positionViewAtBeginning()):这是 QML 提供的 API 方法,专门用于将视图滚动到列表的起始位置。它会自动处理 headermargins 的存在,确保列表的第一个可见项正确显示在视图中。
  • 结论:逻辑上,新代码更加健壮和语义化,它封装了"回到顶部"的复杂逻辑,而不是简单地重置坐标。

2. 代码质量审查

  • 可读性与可维护性positionViewAtBeginning() 是一个自解释的函数名,清晰地表达了"将视图定位到开头"的意图。相比之下,contentY = 0 只是底层的属性赋值,阅读代码时需要脑补其意图。新代码提高了代码的可读性。
  • 封装性:使用 API 方法(positionViewAtBeginning)优于直接操作内部属性(contentY),符合良好的封装原则。如果 Qt 框架在未来改变了内部实现细节,API 方法通常会保持兼容,而直接操作属性则可能失效。

3. 代码性能审查

  • 动画效果positionViewAtBeginning() 默认可能会触发平滑滚动动画(取决于 ListViewhighlightMoveDurationbehavior 设置),而 contentY = 0 是瞬间跳转。
    • 如果该函数的目的是"瞬间重置"(例如刷新列表时),原代码可能在视觉上更干脆。
    • 如果该函数的目的是"用户点击回到顶部",新代码带来的平滑滚动体验更好。
  • 计算开销:两者开销差异极小,可以忽略不计。positionViewAtBeginning 内部也是计算并设置 contentY

4. 代码安全审查

  • 边界条件处理:如前所述,positionViewAtBeginning() 能更好地处理包含 headermargins 的列表,避免了因直接设置 contentY 导致的布局错位风险。
  • 状态一致性:代码片段中先保存并禁用了 highlightFollowsCurrentItem,操作结束后再恢复。这是一个很好的做法,防止在滚动过程中高亮框产生不必要的动画跳动,保证了状态的一致性。

改进建议

虽然这次修改是正向的,但根据具体场景,可以考虑以下微调:

  1. 明确滚动模式:如果业务逻辑要求必须瞬间回到顶部(例如数据重载场景),而不需要任何过渡动画,建议使用 positionViewAtBeginning() 的重载形式,明确指定模式:

    listView.positionViewAtBeginning(ListView.Beginning)
    // 或者如果需要确保不触发动画,检查具体 Qt 版本的 API,
    // 某些版本可能需要配合 highlightMoveDuration = 0 使用

    注:标准 QML positionViewAtBeginning() 默认行为通常是平滑滚动的。如果原代码 contentY = 0 是为了实现"瞬间跳变"的效果,那么这次修改可能会改变 UI 交互体验(从瞬间跳变变为平滑滚动)。请确认这是否符合预期设计。

  2. 上下文检查:虽然 listView 应该已定义,但在更严格的代码审查中,确保 listView 对象在该作用域内确实存在且是 ListView 类型。

总结

这次修改是推荐的。它将直接操作属性替换为语义化的 API 调用,增强了代码的可读性和对复杂布局(如带 Header 的列表)的兼容性。唯一需要注意的是确认是否需要保留"瞬间跳转"的视觉效果,如果需要,可能需要配合禁用动画的属性使用。

Copy link
Copy Markdown

@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 reviewed your changes and they look great!


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.

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, wjyrich

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

@BLumia BLumia merged commit c74a59d into linuxdeepin:master Apr 15, 2026
11 checks passed
@BLumia BLumia deleted the pms-bug-356795 branch April 15, 2026 10:07
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