Skip to content

Conversation

@pengfeixx
Copy link
Contributor

@pengfeixx pengfeixx commented Nov 13, 2025

Fix the tab switching focus order

Log: Fix the tab switching focus order
pms: BUG-339891

Summary by Sourcery

Bug Fixes:

  • Ensure tab focus order matches visual order by rebuilding QML delegates after expanding or collapsing notification groups

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pengfeixx

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

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

@sourcery-ai
Copy link

sourcery-ai bot commented Nov 13, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR introduces a new rebuildDelegates method in NotifyModel to reset the model and force QML ListView delegate recreation, and invokes it after notification group expand/collapse operations to correct the tab focus order to match the visual order.

Sequence diagram for tab focus order update after expand/collapse

sequenceDiagram
participant User
participant NotifyModel
participant QML_ListView
User->>NotifyModel: Expand or collapse notification group
NotifyModel->>NotifyModel: expandApp()/collapseApp()
NotifyModel->>NotifyModel: rebuildDelegates()
NotifyModel->>QML_ListView: beginResetModel()
NotifyModel->>QML_ListView: endResetModel()
QML_ListView->>QML_ListView: Destroy and recreate delegates
QML_ListView->>User: Tab focus order matches visual order
Loading

Class diagram for updated NotifyModel

classDiagram
class NotifyModel {
  +void expandApp(int row)
  +void collapseApp(int row)
  +void rebuildDelegates()
  QList<AppNotifyItem *> m_appNotifies
}
NotifyModel : rebuildDelegates()
Loading

File-Level Changes

Change Details Files
Implement delegate rebuild mechanism via model reset
  • Add rebuildDelegates method in notifymodel.cpp with beginResetModel/endResetModel
  • Declare rebuildDelegates slot in notifymodel.h
panels/notification/center/notifymodel.cpp
panels/notification/center/notifymodel.h
Invoke rebuildDelegates after expanding and collapsing notification groups
  • Insert rebuildDelegates() call at end of expandApp
  • Insert rebuildDelegates() call at end of collapseApp
panels/notification/center/notifymodel.cpp

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 there - 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-bot
Copy link

deepin-bot bot commented Nov 13, 2025

TAG Bot

New tag: 2.0.18
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1349

{
// Force a model reset so that QML ListView delegates are destroyed and recreated
// in the current order, which helps synchronize Tab focus order with visual order.
beginResetModel();
Copy link
Contributor

@18202781743 18202781743 Nov 14, 2025

Choose a reason for hiding this comment

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

这个这样直接reset,可能会影响动画的,
为什么要让它重新生成呀,之前的代码是特意不让它重新生成的,焦点问题是不是可以按照当前应该的index,主动在界面上用listview的接口设置,让它特定的Item获得焦点,类似这样https://github.com/linuxdeepin/dde-shell/blob/master/panels/notification/center/NotifyView.qml#L32

@deepin-bot
Copy link

deepin-bot bot commented Nov 14, 2025

TAG Bot

New tag: 2.0.19
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1350

Fix the tab switching focus order

Log: Fix the tab switching focus order
pms: BUG-339891
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码变更进行审查:

  1. 功能逻辑分析:
  • 新增了rebuildDelegates()方法,通过beginResetModel()和endResetModel()强制重置模型
  • 在expandApp()和collapseApp()方法中调用rebuildDelegates()来重建委托
  • 目的是确保Tab键的焦点顺序与视觉顺序保持一致
  1. 代码质量评价:
    优点:
  • 添加了清晰的注释说明重建委托的目的
  • 方法命名直观,易于理解
  • 代码结构清晰,遵循了Qt的模型重置标准流程

可改进之处:

  • beginResetModel()/endResetModel()会触发整个视图的重建,可能会造成性能开销
  • 可以考虑只在必要时才重建委托,比如当焦点顺序确实需要更新时
  1. 性能考虑:
  • 每次展开/折叠应用都会触发整个模型重置,这可能导致:
    • UI闪烁
    • 滚动位置丢失
    • 不必要的性能开销
  1. 安全性:
  • 代码本身没有安全性问题
  • 但建议添加错误处理,比如在模型重置前检查模型状态

改进建议:

void NotifyModel::rebuildDelegates()
{
    // 只在有可见项时才重置模型
    if (rowCount() == 0) {
        return;
    }
    
    // 记录当前滚动位置
    emit saveScrollPosition();
    
    beginResetModel();
    endResetModel();
    
    // 恢复滚动位置
    emit restoreScrollPosition();
}

同时,可以考虑以下优化方案:

  1. 使用更细粒度的更新机制,而不是完全重置模型
  2. 添加一个标志位来跟踪是否需要重建委托
  3. 考虑使用QPersistentModelIndex来维护重要的视图状态
private:
    bool m_needsDelegateRebuild = false;

void NotifyModel::setNeedsDelegateRebuild(bool needs)
{
    if (m_needsDelegateRebuild != needs) {
        m_needsDelegateRebuild = needs;
        if (needs) {
            QMetaObject::invokeMethod(this, [this]() {
                if (m_needsDelegateRebuild) {
                    rebuildDelegates();
                    m_needsDelegateRebuild = false;
                }
            }, Qt::QueuedConnection);
        }
    }
}

这样可以:

  • 避免频繁的模型重置
  • 通过异步调用提高响应性
  • 减少不必要的UI更新

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