Skip to content

refactor: remove redundant positionViewAtBeginning functions#736

Merged
wjyrich merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-335535
Mar 16, 2026
Merged

refactor: remove redundant positionViewAtBeginning functions#736
wjyrich merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-335535

Conversation

@wjyrich
Copy link
Contributor

@wjyrich wjyrich commented Mar 16, 2026

Removed multiple positionViewAtBeginning() functions from various QML view components (GridViewContainer, AnalysisView, FreeSortListView, FrequentlyUsedView, RecentlyInstalledView, SearchResultView) as they were redundant wrapper functions that simply called the underlying QML view's positionViewAtBeginning() method. Added a new itemBroughtToFront() signal to ItemArrangementProxyModel that triggers automatic view repositioning in FreeSortListView when items are brought to front, eliminating the need for manual view positioning calls.

Log: N/A

Influence:

  1. Test that bringing items to front in free sort mode automatically scrolls the view to beginning
  2. Verify all grid and list views still function correctly without explicit positionViewAtBeginning calls
  3. Test search results display and navigation
  4. Verify frequently used and recently installed views maintain proper scrolling behavior
  5. Test drag and drop operations in free sort mode
  6. Verify view focus behavior remains intact

refactor: 移除冗余的positionViewAtBeginning函数

从多个QML视图组件(GridViewContainer、AnalysisView、FreeSortListView、 FrequentlyUsedView、RecentlyInstalledView、SearchResultView)中移除了 positionViewAtBeginning()函数,这些函数是冗余的包装函数,仅调用底层QML视 图的positionViewAtBeginning()方法。在ItemArrangementProxyModel中添加了新 的itemBroughtToFront()信号,当项目被置顶时自动触发FreeSortListView中的视 图重新定位,消除了手动调用视图定位的需求。

Log: 无

Influence:

  1. 测试在自由排序模式下将项目置顶时是否自动滚动视图到起始位置
  2. 验证所有网格和列表视图在没有显式positionViewAtBeginning调用的情况下仍 能正常工作
  3. 测试搜索结果展示和导航功能
  4. 验证常用应用和最近安装视图保持正确的滚动行为
  5. 测试自由排序模式下的拖放操作
  6. 验证视图焦点行为保持完整

PMS: BUG-335535

Summary by Sourcery

Refactor QML views to remove redundant positionViewAtBeginning wrappers and hook automatic scrolling to model changes when items are brought to the front.

New Features:

  • Trigger automatic repositioning of the free-sort list view to the beginning when items are brought to the front via the ItemArrangementProxyModel.

Enhancements:

  • Remove redundant positionViewAtBeginning() helper functions from multiple QML view components now that views can rely directly on their internal positionViewAtBeginning APIs.
  • Emit a new itemBroughtToFront signal from ItemArrangementProxyModel during bringToFront operations so views can update scroll position reactively.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 16, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors several QML views by removing redundant positionViewAtBeginning() wrapper functions and introduces a new itemBroughtToFront signal in ItemArrangementProxyModel, which FreeSortListView now listens to in order to automatically scroll to the beginning when items are brought to the front.

Sequence diagram for automatic view repositioning on itemBroughtToFront

sequenceDiagram
    actor User
    participant FreeSortListView
    participant ItemArrangementProxyModel
    participant listView

    User ->> FreeSortListView: dragAndDropItemToFront()
    FreeSortListView ->> ItemArrangementProxyModel: bringToFront(id)
    ItemArrangementProxyModel ->> ItemArrangementProxyModel: reorderItems()
    ItemArrangementProxyModel -->> ItemArrangementProxyModel: emit dataChanged
    ItemArrangementProxyModel -->> FreeSortListView: itemBroughtToFront
    FreeSortListView ->> FreeSortListView: onItemBroughtToFront()
    FreeSortListView ->> listView: positionViewAtBeginning()
Loading

Class diagram for ItemArrangementProxyModel and FreeSortListView changes

classDiagram
    class QConcatenateTablesProxyModel

    class ItemArrangementProxyModel {
        +ItemArrangementProxyModel(QObject parent)
        +void bringToFront(QString id)
        +void commitDndOperation(QString dragId, QString dropId, DndOperation op, int pageHint)
        <<signal>> void topLevelPageCountChanged()
        <<signal>> void folderPageCountChanged(int folderId)
        <<signal>> void itemBroughtToFront()
    }

    class FreeSortListView {
        +onFocusChanged()
        +onItemBroughtToFront()
        +resetViewState()
        ListView listView
    }

    class ListView {
        +void positionViewAtBeginning()
        +bool focus
    }

    ItemArrangementProxyModel --|> QConcatenateTablesProxyModel
    FreeSortListView ..> ItemArrangementProxyModel : listensTo itemBroughtToFront
    FreeSortListView o--> ListView : contains listView
    FreeSortListView ..> ListView : calls positionViewAtBeginning
Loading

File-Level Changes

