Skip to content

fix: correct ActiveRole and AttentionRole data retrieval for active#1570

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bgNoChange
Apr 27, 2026
Merged

fix: correct ActiveRole and AttentionRole data retrieval for active#1570
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bgNoChange

Conversation

@wjyrich
Copy link
Copy Markdown
Contributor

@wjyrich wjyrich commented Apr 27, 2026

app model

  1. Added case handling for TaskManager::ActiveRole and TaskManager::AttentionRole in the data() method of DockGlobalElementModel
  2. Fixed issue where these roles returned incorrect values when queried from the active app model (m_activeAppModel)
  3. Previously, these roles were not explicitly handled, potentially falling through to a default behavior that could return wrong data
  4. Now correctly delegates to the underlying model's data method for the specified roles when the model is the active app model
  5. Returns false for other models (e.g., window group model) as these concepts don't apply there

Log: Fixed incorrect active state and attention state display for dock app icons

Influence:

  1. Verify that active dock app icons properly reflect their active state
  2. Test attention-requesting apps show correct visual feedback (e.g., blinking)
  3. Check that window group icons do not incorrectly display active/ attention states
  4. Regression test on app launching and window switching scenarios
  5. Test with multiple workspaces and virtual desktops

fix: 修复活跃应用模型的 ActiveRole 和 AttentionRole 数据获取

  1. DockGlobalElementModeldata() 方法中增加了 TaskManager::ActiveRoleTaskManager::AttentionRole 的处理分支
  2. 修复了从活跃应用模型查询这些角色时返回错误值的问题
  3. 现在对于活跃应用模型会正确委托到底层模型的数据方法
  4. 其他模型(如窗口组模型)返回 false,因为这些概念不适用

Log: 修复任务栏应用图标活跃状态和提醒状态显示异常

Influence:

  1. 验证任务栏中活跃应用的图标是否正确显示活跃状态
  2. 测试请求关注的应用是否能显示正确的视觉反馈(如闪烁)
  3. 确保窗口组图标不会错误显示活跃/关注状态
  4. 回归测试应用启动和窗口切换场景
  5. 测试多工作区和虚拟桌面下的表现

此问题主要是处理
拆分模式下,关闭某个驻留应用窗口时(有焦点的窗口),他的背景不会消失,问题在于:属性绑定机制里,如果绑定的数据源返回了 undefined(对应 C++ 的空的/无效的 QVariant),QML
会选择不更新此属性,保留上一次的旧值!

因此 默认default返回的是一个 return {}; 会导致 保留了上次值,导致问题。

Summary by Sourcery

Fix active and attention state data retrieval for dock app entries in the global element model.

Bug Fixes:

  • Correct ActiveRole and AttentionRole data returned from the active app model by delegating to the underlying model.
  • Prevent non-active models such as window groups from incorrectly reporting active or attention states.

app model

1. Added case handling for `TaskManager::ActiveRole` and
`TaskManager::AttentionRole` in the `data()` method of
`DockGlobalElementModel`
2. Fixed issue where these roles returned incorrect values when queried
from the active app model (`m_activeAppModel`)
3. Previously, these roles were not explicitly handled, potentially
falling through to a default behavior that could return wrong data
4. Now correctly delegates to the underlying model's data method for the
specified roles when the model is the active app model
5. Returns `false` for other models (e.g., window group model) as these
concepts don't apply there

Log: Fixed incorrect active state and attention state display for dock
app icons

Influence:
1. Verify that active dock app icons properly reflect their active state
2. Test attention-requesting apps show correct visual feedback (e.g.,
blinking)
3. Check that window group icons do not incorrectly display active/
attention states
4. Regression test on app launching and window switching scenarios
5. Test with multiple workspaces and virtual desktops

fix: 修复活跃应用模型的 ActiveRole 和 AttentionRole 数据获取

1. 在 `DockGlobalElementModel` 的 `data()` 方法中增加了
`TaskManager::ActiveRole` 和 `TaskManager::AttentionRole` 的处理分支
2. 修复了从活跃应用模型查询这些角色时返回错误值的问题
3. 现在对于活跃应用模型会正确委托到底层模型的数据方法
4. 其他模型(如窗口组模型)返回 `false`,因为这些概念不适用

Log: 修复任务栏应用图标活跃状态和提醒状态显示异常

Influence:
1. 验证任务栏中活跃应用的图标是否正确显示活跃状态
2. 测试请求关注的应用是否能显示正确的视觉反馈(如闪烁)
3. 确保窗口组图标不会错误显示活跃/关注状态
4. 回归测试应用启动和窗口切换场景
5. 测试多工作区和虚拟桌面下的表现

此问题主要是处理
拆分模式下,关闭某个驻留应用窗口时(有焦点的窗口),他的背景不会消失,问题在于:属性绑定机制里,如果绑定的数据源返回了
undefined(对应 C++ 的空的/无效的 QVariant),QML
会选择不更新此属性,保留上一次的旧值!

因此 默认default返回的是一个 return {}; 会导致 保留了上次值,导致问题。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段. 语法逻辑审查:

  • 代码在语法上是正确的,符合C++的switch-case语句结构。
  • 逻辑上,对于ActiveRoleAttentionRole的处理是:如果当前model是m_activeAppModel,则返回对应的数据;否则返回false。
  1. 代码质量:

    • 代码的可读性良好,变量命名清晰。
    • 但是,对于ActiveRoleAttentionRole的处理逻辑完全相同,可以考虑合并这两个case,以减少重复代码。
    • 返回值类型的一致性:model->index(row, 0).data(role)返回的是QVariant,而直接返回false会被隐式转换为QVariant。虽然Qt的QVariant可以处理这种转换,但显式地返回QVariant(false)会更清晰。
  2. 代码性能:

    • 代码性能方面没有明显问题。model->index(row, 0).data(role)是必要的操作,无法避免。
    • 如果m_activeAppModel经常不是当前model,那么每次都会执行model == m_activeAppModel的比较,可能会有轻微的性能开销,但这种开销通常可以忽略不计。
  3. 代码安全:

    • 代码安全性方面没有明显问题。
    • 但是,如果model指针为空,那么model == m_activeAppModel的比较会导致未定义行为。虽然在这个上下文中model可能保证不为空,但为了代码的健壮性,建议添加空指针检查。
  4. 改进建议:

    • 合并ActiveRoleAttentionRole的case。
    • 显式返回QVariant(false)
    • 添加对model指针的空指针检查。

改进后的代码示例:

case TaskManager::ActiveRole:
case TaskManager::AttentionRole: {
    if (!model) {
        return QVariant(false);
    }
    if (model == m_activeAppModel) {
        return model->index(row, 0).data(role);
    }
    return QVariant(false);
}
  1. 其他考虑:

    • 如果ActiveRoleAttentionRolem_activeAppModel中的行为与其他model中的行为确实不同,那么当前的实现是合理的。
    • 如果这两个角色在所有model中的行为都应该相同,那么可以考虑移除model == m_activeAppModel的检查,直接返回model->index(row, 0).data(role)
  2. 最终建议:

    • 如果这两个角色在所有model中的行为确实不同,那么使用改进后的代码示例。
    • 如果这两个角色在所有model中的行为应该相同,那么简化为:
      case TaskManager::ActiveRole:
      case TaskManager::AttentionRole:
          return model ? model->index(row, 0).data(role) : QVariant(false);
    • 根据实际业务逻辑选择最适合的方案。

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 27, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds explicit handling of TaskManager::ActiveRole and TaskManager::AttentionRole in DockGlobalElementModel::data() so that active/attention state is correctly delegated to the active app model and returns false for non-app models, fixing incorrect dock icon active/attention visuals.

Sequence diagram for ActiveRole and AttentionRole data retrieval

sequenceDiagram
    actor User
    participant QMLBinding
    participant DockGlobalElementModel
    participant ActiveAppModel

    User->>QMLBinding: triggers icon state update
    QMLBinding->>DockGlobalElementModel: data(index, ActiveRole)
    DockGlobalElementModel->>DockGlobalElementModel: determine model and row
    alt model is m_activeAppModel
        DockGlobalElementModel->>ActiveAppModel: index(row, 0)
        ActiveAppModel-->>DockGlobalElementModel: QModelIndex
        DockGlobalElementModel->>ActiveAppModel: data(ActiveRole)
        ActiveAppModel-->>DockGlobalElementModel: QVariant(bool)
        DockGlobalElementModel-->>QMLBinding: QVariant(bool)
    else model is not m_activeAppModel
        DockGlobalElementModel-->>QMLBinding: QVariant(false)
    end
    QMLBinding-->>User: icon reflects correct active state

    User->>QMLBinding: attention state change
    QMLBinding->>DockGlobalElementModel: data(index, AttentionRole)
    DockGlobalElementModel->>DockGlobalElementModel: determine model and row
    alt model is m_activeAppModel
        DockGlobalElementModel->>ActiveAppModel: index(row, 0)
        ActiveAppModel-->>DockGlobalElementModel: QModelIndex
        DockGlobalElementModel->>ActiveAppModel: data(AttentionRole)
        ActiveAppModel-->>DockGlobalElementModel: QVariant(bool)
        DockGlobalElementModel-->>QMLBinding: QVariant(bool)
    else model is not m_activeAppModel
        DockGlobalElementModel-->>QMLBinding: QVariant(false)
    end
    QMLBinding-->>User: icon shows correct attention feedback
Loading

Class diagram for updated DockGlobalElementModel data handling

classDiagram
    class DockGlobalElementModel {
        +QVariant data(QModelIndex index, int role)
        -QAbstractItemModel* m_activeAppModel
        -QAbstractItemModel* model
    }

    class QAbstractItemModel {
        +QModelIndex index(int row, int column)
        +QVariant data(int role)
    }

    class TaskManagerRoles {
        <<enumeration>>
        +int WindowsRole
        +int ActiveRole
        +int AttentionRole
        +int MenusRole
    }

    DockGlobalElementModel --> QAbstractItemModel : uses
    DockGlobalElementModel --> TaskManagerRoles : uses

    class DataFlowForActiveAndAttentionRoles {
        +QVariant handleActiveRole(QModelIndex index)
        +QVariant handleAttentionRole(QModelIndex index)
        +QVariant delegateToActiveAppModel(QModelIndex index, int role)
        +QVariant returnFalseForNonAppModels()
    }

    DockGlobalElementModel ..> DataFlowForActiveAndAttentionRoles : implements
Loading

File-Level Changes

Change Details Files
Handle ActiveRole and AttentionRole explicitly in DockGlobalElementModel::data() to fix active/attention state reporting.
  • Added a switch-case branch for TaskManager::ActiveRole and TaskManager::AttentionRole in the model data() method.
  • When the underlying model for the row is the active app model, delegate the requested role directly to the underlying model index.
  • Return false for these roles when the underlying model is not the active app model, avoiding default fall-through that produced undefined/invalid QVariant values.
panels/dock/taskmanager/dockglobalelementmodel.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
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 left some high level feedback:

  • Consider adding a brief comment above the ActiveRole/AttentionRole branch to explain why false is returned instead of an invalid QVariant (to force QML property updates), since this is non-obvious behavior that future maintainers might otherwise “fix” back.
  • If ActiveRole/AttentionRole are conceptually "not applicable" for non-active models rather than explicitly false, it might be worth clarifying this choice (e.g., via naming or comments) so consumers don’t confuse the two states.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider adding a brief comment above the `ActiveRole`/`AttentionRole` branch to explain why `false` is returned instead of an invalid `QVariant` (to force QML property updates), since this is non-obvious behavior that future maintainers might otherwise “fix” back.
- If `ActiveRole`/`AttentionRole` are conceptually "not applicable" for non-active models rather than explicitly `false`, it might be worth clarifying this choice (e.g., via naming or comments) so consumers don’t confuse the two states.

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

@wjyrich
Copy link
Copy Markdown
Contributor Author

wjyrich commented Apr 27, 2026

/forcemerge

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented Apr 27, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 0befd40 into linuxdeepin:master Apr 27, 2026
10 of 12 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