Skip to content

fix: restore checkbox binding after menu item triggered#1433

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Ivy233:fix-dock-menu-checkbox-state
Feb 5, 2026
Merged

fix: restore checkbox binding after menu item triggered#1433
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Ivy233:fix-dock-menu-checkbox-state

Conversation

@Ivy233
Copy link
Contributor

@Ivy233 Ivy233 commented Feb 5, 2026

  1. Removed conditional check in onTriggered handler that prevented property updates when value was already set
  2. Added explicit Qt.binding restoration for checked property after setting the value
  3. This ensures checkbox state remains synchronized with Applet property even after user interaction
  4. Fixes issue where checkbox state could become desynchronized from actual property value after menu item is triggered

Log: Fixed dock menu checkbox state synchronization issue

Influence:

  1. Test dock menu items with checkbox to ensure state updates correctly
  2. Verify checkbox reflects actual property value after clicking
  3. Test multiple clicks on same menu item to ensure binding persists
  4. Verify checkbox state updates when property changes from other sources
  5. Test with different dock menu options (position, display mode, etc.)

fix: 修复菜单项触发后复选框绑定问题

  1. 移除了 onTriggered 处理器中的条件检查,该检查会在值已设置时阻止属性更新
  2. 在设置值后添加了显式的 Qt.binding 恢复,用于 checked 属性
  3. 确保复选框状态在用户交互后仍与 Applet 属性保持同步
  4. 修复了菜单项触发后复选框状态可能与实际属性值不同步的问题

Log: 修复了任务栏菜单复选框状态同步问题

Influence:

  1. 测试带复选框的任务栏菜单项,确保状态正确更新
  2. 验证点击后复选框反映实际属性值
  3. 测试多次点击同一菜单项,确保绑定持续有效
  4. 验证属性从其他来源更改时复选框状态更新
  5. 测试不同的任务栏菜单选项(位置、显示模式等)

PMS: BUG-349947

Summary by Sourcery

Bug Fixes:

  • Fix desynchronization where dock menu checkbox state could stop reflecting the corresponding Applet property value after a menu item is clicked.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 5, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Restores and maintains the binding between dock menu checkbox states and their corresponding Applet properties by always updating the property on trigger and re-establishing a checked binding to the property/value comparison.

Sequence diagram for dock menu checkbox trigger and binding restoration

sequenceDiagram
    actor User
    participant DockMenuItem
    participant Applet

    User->>DockMenuItem: click
    activate DockMenuItem
    DockMenuItem->>Applet: set prop = value
    Applet-->>DockMenuItem: prop updated
    DockMenuItem->>DockMenuItem: set checked binding
    deactivate DockMenuItem

    User->>Applet: change prop from other source
    activate Applet
    Applet-->>DockMenuItem: prop changed
    DockMenuItem->>DockMenuItem: checked recomputed from Applet[prop] === value
    deactivate Applet
Loading

State diagram for dock menu checkbox binding lifecycle

stateDiagram-v2
    [*] --> BoundToApplet

    BoundToApplet: checked bound to Applet[prop] === value

    BoundToApplet --> UserTriggered: user clicks menu item

    UserTriggered: onTriggered handler runs

    UserTriggered --> PropertyUpdated: Applet[prop] set to value

    PropertyUpdated: Applet property updated

    PropertyUpdated --> BindingRestored: checked binding recreated to Applet[prop] === value

    BindingRestored: checked bound to Applet[prop] === value

    BindingRestored --> BoundToApplet
Loading

File-Level Changes

Change Details Files
Ensure menu checkbox remains bound to Applet property after a menu item is triggered
  • Remove the conditional guard that skipped Applet property assignment when the value was unchanged
  • Always assign Applet[prop] to value in the onTriggered handler
  • Re-establish a Qt.binding for the checked property after the trigger so it derives from Applet[prop] === value
  • Keep the initial checked expression based on Applet[prop] === value for initial binding
panels/dock/package/main.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 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.

1. Removed conditional check in onTriggered handler that prevented
property updates when value was already set
2. Added explicit Qt.binding restoration for checked property after
setting the value
3. This ensures checkbox state remains synchronized with Applet
property even after user interaction
4. Fixes issue where checkbox state could become desynchronized from
actual property value after menu item is triggered

Log: Fixed dock menu checkbox state synchronization issue

Influence:
1. Test dock menu items with checkbox to ensure state updates correctly
2. Verify checkbox reflects actual property value after clicking
3. Test multiple clicks on same menu item to ensure binding persists
4. Verify checkbox state updates when property changes from other sources
5. Test with different dock menu options (position, display mode, etc.)

fix: 修复菜单项触发后复选框绑定问题

1. 移除了 onTriggered 处理器中的条件检查,该检查会在值已设置时阻止属性更新
2. 在设置值后添加了显式的 Qt.binding 恢复,用于 checked 属性
3. 确保复选框状态在用户交互后仍与 Applet 属性保持同步
4. 修复了菜单项触发后复选框状态可能与实际属性值不同步的问题

Log: 修复了任务栏菜单复选框状态同步问题

Influence:
1. 测试带复选框的任务栏菜单项,确保状态正确更新
2. 验证点击后复选框反映实际属性值
3. 测试多次点击同一菜单项,确保绑定持续有效
4. 验证属性从其他来源更改时复选框状态更新
5. 测试不同的任务栏菜单选项(位置、显示模式等)

PMS: BUG-349947
@Ivy233 Ivy233 force-pushed the fix-dock-menu-checkbox-state branch from 5f43fa4 to 88fc663 Compare February 5, 2026 02:03
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码是对一个QML组件(可能是菜单项或按钮)的修改,主要涉及属性绑定和状态更新的逻辑。以下是对该代码变更的详细审查和改进意见:

1. 代码逻辑审查

变更分析:

  • 原代码:在 onTriggered 处理函数中,首先检查 Applet[prop] 是否等于 value,如果不等才进行赋值。这是一个防御性编程的写法,避免了不必要的赋值操作。
  • 新代码:移除了条件判断,直接执行 Applet[prop] = value,并且添加了一个绑定来更新 checked 属性。

潜在问题:

  • 性能问题:移除条件判断意味着每次触发都会执行赋值操作,即使值没有变化。这可能导致不必要的属性通知信号发射,进而触发绑定的重新计算。
  • 绑定覆盖风险:在 onTriggered 中使用 Qt.binding() 会重新建立绑定。如果 checked 属性原本已经是一个绑定,这种写法是安全的;但如果 checked 属性在其他地方被赋值(破坏了绑定),这里会重新建立绑定,这可能是预期的行为,但也可能掩盖了其他地方的逻辑问题。

2. 代码质量与最佳实践

改进意见:

  • 保留条件判断:原代码的条件判断 if (Applet[prop] !== value) 是一种良好的实践,可以避免不必要的属性更新和信号发射。建议保留。
  • 绑定的一致性checked 属性已经在声明时通过 checked: Applet[prop] === value 建立了绑定。在 onTriggered 中重新建立绑定是冗余的,除非有特殊需求(例如动态切换绑定逻辑)。

3. 代码安全

  • 属性访问安全性Applet[prop] 的动态属性访问方式是灵活的,但需要确保 prop 是一个有效的属性名,否则会导致运行时错误。建议在调用前验证 prop 的有效性(如果可能)。
  • 绑定循环风险:如果 Applet[prop] 的赋值会间接影响 valueprop,可能会导致绑定循环。需要确保 Applet 的属性更新逻辑是单向的。

4. 性能优化

  • 减少不必要的赋值:保留原代码的条件判断可以避免不必要的赋值操作,从而减少属性通知信号的发射和绑定的重新计算。
  • 避免重复绑定:如果 checked 的绑定逻辑在声明时已经定义,无需在 onTriggered 中重复建立绑定。

5. 改进后的代码建议

MenuItem {
    id: menuItem
    text: name

    onTriggered: {
        // 保留条件判断,避免不必要的赋值
        if (Applet[prop] !== value) {
            Applet[prop] = value
        }
    }
    // 保持原有的绑定逻辑
    checked: Applet[prop] === value
}

6. 进一步优化(如果需要动态绑定)

如果确实需要在运行时动态更新绑定逻辑(例如 propvalue 可能变化),可以如下优化:

MenuItem {
    id: menuItem
    text: name

    onTriggered: {
        if (Applet[prop] !== value) {
            Applet[prop] = value
        }
    }
    // 使用函数返回绑定表达式,确保动态更新
    checked: {
        return Applet[prop] === value
    }
}

总结

原代码的变更试图通过动态绑定更新 checked 属性,但这种做法可能是冗余的,且可能引入不必要的性能开销。建议保留原代码的条件判断,并确保 checked 属性的绑定逻辑在声明时正确定义。如果确实需要动态绑定,应明确其必要性,并避免重复绑定。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, Ivy233

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

@Ivy233
Copy link
Contributor Author

Ivy233 commented Feb 5, 2026

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Feb 5, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit e1bfe70 into linuxdeepin:master Feb 5, 2026
10 of 11 checks passed
@Ivy233 Ivy233 deleted the fix-dock-menu-checkbox-state branch February 5, 2026 02:26
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