Change Details Files
Removed redundant positionViewAtBeginning wrapper functions in various QML view components so callers use the underlying view APIs directly.
  • Deleted positionViewAtBeginning() in windowed/AnalysisView and rely on frequentlyUsedView/recentlyInstalledView containers directly for positioning if needed.
  • Deleted positionViewAtBeginning() in both root and windowed GridViewContainer components, removing simple pass-through wrappers to gridView.positionViewAtBeginning().
  • Deleted positionViewAtBeginning() in FrequentlyUsedView, RecentlyInstalledView, and SearchResultView, which only forwarded to their internal *ViewContainer components.
qml/windowed/AnalysisView.qml
qml/GridViewContainer.qml
qml/windowed/GridViewContainer.qml
qml/windowed/FrequentlyUsedView.qml
qml/windowed/RecentlyInstalledView.qml
qml/windowed/SearchResultView.qml
Wire FreeSortListView to automatically reposition at the beginning when items are brought to the front via a new ItemArrangementProxyModel signal instead of manual calls.
  • Added an itemBroughtToFront() signal to ItemArrangementProxyModel and emitted it at the end of bringToFront().
  • Added a Connections block in FreeSortListView that listens to ItemArrangementProxyModel.onItemBroughtToFront to call listView.positionViewAtBeginning().
  • Removed the now-redundant positionViewAtBeginning() function from FreeSortListView while keeping focus handling behavior unchanged.
src/models/itemarrangementproxymodel.h
src/models/itemarrangementproxymodel.cpp
qml/windowed/FreeSortListView.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 Connections in FreeSortListView.qml uses target: ItemArrangementProxyModel, which looks like a type rather than a specific model instance; consider binding the target to the actual proxy model object used by this view to avoid unintended global coupling or missed signals.
  • ItemArrangementProxyModel::bringToFront always emits itemBroughtToFront() even if the item is already at the front; you may want to guard this with a check so the signal is only emitted when the ordering actually changes, to avoid unnecessary repositioning in the view.
  • Since itemBroughtToFront has no parameters, multiple FreeSortListView instances listening to the same model cannot distinguish which view should react; consider adding context (e.g., the item id or an index) or scoping the signal more tightly if multiple views can be backed by the same proxy model.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `Connections` in `FreeSortListView.qml` uses `target: ItemArrangementProxyModel`, which looks like a type rather than a specific model instance; consider binding the target to the actual proxy model object used by this view to avoid unintended global coupling or missed signals.
- `ItemArrangementProxyModel::bringToFront` always emits `itemBroughtToFront()` even if the item is already at the front; you may want to guard this with a check so the signal is only emitted when the ordering actually changes, to avoid unnecessary repositioning in the view.
- Since `itemBroughtToFront` has no parameters, multiple `FreeSortListView` instances listening to the same model cannot distinguish which view should react; consider adding context (e.g., the item id or an index) or scoping the signal more tightly if multiple views can be backed by the same proxy model.

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.

@wjyrich wjyrich force-pushed the fix-bug-335535 branch 2 times, most recently from 2735ce7 to 2b7a8b6 Compare March 16, 2026 07:51
Copy link
Member

@BLumia BLumia left a comment

Choose a reason for hiding this comment

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

之前 positionViewAtBeginning 这个会在显示启动器窗口的时候调一下来确保显示的时候它总是被重置到顶部了。现在这个修改后这个行为还在吗?

@wjyrich
Copy link
Contributor Author

wjyrich commented Mar 16, 2026

之前 positionViewAtBeginning 这个会在显示启动器窗口的时候调一下来确保显示的时候它总是被重置到顶部了。现在这个修改后这个行为还在吗?

行为还在,现在主要是由resetViewState来管理关闭启动器后的逻辑,目前是看到相关函数,没有地方使用了就删掉了。。

Removed multiple positionViewAtBeginning() functions from various QML
view components (GridViewContainer, AnalysisView, FreeSortListView,
FrequentlyUsedView, RecentlyInstalledView, SearchResultView)
as they were redundant wrapper functions that simply called the
underlying QML view's positionViewAtBeginning() method. Added a new
itemBroughtToFront() signal to ItemArrangementProxyModel that triggers
automatic view repositioning in FreeSortListView when items are brought
to front, eliminating the need for manual view positioning calls.

Log: N/A

Influence:
1. Test that bringing items to front in free sort mode automatically
scrolls the view to beginning
2. Verify all grid and list views still function correctly without
explicit positionViewAtBeginning calls
3. Test search results display and navigation
4. Verify frequently used and recently installed views maintain proper
scrolling behavior
5. Test drag and drop operations in free sort mode
6. Verify view focus behavior remains intact

refactor: 移除冗余的positionViewAtBeginning函数

从多个QML视图组件(GridViewContainer、AnalysisView、FreeSortListView、
FrequentlyUsedView、RecentlyInstalledView、SearchResultView)中移除了
positionViewAtBeginning()函数,这些函数是冗余的包装函数,仅调用底层QML视
图的positionViewAtBeginning()方法。在ItemArrangementProxyModel中添加了新
的itemBroughtToFront()信号,当项目被置顶时自动触发FreeSortListView中的视
图重新定位,消除了手动调用视图定位的需求。

Log: 无

Influence:
1. 测试在自由排序模式下将项目置顶时是否自动滚动视图到起始位置
2. 验证所有网格和列表视图在没有显式positionViewAtBeginning调用的情况下仍
能正常工作
3. 测试搜索结果展示和导航功能
4. 验证常用应用和最近安装视图保持正确的滚动行为
5. 测试自由排序模式下的拖放操作
6. 验证视图焦点行为保持完整

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

deepin pr auto review

代码审查报告

1. 总体评价

本次提交主要对代码进行了重构,将多个QML组件中的 positionViewAtBeginning() 方法调用,从显式调用改为基于信号驱动(Signal-Slot)的自动触发机制。同时更新了部分文件的版权年份声明。整体逻辑清晰,减少了外部组件对视图内部状态的直接干预,符合Qt/QML的信号驱动编程范式。

2. 详细审查

2.1 逻辑与功能变更

  • 核心改动

    • 删除:在 GridViewContainer.qmlAnalysisView.qmlFrequentlyUsedView.qmlRecentlyInstalledView.qmlSearchResultView.qml 中删除了 positionViewAtBeginning() 函数。这意味着这些组件不再对外提供直接重置视图位置的接口。
    • 新增:在 FreeSortListView.qml 中引入了 Connections 对象,监听 ItemArrangementProxyModelitemBroughtToFront() 信号。当模型中的项目被置顶时,自动调用 listView.positionViewAtBeginning()
    • 后端变更:在 itemarrangementproxymodel.cppbringToFront 方法末尾添加了 emit itemBroughtToFront(),并在头文件中声明了该信号。
  • 逻辑影响

    • 副作用分析:之前的逻辑允许外部调用者(如父组件或控制器)在任意时刻调用 positionViewAtBeginning() 来重置视图。改动后,重置视图的唯一触发条件是 ItemArrangementProxyModel 发出 itemBroughtToFront 信号。
    • 潜在问题:如果 FreeSortListView 是唯一需要监听此信号并滚动的视图,那么改动是合理的。但如果 GridViewContainer(通常用于桌面或文件夹视图)中的视图也需要响应模型变化而滚动,目前的改动会导致 GridViewContainer 失去响应模型置顶操作而自动滚动的功能。建议确认 GridViewContainer 是否也需要响应 itemBroughtToFront 信号。如果需要,它也应该像 FreeSortListView 一样添加 Connections

2.2 代码质量与规范

  • 版权声明
    • SPDX-FileCopyrightText 从单一年份(如 2023 或 2024)更新为范围(如 2023 - 2026),这是标准的做法,符合REUSE规范。
  • 代码格式
    • itemarrangementproxymodel.cppemit itemBroughtToFront(); 后的空格和分号使用正确。
    • QML 中的 Connections 写法符合 QML 语法规范。

2.3 代码性能

  • 性能影响
    • 引入 Connections 监听信号是轻量级的操作。
    • 将函数调用改为信号槽机制,可能会带来微不足道的性能开销(信号分发机制),但在UI交互场景下可忽略不计。
    • bringToFront 方法中已经触发了 dataChanged,再触发 itemBroughtToFront 信号是必要的,用于通知UI层进行视图更新,逻辑合理。

2.4 代码安全

  • 安全性
    • 本次改动不涉及内存管理、权限控制或外部输入处理,因此没有引入明显的安全风险。
    • 信号槽机制有助于解耦,降低了组件间的耦合度,从架构角度看是更安全的。

3. 改进建议

  1. 功能完整性检查(重要)

    • 如上文逻辑分析所述,请确认 GridViewContainer.qml(位于 qml/qml/windowed/ 下)是否也需要在模型项置顶时自动滚动。
    • 如果 GridViewContainer 也需要此行为,建议在其代码中也添加对 ItemArrangementProxyModel.itemBroughtToFront 的监听。
    • 如果不需要,请确保删除显式的 positionViewAtBeginning 函数不会导致其他调用该函数的地方(如 C++ 侧或其他未被 Diff 包含的 QML 文件)出现编译错误或运行时异常。
  2. 信号命名与语义

    • itemBroughtToFront 这个信号名非常具体。如果未来有其他原因也需要触发视图回到顶部(例如:搜索清空、排序变更),目前的实现可能需要扩展。目前的实现仅针对"置顶"操作,如果这是预期行为,则无需修改。
  3. 代码注释

    • 建议在 FreeSortListView.qmlConnections 块前添加简要注释,解释为什么这里要监听该信号(例如:当模型项顺序变化时,确保视图回到顶部以展示最新状态)。
    // 当模型中有项目被置顶时,自动滚动视图到起始位置
    Connections {
        target: ItemArrangementProxyModel
        function onItemBroughtToFront() {
            listView.positionViewAtBeginning()
        }
    }

4. 总结

该代码改动通过引入信号槽机制,优化了视图与模型之间的交互逻辑,减少了直接的方法调用,提高了代码的解耦程度。主要风险在于是否遗漏了其他需要响应此信号的视图组件。建议在合并前确认 GridViewContainer 等组件的行为是否符合预期。

@deepin-ci-robot
Copy link

[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

@wjyrich wjyrich merged commit 3cc4074 into linuxdeepin:master Mar 16, 2026
11 checks passed
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