fix: prevent duplicate OSD actions for same type#1113
Conversation
18202781743
requested review from
BLumia,
caixr23,
fly602,
mhduiy,
robertkill and
yixinshark
Reviewer's GuideThis pull request prevents duplicate On-Screen Display (OSD) actions for rapidly received notifications of the same type. It introduces state tracking in the C++ Sequence diagram for QML OSD action checksequenceDiagram
participant OsdPanel as OsdPanel (C++)
participant QMLHandler as QML Handler (e.g., displaymode)
OsdPanel->>QMLHandler: osdTypeChanged(newType) signal
activate QMLHandler
QMLHandler->>QMLHandler: update(newType) function called
QMLHandler->>OsdPanel: Read lastOsdType property
activate OsdPanel
OsdPanel-->>QMLHandler: Return lastOsdTypeValue
deactivate OsdPanel
QMLHandler->>QMLHandler: Check if lastOsdTypeValue == newType
alt Condition is true (lastOsdTypeValue == newType)
QMLHandler->>QMLHandler: Perform OSD Action (e.g., Applet.next())
else Condition is false
QMLHandler->>QMLHandler: Skip OSD Action
end
deactivate QMLHandler
Class diagram for OsdPanel changesclassDiagram
class OsdPanel {
<<C++ Class>>
-QString m_lastOsdType
+QString lastOsdType() const
#void hideOsd()
#void showOsd()
#void updateLastOsdType(const QString &osdType)
+Q_PROPERTY(QString lastOsdType READ lastOsdType CONSTANT)
}
note for OsdPanel "New members:\n- m_lastOsdType\n- lastOsdType()\n- updateLastOsdType()\n- Q_PROPERTY lastOsdType\nModified methods:\n- hideOsd()\n- showOsd()"
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 @18202781743 - I've reviewed your changes - here's some feedback:
- The logic triggers actions (like cycling modes) only when the current OSD type matches the previous one; consider if the action should trigger on the first notification instead.
- The
lastOsdTypeQ_PROPERTY is markedCONSTANTbut its underlying value changes; consider adding aNOTIFYsignal or removingCONSTANT.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
force-pushed
the
master
branch
2 times, most recently
from
41c8a9d to
b8b76aa
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy 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 |
1. Added lastOsdType tracking in OsdPanel to store the most recent OSD type 2. Modified displaymode, kblayout and windoweffect OSD handlers to only perform actions if the current OSD type matches the last one 3. This prevents duplicate actions when multiple OSD events of same type are triggered in quick succession 4. Added updateLastOsdType method to track and clear the last OSD type when hiding fix: 防止相同类型OSD的重复操作 1. 在OsdPanel中添加lastOsdType跟踪最近显示的OSD类型 2. 修改了displaymode、kblayout和windoweffect的OSD处理逻辑，仅在当前OSD类 型与最后一次相同时才执行操作 3. 防止在快速连续触发相同类型OSD事件时重复执行操作 4. 添加updateLastOsdType方法来跟踪并在隐藏时清除最后OSD类型
deepin pr auto review代码审查意见：
以上是针对代码审查的几点意见，希望能够对你有所帮助。 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
3 participants
trigger actions when current type matches last type
notifications of same type in quick succession
state
fix: 防止相同类型OSD的重复操作
与上次类型匹配时才触发操作
pms: BUG-304139
Summary by Sourcery
Prevent duplicate OSD actions for the same type by tracking the last OSD type and adding logic to only trigger actions when the current OSD type matches the previous one
New Features:
Bug Fixes:
Enhancements: