chore: bump version to 2.0.16 #1324
Closed
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
|
TAG Bot TAG: 2.0.16 |
Reviewer's GuideThis PR bumps the version to 2.0.16 and updates the changelog, while adding debounced, threshold-based tooltip logic for dragging over the trash icon in QML, and optimizing dock position updates with early exits and logging in the C++ positioners. Sequence diagram for drag-and-drop tooltip logic over trash iconsequenceDiagram
participant User as actor User
participant AppItem
participant DropArea
participant dragToolTip
participant tooltipDelayTimer
User->>DropArea: Drag enters trash icon
DropArea->>DropArea: Check debounce threshold
alt Not too soon after exit
DropArea->>DropArea: Set isDragOver, shouldShowTooltip
DropArea->>tooltipDelayTimer: Start timer
tooltipDelayTimer->>DropArea: Timer triggers
DropArea->>dragToolTip: Open tooltip
DropArea->>DropArea: Set tooltipVisible
else Too soon after exit
DropArea->>DropArea: Ignore event
end
User->>DropArea: Drag moves
DropArea->>DropArea: Check movement threshold
alt Movement > threshold
DropArea->>dragToolTip: Update tooltip position
else Movement <= threshold
DropArea->>DropArea: Ignore position update
end
User->>DropArea: Drag exits trash icon
DropArea->>DropArea: Check debounce threshold
alt Not too soon after enter
DropArea->>dragToolTip: Close tooltip
DropArea->>DropArea: Reset tooltip state
else Too soon after enter
DropArea->>DropArea: Ignore event
end
User->>DropArea: Drop files on trash icon
DropArea->>dragToolTip: Close tooltip
DropArea->>DropArea: Reset tooltip state
DropArea->>AppItem: dropFilesOnItem
Class diagram for updated DropArea tooltip logic in AppItem.qmlclassDiagram
class DropArea {
+bool isDragOver
+var lastDragPosition
+int dragStableThreshold
+bool shouldShowTooltip
+int lastEnterTime
+int lastExitTime
+int debounceThreshold
+bool tooltipVisible
+onEntered(drag)
+onExited(drag)
+onPositionChanged(drag)
+onDropped(drop)
}
class Timer {
+interval
+onTriggered()
}
DropArea "1" -- "1" Timer : tooltipDelayTimer
DropArea "1" -- "1" dragToolTip
DropArea "1" -- "1" AppItem : root
Class diagram for DockPositioner and DockPanelPositioner update logicclassDiagram
class DockPositioner {
+int m_x
+int m_y
+QRect m_bounding
+QTimer m_positionTimer
+void update()
+void setY(int y)
}
class DockPanelPositioner {
+QRect m_bounding
+int m_horizontalOffset
+int m_vertialOffset
+void updatePosition()
}
DockPanelPositioner --|> DockPositioner
Class diagram for PanelToolTip open/close loggingclassDiagram
class PanelToolTip {
+string text
+function open()
+function close()
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这段代码进行审查，从语法逻辑、代码质量、性能和安全几个方面提出改进建议：
具体改进建议：
void DockPositioner::calculatePosition(int& x, int& y) const {
switch (dockPosition()) {
case dock::Top:
x = m_bounding.x();
y = m_bounding.y();
break;
case dock::Right:
x = m_bounding.x() - m_bounding.width();
y = m_bounding.y();
break;
case dock::Bottom:
x = m_bounding.x();
y = m_bounding.y() - m_bounding.height();
break;
case dock::Left:
x = m_bounding.x();
y = m_bounding.y();
break;
}
}
void DockPositioner::update() {
int newX, newY;
calculatePosition(newX, newY);
if (m_x == newX && m_y == newY) {
return;
}
m_positionTimer->start();
}
DropArea {
// ... 其他属性保持不变 ...
onEntered: function (drag) {
handleDragEnter(drag);
}
onExited: function (drag) {
handleDragExit(drag);
}
onPositionChanged: function (drag) {
handleDragPosition(drag);
}
onDropped: function (drop) {
handleDragDrop(drop);
}
function handleDragEnter(drag) {
if (!shouldProcessDragEvent()) return;
if (root.itemId !== "dde-trash") return;
isDragOver = true;
lastDragPosition = Qt.point(drag.x, drag.y);
lastEnterTime = Date.now();
shouldShowTooltip = true;
if (!tooltipVisible) {
tooltipDelayTimer.start();
}
}
function handleDragExit(drag) {
if (!shouldProcessDragEvent()) return;
if (root.itemId !== "dde-trash") return;
isDragOver = false;
lastExitTime = Date.now();
shouldShowTooltip = false;
tooltipVisible = false;
tooltipDelayTimer.stop();
dragToolTip.close();
}
function handleDragPosition(drag) {
if (!isDragOver || !drag || root.itemId !== "dde-trash") return;
const currentPos = Qt.point(drag.x, drag.y);
const distance = calculateDistance(currentPos, lastDragPosition);
if (distance > dragStableThreshold) {
updateTooltipPosition();
}
}
function handleDragDrop(drop) {
if (root.itemId === "dde-trash") {
cleanupTooltip();
}
root.dropFilesOnItem(root.itemId, drop.urls);
}
function shouldProcessDragEvent() {
const currentTime = Date.now();
return currentTime - lastExitTime >= debounceThreshold;
}
function calculateDistance(pos1, pos2) {
return Math.sqrt(Math.pow(pos1.x - pos2.x, 2) + Math.pow(pos1.y - pos2.y, 2));
}
function updateTooltipPosition() {
const point = root.mapToItem(null, root.width / 2, root.height / 2);
dragToolTip.DockPanelPositioner.bounding = Qt.rect(point.x, point.y, dragToolTip.width, dragToolTip.height);
}
function cleanupTooltip() {
isDragOver = false;
shouldShowTooltip = false;
tooltipVisible = false;
tooltipDelayTimer.stop();
dragToolTip.close();
}
}
onDropped: function (drop) {
if (root.itemId === "dde-trash") {
// 验证URL安全性
const validUrls = drop.urls.filter(url => {
try {
const path = decodeURIComponent(url.toString());
return !path.includes("..") && !path.includes("~");
} catch (e) {
console.warn("Invalid URL:", url);
return false;
}
});
if (validUrls.length > 0) {
root.dropFilesOnItem(root.itemId, validUrls);
}
}
}这些改进建议主要关注：
建议在实施这些改进时进行充分的测试，特别是拖拽功能和通知显示功能。 |
![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/dock/dockpositioner.cpp:267-270` </location>
<code_context>
int horizontalOffset = m_horizontalOffset == -1 ? m_bounding.width() / 2 : m_horizontalOffset;
int vertialOffset = m_vertialOffset == -1 ? m_bounding.height() / 2 : m_vertialOffset;
+
+ qWarning() << "DockPanelPositioner::updatePosition called for" << parent()->objectName()
+ << "bounding:" << m_bounding << "dockPosition:" << dockPosition();
+
</code_context>
<issue_to_address>
**suggestion (performance):** Excessive debug logging may impact performance in production.
Consider controlling these qWarning logs with a debug flag or removing them before production to avoid unnecessary log output and performance impact.
Suggested implementation:
```cpp
int horizontalOffset = m_horizontalOffset == -1 ? m_bounding.width() / 2 : m_horizontalOffset;
int vertialOffset = m_vertialOffset == -1 ? m_bounding.height() / 2 : m_vertialOffset;
if (m_debugLoggingEnabled) {
qWarning() << "DockPanelPositioner::updatePosition called for" << parent()->objectName()
<< "bounding:" << m_bounding << "dockPosition:" << dockPosition();
}
```
You will need to:
1. Add a `bool m_debugLoggingEnabled` member to the `DockPanelPositioner` class (default to `false`).
2. Optionally, add a setter method like `void setDebugLoggingEnabled(bool enabled);` to control this flag.
3. Initialize the flag in the constructor if needed.
</issue_to_address>
### Comment 2
<location> `panels/dock/dockpositioner.cpp:112` </location>
<code_context>
+
+ int xPosition = 0;
+ int yPosition = 0;
+ switch (dockPosition()) {
+ case dock::Top: {
+ xPosition = newBounding.x();
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the position calculation logic to eliminate duplicate branches and use defaults for Top/Left, adjusting only for Right/Bottom.
Here’s one way to collapse all the duplicate logic while keeping the exact same behavior and your logging:
1. Default `x`/`y` from `m_bounding`.
2. Only adjust for `Right` or `Bottom` (Top/Left just keep the defaults).
3. Move your “position unchanged” check & timer/start exactly as before.
4. If you really need that warning in `updatePosition()`, emit it once after the switch.
```cpp
void DockPositioner::update() {
const auto& b = m_bounding;
int x = b.x(), y = b.y();
switch (dockPosition()) {
case dock::Right:
x -= b.width();
break;
case dock::Bottom:
y -= b.height();
break;
default:
break; // Top & Left use the default x/y
}
if (m_x == x && m_y == y) {
qWarning() << "DockPositioner::update: position unchanged, skipping update";
return;
}
m_positionTimer->start();
}
```
And for the panel positioner, collapse your two warnings into one and only adjust what changes per side:
```cpp
void DockPanelPositioner::updatePosition() {
const auto rect = dockGeometry();
int xo = (m_horizontalOffset == -1 ? m_bounding.width()/2 : m_horizontalOffset);
int yo = (m_vertialOffset == -1 ? m_bounding.height()/2 : m_vertialOffset);
// default = Top
int x = m_bounding.x() - xo;
int y = rect.height() + 10;
switch (dockPosition()) {
case dock::Right:
x = -m_bounding.width() - 10;
y = m_bounding.y() - yo;
break;
case dock::Bottom:
y = -m_bounding.height() - 10;
// x stays as default (centered)
break;
case dock::Left:
x = rect.width() + 10;
y = m_bounding.y() - yo;
break;
default:
break; // Top case already covered
}
qWarning() << "DockPanelPositioner::updatePosition"
<< parent()->objectName()
<< dockPosition()
<< "-> (" << x << "," << y << ")";
setX(x);
setY(y);
}
```
This keeps all of your new logging and guard-clauses but removes the four nearly-identical branches, collapses Top/Left into the default, and only adjusts what actually moves.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
panels/dock/dockpositioner.cpp
Outdated
Comment on lines
226
to
270
| break; | ||
| } | ||
|
|
||
| qWarning() << "DockPanelPositioner::updatePosition result - x:" << xPosition << "y:" << yPosition; |
There was a problem hiding this comment.
suggestion (performance): Excessive debug logging may impact performance in production.
Consider controlling these qWarning logs with a debug flag or removing them before production to avoid unnecessary log output and performance impact.
Suggested implementation:
int horizontalOffset = m_horizontalOffset == -1 ? m_bounding.width() / 2 : m_horizontalOffset;
int vertialOffset = m_vertialOffset == -1 ? m_bounding.height() / 2 : m_vertialOffset;
if (m_debugLoggingEnabled) {
qWarning() << "DockPanelPositioner::updatePosition called for" << parent()->objectName()
<< "bounding:" << m_bounding << "dockPosition:" << dockPosition();
}
You will need to:
- Add a
bool m_debugLoggingEnabledmember to theDockPanelPositionerclass (default tofalse). - Optionally, add a setter method like
void setDebugLoggingEnabled(bool enabled);to control this flag. - Initialize the flag in the constructor if needed.
panels/dock/dockpositioner.cpp
Outdated
|
|
||
| int xPosition = 0; | ||
| int yPosition = 0; | ||
| switch (dockPosition()) { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the position calculation logic to eliminate duplicate branches and use defaults for Top/Left, adjusting only for Right/Bottom.
Here’s one way to collapse all the duplicate logic while keeping the exact same behavior and your logging:
- Default
x/yfromm_bounding. - Only adjust for
RightorBottom(Top/Left just keep the defaults). - Move your “position unchanged” check & timer/start exactly as before.
- If you really need that warning in
updatePosition(), emit it once after the switch.
void DockPositioner::update() {
const auto& b = m_bounding;
int x = b.x(), y = b.y();
switch (dockPosition()) {
case dock::Right:
x -= b.width();
break;
case dock::Bottom:
y -= b.height();
break;
default:
break; // Top & Left use the default x/y
}
if (m_x == x && m_y == y) {
qWarning() << "DockPositioner::update: position unchanged, skipping update";
return;
}
m_positionTimer->start();
}And for the panel positioner, collapse your two warnings into one and only adjust what changes per side:
void DockPanelPositioner::updatePosition() {
const auto rect = dockGeometry();
int xo = (m_horizontalOffset == -1 ? m_bounding.width()/2 : m_horizontalOffset);
int yo = (m_vertialOffset == -1 ? m_bounding.height()/2 : m_vertialOffset);
// default = Top
int x = m_bounding.x() - xo;
int y = rect.height() + 10;
switch (dockPosition()) {
case dock::Right:
x = -m_bounding.width() - 10;
y = m_bounding.y() - yo;
break;
case dock::Bottom:
y = -m_bounding.height() - 10;
// x stays as default (centered)
break;
case dock::Left:
x = rect.width() + 10;
y = m_bounding.y() - yo;
break;
default:
break; // Top case already covered
}
qWarning() << "DockPanelPositioner::updatePosition"
<< parent()->objectName()
<< dockPosition()
<< "-> (" << x << "," << y << ")";
setX(x);
setY(y);
}This keeps all of your new logging and guard-clauses but removes the four nearly-identical branches, collapses Top/Left into the default, and only adjusts what actually moves.
18202781743
reviewed
Oct 30, 2025
panels/dock/dockpositioner.cpp
Outdated
| int horizontalOffset = m_horizontalOffset == -1 ? m_bounding.width() / 2 : m_horizontalOffset; | ||
| int vertialOffset = m_vertialOffset == -1 ? m_bounding.height() / 2 : m_vertialOffset; | ||
|
|
||
| qWarning() << "DockPanelPositioner::updatePosition called for" << parent()->objectName() |
Contributor
There was a problem hiding this comment.
这种日志比较多，加上category，另外使用debug，
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: robertkill 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.
update changelog to 2.0.16
Summary by Sourcery
Enhance drag-and-drop UX by stabilizing the trash tooltip display with debounce and movement thresholds, improving cleanup on drag completion, adding debug logs, and optimizing positioner updates.
Enhancements: