fix: touchpad FN key only shows OSD, no double toggle - #179
Conversation
1. Remove direct DBus TPadEnable toggling from dde-shortcut-tool 2. Keep OSD display (TouchpadToggle/TouchpadOn/TouchpadOff) only 3. Actual touchpad switch is handled by the system keyevent service, avoiding double-toggle that inverted the CC switch state Log: FN touchpad shortcut no longer toggles state twice; control center switch stays in sync Influence: 1. On a laptop with touchpad, press FN touchpad hotkey, verify touchpad toggles exactly once and OSD shows correct icon 2. Check control center touchpad switch matches actual touchpad state after each FN press 3. Verify touchpad enable/disable still works from control center itself fix: 触控板FN快捷键仅显示OSD,不再重复切换状态 1. 移除 dde-shortcut-tool 中直接通过 DBus 切换 TPadEnable 的逻辑 2. 保留 OSD 显示(TouchpadToggle/TouchpadOn/TouchpadOff) 3. 实际开关由系统 keyevent 服务处理,避免双重切换导致 控制中心开关状态与实际相反 Log: 触控板FN快捷键不再重复切换状态,控制中心开关保持同步 Influence: 1. 在带触控板的笔记本上按 FN 触控板快捷键,验证触控板仅切换 一次且 OSD 图标正确 2. 每次按 FN 后检查控制中心触控板开关与实际状态一致 3. 验证从控制中心直接开关触控板仍正常工作 PMS: BUG-377717
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe touchpad shortcut controller no longer changes the touchpad state itself; it only emits the appropriate OSD, preventing the FN key path from toggling twice while leaving actual state changes to the system keyevent service and preserving Control Center synchronization. Sequence diagram for touchpad FN shortcut handlingsequenceDiagram
actor User
participant KeyEventService
participant TouchPadController
participant Touchpad
participant OSD
participant ControlCenter
User->>KeyEventService: FN touchpad shortcut
KeyEventService->>Touchpad: Toggle touchpad state
KeyEventService->>TouchPadController: toggle()
TouchPadController->>OSD: showOSD(TouchpadToggle)
Touchpad-->>ControlCenter: Updated touchpad state
ControlCenter-->>User: Synchronized switch state
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugin-qt/shortcut/tools/dde-shortcut-tool/touchpadcontroller.cpp" line_range="62-64" />
<code_context>
- }
-
- // Toggle always shows TouchpadToggle OSD (Go version logic)
+ // The shortcut only shows the OSD; the actual touchpad switch is handled
+ // elsewhere (e.g. the system keyevent service).
showOSD("TouchpadToggle");
-
return true;
</code_context>
<issue_to_address>
**issue (bug_risk):** The `touchpad toggle`, `touchpad on`, and `touchpad off` commands now return success and display an OSD without changing the touchpad state. Any caller that invokes `dde-shortcut-tool` directly rather than going through the system keyevent service therefore receives a successful no-op, despite the command names and help text promising to toggle or enable/disable the touchpad.
**Triggers:** When a user, script, control-center integration, or custom shortcut invokes these dde-shortcut-tool actions directly.
**Suggested fix:** Keep the state-changing DBus implementation for explicit tool actions, or clearly separate the OSD-only keyevent path from the public `on`, `off`, and `toggle` commands and update their contract/help text.
</issue_to_address>| // The shortcut only shows the OSD; the actual touchpad switch is handled | ||
| // elsewhere (e.g. the system keyevent service). | ||
| showOSD("TouchpadToggle"); |
There was a problem hiding this comment.
issue (bug_risk): The touchpad toggle, touchpad on, and touchpad off commands now return success and display an OSD without changing the touchpad state. Any caller that invokes dde-shortcut-tool directly rather than going through the system keyevent service therefore receives a successful no-op, despite the command names and help text promising to toggle or enable/disable the touchpad.
Triggers: When a user, script, control-center integration, or custom shortcut invokes these dde-shortcut-tool actions directly.
Suggested fix: Keep the state-changing DBus implementation for explicit tool actions, or clearly separate the OSD-only keyevent path from the public on, off, and toggle commands and update their contract/help text.
|
/test all |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, yixinshark The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto reviewAI 代码审查报告提交信息
需求描述修复触控板 FN 快捷键重复切换状态的问题:
总体评分
漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个 维度1: 语法逻辑(25分) - 得分: 25 ✓
审查内容文件:
结论
维度2: 代码质量(25分) - 得分: 25 ✓
审查内容
结论
维度3: 代码性能(20分) - 得分: 20 ✓
审查内容
结论
维度4: 代码安全(30分) - 得分: 30 ✓
审查内容
结论
审查结论本次 PR 的目的是修复触控板 FN 快捷键重复切换状态的问题。代码变更完全符合这一目的:
审查通过,建议合并。 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Log: FN touchpad shortcut no longer toggles state twice; control center switch stays in sync
Influence:
fix: 触控板FN快捷键仅显示OSD,不再重复切换状态
Log: 触控板FN快捷键不再重复切换状态,控制中心开关保持同步
Influence:
PMS: BUG-377717
Summary by Sourcery
Prevent duplicate touchpad state changes by limiting shortcut handling to OSD notifications and delegating actual toggling to the system keyevent service.
Bug Fixes:
Enhancements: