fix: Fixed display issues when switching taskbar positions #1400
+90
−23
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
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233 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 GuideRefines the dock’s position-change handling by decoupling menu actions from animation callbacks and introducing a robust, state-driven animation flow that hides the dock at the old position, switches logical position, and then shows it at the new position without visual glitches. Sequence diagram for dock position change animation flowsequenceDiagram
actor User
participant MenuItem
participant Applet
participant Panel
participant positionChangeConnections
participant dockAnimation
participant dock
participant hideShowAnimation
User->>MenuItem: select new position value
MenuItem->>Applet: set position value
Applet->>Panel: update position
Panel-->>positionChangeConnections: onPositionChanged
activate positionChangeConnections
positionChangeConnections->>positionChangeConnections: check isRestoringPosition
alt isRestoringPosition true
positionChangeConnections-->>positionChangeConnections: return (ignore change)
else isRestoringPosition false
positionChangeConnections->>positionChangeConnections: savedNewPosition = Panel.position
positionChangeConnections->>positionChangeConnections: isRestoringPosition = true
positionChangeConnections->>Applet: position = previousPosition
positionChangeConnections->>positionChangeConnections: isRestoringPosition = false
positionChangeConnections->>dockAnimation: stop
positionChangeConnections->>hideShowAnimation: stop
positionChangeConnections->>dockAnimation: isPositionChanging = true
positionChangeConnections->>Panel: read hideState
positionChangeConnections->>dock: read visible
alt Panel.hideState is Hide and dock not visible
positionChangeConnections->>dockAnimation: isPositionChanging = false
positionChangeConnections->>positionChangeConnections: handlePositionChangeAfterHide
else dock is visible
positionChangeConnections->>dockAnimation: startAnimation(false)
end
end
deactivate positionChangeConnections
rect rgb(230,230,255)
dockAnimation-->>dockAnimation: onStopped
alt isPositionChanging true and isShowing false
dockAnimation->>dockAnimation: isPositionChanging = false
dockAnimation->>positionChangeConnections: handlePositionChangeAfterHide
else isShowing true
dockAnimation->>Panel: read hideState
alt Panel.hideState is Hide
dockAnimation->>hideTimer: start
end
end
end
rect rgb(230,255,230)
positionChangeConnections->>positionChangeConnections: handlePositionChangeAfterHide
positionChangeConnections->>positionChangeConnections: update previousPosition
positionChangeConnections->>Applet: position = savedNewPosition
positionChangeConnections->>positionChangeConnections: savedNewPosition = -1
positionChangeConnections->>Panel: changeDragAreaAnchor
positionChangeConnections->>Panel: requestClosePopup
positionChangeConnections->>dockAnimation: configure dockTransform for hidden offset
positionChangeConnections->>dockAnimation: startAnimation(true)
end
Updated class diagram for dockAnimation and positionChangeConnectionsclassDiagram
class DockAnimation {
bool useTransformBasedAnimation
bool isShowing
bool isPositionChanging
var target
string animProperty
startAnimation(show)
stop()
onStopped()
}
class PositionChangeConnections {
int previousPosition
int savedNewPosition
bool isRestoringPosition
onPositionChanged()
handlePositionChangeAfterHide()
onDockSizeChanged()
onCompleted()
}
class Dock {
bool visible
bool useColumnLayout
int width
int height
}
class DockTransform {
int x
int y
}
class Panel {
int position
int dockSize
int hideState
requestClosePopup()
changeDragAreaAnchor()
}
class Applet {
int position
}
DockAnimation --> Dock : animates
DockAnimation --> DockTransform : uses
PositionChangeConnections --> DockAnimation : controls
PositionChangeConnections --> Panel : updates
PositionChangeConnections --> Applet : restores_and_applies_position
PositionChangeConnections --> DockTransform : sets_hidden_offset
Panel --> Dock : configures_size
Flow diagram for dock position change and animation statesflowchart TD
A[User changes dock position via menu] --> B[Panel.position changes]
B --> C[onPositionChanged in positionChangeConnections]
C --> D{isRestoringPosition}
D -->|true| E[Return, ignore change]
D -->|false| F[Save savedNewPosition from Panel.position]
F --> G[Set isRestoringPosition = true]
G --> H[Restore Applet.position to previousPosition]
H --> I[Set isRestoringPosition = false]
I --> J[Stop dockAnimation and hideShowAnimation]
J --> K[Set dockAnimation.isPositionChanging = true]
K --> L{Panel.hideState is Hide and dock not visible}
L -->|true| M[Set dockAnimation.isPositionChanging = false]
M --> N[handlePositionChangeAfterHide]
L -->|false| O["startAnimation(false) for hide"]
O --> P[dockAnimation onStopped]
P --> Q{isPositionChanging and not isShowing}
Q -->|true| R[Set isPositionChanging = false]
R --> N
Q -->|false| S{isShowing and Panel.hideState is Hide}
S -->|true| T[start hideTimer]
S -->|false| U[End]
N --> V{savedNewPosition is valid}
V -->|false| U
V -->|true| W[Set previousPosition = savedNewPosition]
W --> X[Apply Applet.position = savedNewPosition]
X --> Y[Reset savedNewPosition = -1]
Y --> Z[changeDragAreaAnchor and requestClosePopup]
Z --> AA[Set dockTransform to hidden offset based on position]
AA --> AB["startAnimation(true) for show"]
AB --> AC[dockAnimation onStopped with isShowing true]
AC --> AD{Panel.hideState is Hide}
AD -->|true| T
AD -->|false| U
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 left some high level feedback:
- The
dockAnimation.isPositionChangingflag is only reset inonStoppedand the early-return path for hidden docks; if the animation is interrupted (e.g., by another position change or a manualstop()), this flag may remain true and affect later animations—consider centralizing state reset so all code paths that stop animations clear this flag consistently. - In the
Connectionshandler foronPositionChanged, theisRestoringPositionflag is set and immediately cleared after assigningApplet.position; this relies on synchronous behavior of the signal emission—if this logic ever changes or other async work is added, it may become brittle, so consider scoping the restore logic into a helper that manages the flag with a clearer critical section.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `dockAnimation.isPositionChanging` flag is only reset in `onStopped` and the early-return path for hidden docks; if the animation is interrupted (e.g., by another position change or a manual `stop()`), this flag may remain true and affect later animations—consider centralizing state reset so all code paths that stop animations clear this flag consistently.
- In the `Connections` handler for `onPositionChanged`, the `isRestoringPosition` flag is set and immediately cleared after assigning `Applet.position`; this relies on synchronous behavior of the signal emission—if this logic ever changes or other async work is added, it may become brittle, so consider scoping the restore logic into a helper that manages the flag with a clearer critical section.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Ivy233
force-pushed
the
fix-dock-position-transform
branch
from
9b5c6fe to
fffc65c
Compare
Ivy233
changed the title
Improve position switching logic with smooth transition animations: 1. Save new position and temporarily restore to old position when switching 2. Play hide animation at old position 3. Switch to new position and play show animation from hidden state 4. Use state flags to precisely control animation flow and avoid flickering This ensures the dock doesn't show blank areas, content overflow, or sudden jumps during position changes. Log: Fix display issues when switching dock position PMS: BUG-346777
Ivy233
force-pushed
the
fix-dock-position-transform
branch
from
fffc65c to
72c38ac
Compare
deepin pr auto review这段代码主要是为了修复Dock面板在切换位置（如从底部切换到左侧）时的动画和显示逻辑问题。代码整体逻辑清晰，但存在一些可以优化的地方，特别是在逻辑严谨性、性能和代码风格上。 以下是详细的审查意见和改进建议： 1. 代码逻辑与语法审查问题 1：竞态条件与状态管理
问题 2：
问题 3：
2. 代码质量与可读性问题 1：魔法数字
问题 2：函数职责
问题 3：注释缺失
3. 代码性能问题 1：频繁的属性绑定
问题 2：动画停止操作
4. 代码安全问题 1：潜在的无限循环或卡死
5. 综合改进代码示例基于以上分析，以下是优化后的代码片段建议： Connections {
id: positionChangeConnections
property int previousPosition: Panel.position
property int savedNewPosition: -1
property bool isRestoringPosition: false
function onPositionChanged() {
// 忽略由我们自己的恢复操作触发的位置变化
if (isRestoringPosition) {
return;
}
// 保存新位置
savedNewPosition = Panel.position;
// 设置标志以忽略下一次位置变化（由恢复操作触发）
isRestoringPosition = true;
// 暂时恢复到旧位置以执行隐藏动画
Applet.position = previousPosition;
// 清除标志
isRestoringPosition = false;
// 停止任何正在运行的动画
dockAnimation.stop();
hideShowAnimation.stop();
// 标记我们正在改变位置
dockAnimation.isPositionChanging = true;
// 检查Dock当前是否已隐藏
if (Panel.hideState === Dock.Hide && !dock.visible) {
// 如果已隐藏，直接处理位置变更，无需隐藏动画
dockAnimation.isPositionChanging = false;
handlePositionChangeAfterHide();
} else {
// 否则，在旧位置启动隐藏动画
dockAnimation.startAnimation(false);
}
}
function handlePositionChangeAfterHide() {
if (savedNewPosition === -1) return;
// 应用位置变更
previousPosition = savedNewPosition;
Applet.position = savedNewPosition;
savedNewPosition = -1;
changeDragAreaAnchor();
Panel.requestClosePopup();
// 在显示前将变换设置为隐藏位置
prepareTransformForAnimation();
// 启动显示动画
dockAnimation.startAnimation(true);
}
// 辅助函数：准备变换属性，使动画从隐藏状态开始
function prepareTransformForAnimation() {
if (dockAnimation.useTransformBasedAnimation) {
// 计算隐藏偏移量：左侧/顶部为负，右侧/底部为正
var hideOffset = (Applet.position === Dock.Left || Applet.position === Dock.Top)
? -Panel.dockSize
: Panel.dockSize;
if (dock.useColumnLayout) {
dockTransform.x = hideOffset;
dockTransform.y = 0;
} else {
dockTransform.x = 0;
dockTransform.y = hideOffset;
}
} else {
// 如果不使用变换，确保重置
dockTransform.x = 0;
dockTransform.y = 0;
}
}
Component.onCompleted: {
previousPosition = Panel.position
}
function onDockSizeChanged() {
dock.dockSize = Panel.dockSize
}
}总结这段代码的修改是为了实现更平滑的Dock位置切换动画。主要改进点在于：
建议采纳上述关于注释、魔法数字和函数提取的建议，以提高代码的可维护性。逻辑本身在QML环境下是合理且安全的。 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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: 修复任务栏位置切换时的显示异常
PMS: BUG-346777
Summary by Sourcery
Improve dock position change handling to provide smooth hide/show transitions and avoid visual glitches when moving the taskbar.
Bug Fixes:
Enhancements: