Fix: Abnormal interface display when clicking any hotkey#2972
Open
JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
Open
Fix: Abnormal interface display when clicking any hotkey#2972JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JWWTSL 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 |
Reviewer's GuideKeeps the Control Center main window visually active while capturing global shortcut keys by introducing a forceActiveAppearance flag on the main window and toggling it from the shortcut editing QML components during key capture lifecycle events. Sequence diagram for maintaining active appearance during shortcut key capture startsequenceDiagram
actor User
participant ShortcutsView
participant ShortcutSettingDialog
participant dccData
participant DccApp
participant MainWindow
participant RequestActivateTimer
User->>ShortcutsView: Click_edit_global_shortcut
ShortcutsView->>dccData: updateKey(id,type)
ShortcutsView->>DccApp: mainWindow()
DccApp-->>ShortcutsView: MainWindow_reference
ShortcutsView->>MainWindow: set forceActiveAppearance = true
User->>ShortcutSettingDialog: Open_shortcut_dialog_and_click_capture
ShortcutSettingDialog->>dccData: updateKey(keyId,1)
ShortcutSettingDialog->>DccApp: mainWindow()
DccApp-->>ShortcutSettingDialog: MainWindow_reference
ShortcutSettingDialog->>MainWindow: set forceActiveAppearance = true
Note over MainWindow,RequestActivateTimer: Later, when window loses focus
MainWindow-->>MainWindow: active_changed_to_false
MainWindow->>RequestActivateTimer: start()
RequestActivateTimer-->>RequestActivateTimer: timeout
RequestActivateTimer->>MainWindow: requestActivate() when forceActiveAppearance == true
Sequence diagram for releasing active appearance after shortcut key capture endsequenceDiagram
participant dccData
participant ShortcutsView
participant ShortcutSettingDialog
participant DccApp
participant MainWindow
%% Ending key capture from shortcuts page
dccData-->>ShortcutsView: requestRestore
ShortcutsView->>DccApp: mainWindow()
DccApp-->>ShortcutsView: MainWindow_reference
ShortcutsView->>MainWindow: set forceActiveAppearance = false
ShortcutsView-->>ShortcutsView: restoreShortcutView()
dccData-->>ShortcutsView: requestClear
ShortcutsView->>DccApp: mainWindow()
DccApp-->>ShortcutsView: MainWindow_reference
ShortcutsView->>MainWindow: set forceActiveAppearance = false
ShortcutsView-->>ShortcutsView: clearShortcut()
dccData-->>ShortcutsView: keyConflicted
ShortcutsView->>DccApp: mainWindow()
DccApp-->>ShortcutsView: MainWindow_reference
ShortcutsView->>MainWindow: set forceActiveAppearance = false
dccData-->>ShortcutsView: keyDone
ShortcutsView->>DccApp: mainWindow()
DccApp-->>ShortcutsView: MainWindow_reference
ShortcutsView->>MainWindow: set forceActiveAppearance = false
%% Ending key capture from dialog
dccData-->>ShortcutSettingDialog: requestRestore
ShortcutSettingDialog->>DccApp: mainWindow()
DccApp-->>ShortcutSettingDialog: MainWindow_reference
ShortcutSettingDialog->>MainWindow: set forceActiveAppearance = false
dccData-->>ShortcutSettingDialog: keyConflicted
ShortcutSettingDialog->>DccApp: mainWindow()
DccApp-->>ShortcutSettingDialog: MainWindow_reference
ShortcutSettingDialog->>MainWindow: set forceActiveAppearance = false
dccData-->>ShortcutSettingDialog: keyDone
ShortcutSettingDialog->>DccApp: mainWindow()
DccApp-->>ShortcutSettingDialog: MainWindow_reference
ShortcutSettingDialog->>MainWindow: set forceActiveAppearance = false
Updated class diagram for main window and shortcut editing componentsclassDiagram
class DccWindow {
+string appLicense
+real currentIndex
+var sidebarPage
+bool forceActiveAppearance
+real minimumWidth
+real minimumHeight
+bool visible
+bool active
+requestActivate()
}
class RequestActivateTimer {
+int interval
+start()
+onTriggered()
}
class DccApp {
+mainWindow() DccWindow
}
class dccData {
+updateKey(id,type)
+requestRestore()
+requestClear()
+keyConflicted(oldAccels,newAccels)
+keyDone(accels)
+conflictText
+formatKeys(accels)
}
class ShortcutsView {
+int searchEditWidth
+var viewScrollbar
+var shortcutSettingsBody
+var shortcutView
+setForceActiveAppearance(value)
}
class ShortcutSettingDialog {
+string keyId
+string keyName
+string cmdName
+string keySequence
+var edit
+var conflictText
+var saveKeys
+setForceActiveAppearance(value)
}
DccWindow o-- RequestActivateTimer : contains
DccApp --> DccWindow : returns_mainWindow
ShortcutsView --> DccApp : calls_mainWindow
ShortcutSettingDialog --> DccApp : calls_mainWindow
ShortcutsView --> dccData : uses_updateKey_and_signals
ShortcutSettingDialog --> dccData : uses_updateKey_and_signals
ShortcutsView ..> DccWindow : toggles_forceActiveAppearance
ShortcutSettingDialog ..> DccWindow : toggles_forceActiveAppearance
RequestActivateTimer ..> DccWindow : calls_requestActivate_when_forceActiveAppearance
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
setForceActiveAppearancehelper is duplicated in bothShortcuts.qmlandShortcutSettingDialog.qml; consider centralizing this logic (e.g., a shared util or a common base component) to avoid duplication and keep behavior consistent. - The
requestActivateTimerinDccWindow.qmlhas norepeatflag specified, so it will fire repeatedly once started; if you only intend a single re-activation, setrepeat: falseorrunning: falsewith explicitstart()to avoid unnecessary wakeups. - You rely on several specific
dccDatasignals to resetforceActiveAppearancetofalse; review whether all possible exit/cancel/error paths from global key capture also clear this flag to avoid leaving the window permanently in a forced-active state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `setForceActiveAppearance` helper is duplicated in both `Shortcuts.qml` and `ShortcutSettingDialog.qml`; consider centralizing this logic (e.g., a shared util or a common base component) to avoid duplication and keep behavior consistent.
- The `requestActivateTimer` in `DccWindow.qml` has no `repeat` flag specified, so it will fire repeatedly once started; if you only intend a single re-activation, set `repeat: false` or `running: false` with explicit `start()` to avoid unnecessary wakeups.
- You rely on several specific `dccData` signals to reset `forceActiveAppearance` to `false`; review whether all possible exit/cancel/error paths from global key capture also clear this flag to avoid leaving the window permanently in a forced-active state.
## Individual Comments
### Comment 1
<location> `src/plugin-keyboard/qml/Shortcuts.qml:20` </location>
<code_context>
modality: Qt.WindowModal
property string keyId
+
+ function setForceActiveAppearance(value) {
+ var win = DccApp.mainWindow()
+ if (win && win.forceActiveAppearance !== undefined)
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating `setForceActiveAppearance` implementation across QML files.
This helper is also defined in `ShortcutSettingDialog.qml`. To avoid duplication and keep behavior consistent, centralize it (for example via a helper on `DccApp`, or by calling `DccApp.mainWindow().forceActiveAppearance` directly where needed) instead of reimplementing it in multiple QML components.
Suggested implementation:
```
icon: "keyboard_fn"
weight: parent.weight // 300
property int searchEditWidth: 600
property var viewScrollbar: ScrollBar {
width: 10
}
```
```
edit.keys = ""
var win = DccApp.mainWindow()
if (win && win.forceActiveAppearance !== undefined)
win.forceActiveAppearance = true
dccData.updateKey(model.id, model.type)
```
To fully remove duplication, consider applying a similar change in `ShortcutSettingDialog.qml`: either remove its local `setForceActiveAppearance` helper and call `DccApp.mainWindow().forceActiveAppearance` directly, or (preferably) introduce a single helper on `DccApp` (e.g. `DccApp.setForceActiveAppearance(value)`) and use that from all QML files.
</issue_to_address>
### Comment 2
<location> `src/dde-control-center/plugin/DccWindow.qml:18-21` </location>
<code_context>
property string appLicense: "GPL-3.0-or-later"
property real currentIndex: 1
property var sidebarPage: null
+ property bool forceActiveAppearance: false // 编辑快捷键时保持窗口激活外观
minimumWidth: 520
</code_context>
<issue_to_address>
**issue (bug_risk):** Repeatedly re-activating the window can create focus-stealing or UX issues.
When `forceActiveAppearance` is true, losing activation will always trigger a timer that re-activates the window, which can effectively steal focus when the user switches apps/workspaces. Consider limiting this to a narrow period or a single re-activation per capture so it doesn’t continuously override the user’s focus choice.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Log: When editing hotkeys, forceActiveAppearance is used to keep the Control Center main window “active” during global key capture, preventing the window from briefly turning gray due to focus loss. pms: bug-303139
JWWTSL
force-pushed
the
fix-shortcut-focus
branch
from
86be1ba to
8fc0b75
Compare
deepin pr auto reviewGit Diff 代码审查报告概述这段代码修改主要是为了在快捷键设置期间保持窗口激活外观，防止窗口在快捷键抓取过程中失去焦点。修改涉及三个文件： 语法逻辑分析
代码质量评估
代码性能考虑
代码安全问题
改进建议
总结这段代码修改实现了预期的功能，即保持窗口激活外观，但存在一些代码重复和潜在的优化空间。通过减少重复代码、优化定时器设置和添加适当的清理机制，可以提高代码质量和可维护性。建议在合并前考虑上述改进建议。 |
Contributor
|
录入快捷键的时候使用后端的SelectKeystroke接口，这个接口的grab操作会导致控制中心失焦，理论上可以直接使用Qt的接口去拿到当前用户按下的快捷键，暂时不清楚为什么要这样处理。另外快捷键（比如控制音量）也会导致短暂失焦，这些问题都需要在快捷键服务重构之间解决，所以这个问题在重构过程中一起看有没有其他更好的办法 |
|
TAG Bot New tag: 6.1.71 |
|
TAG Bot New tag: 6.1.72 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: When editing hotkeys, forceActiveAppearance is used to keep the Control Center main window “active” during global key capture, preventing the window from briefly turning gray due to focus loss.
pms: bug-303139
Summary by Sourcery
Keep the Control Center window visually active while capturing global shortcut keys to prevent abnormal interface appearance during hotkey editing.
Bug Fixes:
Enhancements: