-
Notifications
You must be signed in to change notification settings - Fork 60
feat: add left edge click signal for dock panel #1439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideAdds a left-edge click interaction to the dock panel by introducing a new DockPanel signal and a QML MouseArea that computes and emits the minimum dockOrder among applet items when the left margin is clicked. Sequence diagram for left edge click handling in dock panelsequenceDiagram
actor User
participant LeftMarginMouseArea
participant AppletModel as Applet_appletItems
participant Panel
participant DockPanel
User->>LeftMarginMouseArea: Click on left margin
LeftMarginMouseArea->>LeftMarginMouseArea: Iterate items to find min dockOrder
loop For each applet item
LeftMarginMouseArea->>AppletModel: index(i, 0)
AppletModel-->>LeftMarginMouseArea: itemData
LeftMarginMouseArea->>LeftMarginMouseArea: Update minOrder if itemData.dockOrder < minOrder
end
LeftMarginMouseArea->>Panel: leftEdgeClicked(minOrder)
Panel->>DockPanel: leftEdgeClicked(QString itemName)
DockPanel-->>Panel: Handle edge interaction
Updated class diagram for DockPanel with leftEdgeClicked slotclassDiagram
class DockPanel {
+void dockScreenChanged(QScreen* screen)
+void screenNameChanged()
+void requestClosePopup()
+void leftEdgeClicked(QString itemName)
+void devicePixelRatioChanged(qreal ratio)
+void lockedChanged(bool locked)
}
class Panel {
+void leftEdgeClicked(int minDockOrder)
}
class LeftMarginMouseArea {
+int width
+int height
+void onClicked()
}
class Applet_appletItems {
+int rowCount()
+QModelIndex index(int row,int column)
+QVariant data(QModelIndex modelIndex,int role)
}
LeftMarginMouseArea --> Applet_appletItems : reads_dockOrder
LeftMarginMouseArea --> Panel : emits_leftEdgeClicked
Panel --> DockPanel : invokes_leftEdgeClicked
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 2 issues, and left some high level feedback:
- The new
leftEdgeClickedslot is declared asvoid leftEdgeClicked(const QString &itemName);in C++, but QML is invokingPanel.leftEdgeClicked(minOrder)with a numericdockOrder, so the parameter type and semantics should be aligned (e.g., use an integer/variant or pass the actual item name). - When there are no applet items,
minOrderremainsNumber.MAX_VALUEand is still emitted; consider explicitly handling the empty case (e.g., not emitting or emitting a sentinel/null value) to avoid confusing downstream logic. - The
MouseAreawidth/height are bound togridLayout.columnSpacing/rowSpacing, which may be 0 or very small in some layouts; it might be clearer and more robust to use an explicit, fixed edge thickness or a value derived from a known margin rather than spacing.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `leftEdgeClicked` slot is declared as `void leftEdgeClicked(const QString &itemName);` in C++, but QML is invoking `Panel.leftEdgeClicked(minOrder)` with a numeric `dockOrder`, so the parameter type and semantics should be aligned (e.g., use an integer/variant or pass the actual item name).
- When there are no applet items, `minOrder` remains `Number.MAX_VALUE` and is still emitted; consider explicitly handling the empty case (e.g., not emitting or emitting a sentinel/null value) to avoid confusing downstream logic.
- The `MouseArea` width/height are bound to `gridLayout.columnSpacing`/`rowSpacing`, which may be 0 or very small in some layouts; it might be clearer and more robust to use an explicit, fixed edge thickness or a value derived from a known margin rather than spacing.
## Individual Comments
### Comment 1
<location> `panels/dock/dockpanel.h:130` </location>
<code_context>
void dockScreenChanged(QScreen *screen);
void screenNameChanged();
void requestClosePopup();
+ void leftEdgeClicked(const QString &itemName);
void devicePixelRatioChanged(qreal ratio);
void lockedChanged(bool locked);
</code_context>
<issue_to_address>
**issue (bug_risk):** The `leftEdgeClicked` parameter type/name does not match what QML is passing (`dockOrder` numeric value).
In QML, `Panel.leftEdgeClicked(minOrder)` is invoked with a numeric `dockOrder`, but this slot takes a `QString &itemName`, implying a name rather than an order and relying on implicit conversion. Please change the signature to use the actual numeric type of `dockOrder` (e.g. `void leftEdgeClicked(int dockOrder);`) and rename the parameter to match, to keep the semantics clear and type-safe.
</issue_to_address>
### Comment 2
<location> `panels/dock/package/main.qml:458-467` </location>
<code_context>
+ height: useColumnLayout ? gridLayout.rowSpacing : parent.height
+ anchors.left: parent.left
+ anchors.top: parent.top
+ onClicked: {
+ let minOrder = Number.MAX_VALUE
+
+ for (let i = 0; i < Applet.appletItems.rowCount(); i++) {
+ let itemData = Applet.appletItems.data(Applet.appletItems.index(i, 0), Qt.UserRole + 1)
+ if (itemData && itemData.dockOrder < minOrder) {
+ minOrder = itemData.dockOrder
+ }
+ }
+ Panel.leftEdgeClicked(minOrder)
+ }
+ }
</code_context>
<issue_to_address>
**issue:** No handling for the case where there are no applet items, so `minOrder` remains `Number.MAX_VALUE`.
If `Applet.appletItems.rowCount()` is zero, `minOrder` stays `Number.MAX_VALUE` and is passed to `Panel.leftEdgeClicked`, which may misbehave. Either return early when `rowCount() === 0`, or pass a sentinel value (e.g. `-1`/`null`) and handle that explicitly in the C++ slot.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
82ea74e to
ea6d21b
Compare
1. Added leftEdgeClicked signal in DockPanel to handle clicks on the left margin area 2. Implemented MouseArea in QML to capture clicks on the left margin of the dock 3. The signal emits the minimum dockOrder value among all applet items when clicked 4. This allows special handling for edge interactions in the dock panel Log: Added left edge click functionality for dock panel Influence: 1. Test clicking on the left margin area of the dock panel 2. Verify that leftEdgeClicked signal is emitted with correct dockOrder value 3. Check that the signal works with different numbers of applet items 4. Test with various dock configurations and orientations 5. Verify that the click area doesn't interfere with existing applet functionality feat: 为任务栏面板添加左侧边缘点击信号 1. 在DockPanel中添加leftEdgeClicked信号以处理左侧边距区域的点击 2. 在QML中实现MouseArea来捕获任务栏左侧边距的点击 3. 这使得任务栏面板能够特殊处理边缘交互 Log: 新增任务栏面板左侧边缘点击功能 Influence: 1. 测试点击任务栏面板的左侧边距区域 2. 验证leftEdgeClicked信号是否以正确的dockOrder值发射 3. 检查信号在不同数量的小程序项下的工作情况 4. 使用不同的任务栏配置和方向进行测试 5. 验证点击区域不会干扰现有小程序功能 PMS: BUG-345931
ea6d21b to
e645441
Compare
deepin pr auto review这段代码主要实现了一个功能:点击 Dock(任务栏)左侧的空白区域(边距区域)时,计算当前 Dock 上所有小部件中最小的 以下是对该代码的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的改进建议: 1. 语法与逻辑审查
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
5. 综合改进代码示例针对上述问题,建议的优化代码如下: // 修正注释错别字
// 此处为边距区域的点击事件特殊处理
MouseArea {
id: leftMarginArea
// 建议确保此区域不会意外遮挡可交互元素,必要时可设置 z 属性
width: useColumnLayout ? parent.width : gridLayout.columnSpacing
height: useColumnLayout ? gridLayout.rowSpacing : parent.height
anchors.left: parent.left
anchors.top: parent.top
onClicked: {
let minOrder = Number.MAX_VALUE
const count = Applet.appletItems.rowCount()
// 性能优化:如果列表为空,直接处理
if (count === 0) {
console.warn("No applets found when clicking left edge.")
// 根据业务逻辑,可能需要传递特定值(如 -1 或 "0")或不调用
// Panel.leftEdgeClicked("-1")
return
}
for (let i = 0; i < count; i++) {
// 建议将 Qt.UserRole + 1 替换为有意义的常量,例如 AppletModel.DockOrderRole
let itemData = Applet.appletItems.data(Applet.appletItems.index(i, 0), Qt.UserRole + 1)
// 增加数据有效性检查
if (itemData && typeof itemData.dockOrder === 'number') {
if (itemData.dockOrder < minOrder) {
minOrder = itemData.dockOrder
}
}
}
// 安全转换:如果 minOrder 未更新(即没有有效数据),进行兜底处理
if (minOrder === Number.MAX_VALUE) {
minOrder = 0 // 或者其他表示无效的默认值
}
// 确保传递的是字符串或数字,与 C++ 签名匹配
Panel.leftEdgeClicked(minOrder.toString())
}
}总结这段代码实现了基本功能,但在边界情况处理(空列表、无效数据)和类型安全方面存在改进空间。同时,修正注释错别字和优化硬编码常量能提升代码质量。如果 Dock 图标数量巨大,建议考虑在 C++ 端缓存最小值以避免 QML 中的遍历开销。 |
|
[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 |
Log: Added left edge click functionality for dock panel
Influence:
feat: 为任务栏面板添加左侧边缘点击信号
Log: 新增任务栏面板左侧边缘点击功能
Influence:
PMS: BUG-345931
Summary by Sourcery
Add support for handling left-edge click interactions on the dock panel and exposing them via a new signal.
New Features: