Skip to content

Conversation

@robertkill
Copy link
Contributor

@robertkill robertkill commented Dec 30, 2025

  1. Added logic to reset view state to top when switching between grid and list view modes
  2. The reset happens immediately without animation to provide instant feedback
  3. This ensures users always see the top of the app list after changing sort modes
  4. Prevents confusion when switching modes leaves users at a random scroll position

Influence:

  1. Test switching between grid and list view modes
  2. Verify view always resets to top position after mode switch
  3. Check that reset happens instantly without animation
  4. Ensure app list navigation remains consistent across mode changes

fix: 切换排序模式时重置视图状态

  1. 添加逻辑在网格和列表视图模式切换时重置视图到顶部
  2. 重置立即发生且无动画,提供即时反馈
  3. 确保用户在更改排序模式后始终能看到应用列表顶部
  4. 防止切换模式时用户停留在随机滚动位置造成的混淆

Influence:

  1. 测试在网格和列表视图模式之间切换
  2. 验证模式切换后视图始终重置到顶部位置
  3. 检查重置是否立即发生且无动画
  4. 确保应用列表导航在不同模式切换间保持一致

PMS: BUG-346111

Summary by Sourcery

Bug Fixes:

  • Ensure switching between grid and list app views always scrolls to the top to avoid leaving users at an arbitrary scroll position.

@sourcery-ai
Copy link

sourcery-ai bot commented Dec 30, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Resets the app list view to the top immediately and without animation whenever the sort mode is switched between grid and list, by invoking a new view reset hook on the active loader item after updating its source component.

Sequence diagram for resetting view state on sort mode change

sequenceDiagram
    actor User
    participant AppList
    participant Loader
    participant AppListView

    User->>AppList: changeSortMode(newMode)
    AppList->>Loader: set sourceComponent(gridView or listView)
    Loader-->>AppList: sourceComponent updated
    AppList->>Loader: access item
    alt loader.item exists
        AppList->>AppListView: resetViewState()
        AppListView->>AppListView: setScrollPosition(0, noAnimation)
    end
    AppListView-->>User: top of app list visible immediately
Loading

Class diagram for AppList and view reset hook on sort mode change

classDiagram
    class AppList {
        +Loader loader
        +changeSortMode(newMode)
    }

    class Loader {
        +AppListView item
        +setSourceComponent(component)
    }

    class AppListView {
        <<interface>>
        +resetViewState()
    }

    class GridView {
        +resetViewState()
    }

    class ListView {
        +resetViewState()
    }

    AppList --> Loader : uses
    Loader --> AppListView : holds item
    AppListView <|.. GridView : implements
    AppListView <|.. ListView : implements
    AppList ..> AppListView : calls resetViewState() after sort mode change
Loading

File-Level Changes

Change Details Files
Reset the loaded app list view state to the top whenever the sort mode (grid/list) is toggled.
  • After updating the loader sourceComponent based on the sort mode, invoke resetViewState() on the loaded item if it exists
  • Document the behavior with an inline comment explaining that the view is reset to the top immediately and without animation
qml/windowed/AppList.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:

  • Consider guarding the resetViewState() call (e.g., checking it exists or documenting it as part of the required interface) so that changing sourceComponent to something that doesn’t implement this method doesn’t cause runtime errors.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider guarding the `resetViewState()` call (e.g., checking it exists or documenting it as part of the required interface) so that changing `sourceComponent` to something that doesn’t implement this method doesn’t cause runtime errors.

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

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

1. Added logic to reset view state to top when switching between grid
and list view modes
2. The reset happens immediately without animation to provide instant
feedback
3. This ensures users always see the top of the app list after changing
sort modes
4. Prevents confusion when switching modes leaves users at a random
scroll position

Influence:
1. Test switching between grid and list view modes
2. Verify view always resets to top position after mode switch
3. Check that reset happens instantly without animation
4. Ensure app list navigation remains consistent across mode changes

fix: 切换排序模式时重置视图状态

1. 添加逻辑在网格和列表视图模式切换时重置视图到顶部
2. 重置立即发生且无动画,提供即时反馈
3. 确保用户在更改排序模式后始终能看到应用列表顶部
4. 防止切换模式时用户停留在随机滚动位置造成的混淆

Influence:
1. 测试在网格和列表视图模式之间切换
2. 验证模式切换后视图始终重置到顶部位置
3. 检查重置是否立即发生且无动画
4. 确保应用列表导航在不同模式切换间保持一致

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

deepin pr auto review

我来帮你分析这段代码的改动:

  1. 代码逻辑分析:
  • 这段代码在排序模式切换后添加了一个重置视图到顶部的功能
  • 使用了 loader.item 来访问当前加载的组件
  • 调用 resetViewState() 方法来重置视图状态
  1. 改进建议:

a) 安全性改进:

  • 建议在调用 resetViewState() 前检查该方法是否存在,避免运行时错误:
if (loader.item && typeof loader.item.resetViewState === "function") {
    loader.item.resetViewState()
}

b) 代码质量改进:

  • 建议将这个功能封装成一个独立的函数,提高代码可维护性:
function resetViewToTop() {
    if (loader.item && typeof loader.item.resetViewState === "function") {
        loader.item.resetViewState()
    }
}

c) 性能考虑:

  • 当前实现是合理的,因为:
    • 只在排序模式切换时执行
    • 使用了条件检查避免不必要的操作
    • 没有动画效果,减少了性能开销

d) 代码规范:

  • 建议添加注释说明这个功能的目的和行为
  • 考虑将这个逻辑移到 Loader 的 onLoaded 信号处理中,使代码结构更清晰
  1. 其他建议:
  • 可以考虑添加一个状态变量来跟踪视图是否已经重置,避免重复重置
  • 如果 resetViewState 是一个异步操作,可能需要添加相应的状态处理

总体来说,这个改动解决了切换排序模式后视图位置的问题,实现简洁有效。只需要在安全性检查方面做一些加强即可。

@robertkill robertkill merged commit 440db16 into linuxdeepin:master Dec 30, 2025
10 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.

4 participants