fix: improve OSD panel timeout behavior with modifier key #1306
Conversation
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR refactors the OSD panel’s timeout handling by introducing a doneSetting slot that checks for the Meta modifier key before hiding the OSD, restarting or deferring the hide timer as appropriate to prevent premature disappearance during key holds. Sequence diagram for OSD panel timeout with modifier keysequenceDiagram
participant User as actor User
participant OsdPanel
participant QTimer
participant qApp
User->>OsdPanel: Triggers OSD display
OsdPanel->>QTimer: Start m_osdTimer
QTimer-->>OsdPanel: timeout
OsdPanel->>qApp: queryKeyboardModifiers()
alt MetaModifier is pressed
OsdPanel->>QTimer: Restart m_osdTimer
else MetaModifier not pressed
OsdPanel->>QTimer: Create hideTimer
QTimer-->>OsdPanel: hideTimer timeout
OsdPanel->>OsdPanel: hideOsd()
end
Updated class diagram for OsdPanel timeout logicclassDiagram
class OsdPanel {
+void ShowOSD(QString text)
+void hideOsd()
+void doneSetting()
-QTimer* m_osdTimer
-int m_interval
+void showOsd()
+void setVisible(bool visible)
}
OsdPanel --> QTimer : uses
OsdPanel --> qApp : queries keyboard modifiers
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `panels/notification/osd/osdpanel.cpp:79-81` </location>
<code_context>
+void OsdPanel::doneSetting()
+{
+ if (qApp->queryKeyboardModifiers().testFlag(Qt::MetaModifier)) {
+ return m_osdTimer->start();
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Returning m_osdTimer->start() may be misleading since QTimer::start() returns void.
Use 'm_osdTimer->start(); return;' instead to avoid confusion, since QTimer::start() does not return a value.
```suggestion
if (qApp->queryKeyboardModifiers().testFlag(Qt::MetaModifier)) {
m_osdTimer->start();
return;
}
```
</issue_to_address>
### Comment 2
<location> `panels/notification/osd/osdpanel.cpp:84-86` </location>
<code_context>
+ }
+
+ //松下快捷键后，能够延时消失
+ QTimer *hideTimer = new QTimer(this);
+ hideTimer->setInterval(m_osdTimer->interval() - m_osdTimer->remainingTime());
+ hideTimer->setSingleShot(true);
+ QObject::connect(hideTimer, &QTimer::timeout, this, &OsdPanel::hideOsd);
+ hideTimer->start();
</code_context>
<issue_to_address>
**issue (bug_risk):** Creating a new QTimer on each call may lead to resource leaks if not properly managed.
Repeatedly creating QTimer instances without deletion can increase memory usage. Consider reusing a member QTimer or deleting each timer after it finishes.
</issue_to_address>
### Comment 3
<location> `panels/notification/osd/osdpanel.cpp:85` </location>
<code_context>
+
+ //松下快捷键后，能够延时消失
+ QTimer *hideTimer = new QTimer(this);
+ hideTimer->setInterval(m_osdTimer->interval() - m_osdTimer->remainingTime());
+ hideTimer->setSingleShot(true);
+ QObject::connect(hideTimer, &QTimer::timeout, this, &OsdPanel::hideOsd);
</code_context>
<issue_to_address>
**issue (bug_risk):** Subtracting remainingTime from interval may result in a negative value.
If remainingTime() is greater than interval(), hideTimer will have a negative interval, which may cause undefined behavior. Please ensure the interval is always non-negative.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
wjyrich
force-pushed
the
fix-bug-294165
branch
from
81f8820 to
c453ecd
Compare
1. Changed timer connection from hideOsd to doneSetting to handle modifier key detection 2. Added doneSetting method that checks for MetaModifier key state 3. When Meta key is pressed, restarts timer instead of hiding OSD immediately 4. Creates additional hide timer when Meta key is not pressed to ensure proper timeout after key release 5. This prevents OSD from disappearing while user is holding modifier keys for shortcuts fix: 改进 OSD 面板超时行为以支持修饰键 1. 将定时器连接从 hideOsd 改为 doneSetting 以处理修饰键检测 2. 添加 doneSetting 方法检查 MetaModifier 键状态 3. 当 Meta 键按下时，重新启动定时器而不是立即隐藏 OSD 4. 当 Meta 键未按下时，创建额外的隐藏定时器确保按键释放后正确超时 5. 防止用户在按住修饰键进行快捷操作时 OSD 意外消失 PMS: BUG-294169 BUG-294165
wjyrich
force-pushed
the
fix-bug-294165
branch
from
c453ecd to
5df4b55
Compare
deepin pr auto review我来帮你审查这段代码的改动：
a) 逻辑问题： int delay = m_osdTimer->interval() - m_osdTimer->remainingTime();
if (delay < 0) delay = 0;这里的逻辑可能有问题。当定时器已经超时（remainingTime() 返回 0）时，delay 会等于 interval，这可能不是预期行为。建议改为： int remaining = m_osdTimer->remainingTime();
if (remaining <= 0) {
hideOsd();
return;
}
QTimer::singleShot(remaining, this, [this](){ hideOsd(); });b) 命名建议：
c) 代码健壮性：
void OsdPanel::handleTimeout()
{
if (!m_osdTimer || !m_osdTimer->isActive()) {
return;
}
// ... 其余代码
}d) 性能考虑：
e) 安全性：
f) 代码结构：
/**
* @brief 处理 OSD 显示定时器超时事件
*
* 当用户按住 Meta 键时，重新启动定时器
* 否则在适当延迟后隐藏 OSD
*/
void handleTimeout();
这些改进可以让代码更加健壮、可维护，并且更符合 Qt 的编程规范。 |
18202781743
approved these changes
Oct 21, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
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.
fix: 改进 OSD 面板超时行为以支持修饰键
PMS: BUG-294169 BUG-294165
Summary by Sourcery
Improve OSD panel timeout behavior by detecting the Meta modifier key and postponing the hide action until the key is released
Bug Fixes:
Enhancements: