-
Notifications
You must be signed in to change notification settings - Fork 55
fix: Fixed display issues when switching taskbar positions #1400
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
base: master
Are you sure you want to change the base?
Conversation
|
[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
|
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 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.
9b5c6fe to
fffc65c
Compare
fffc65c to
72c38ac
Compare
|
TAG Bot New tag: 2.0.27 |
72c38ac to
b19836d
Compare
panels/dock/package/main.qml
Outdated
| onTriggered: { | ||
| if (Applet[prop] === value) { | ||
| // Manually restore checked state since Qt already toggled it | ||
| checked = true; |
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.
这个checked被手动赋值后,是不是解绑了,之前的绑定不起作用了,
checked = Qt.binding(function() {
return Applet[prop] === value;
});
b19836d to
6c7fc28
Compare
Add beforePositionChanged signal to coordinate position change animations. Implement two-phase position change: hide at old position, show at new position. Add commitPositionChange mechanism to ensure position is updated after animations. Fix auto-hide trigger logic for KeepHidden and SmartHide modes after animations. Add fallback timer to ensure position change completes even if animation fails. 修复:改进任务栏位置切换动画和自动隐藏行为 添加 beforePositionChanged 信号以协调位置切换动画。 实现两阶段位置切换:在旧位置隐藏,在新位置显示。 添加 commitPositionChange 机制确保动画后位置正确更新。 修复动画结束后 KeepHidden 和 SmartHide 模式的自动隐藏触发逻辑。 添加后备定时器确保即使动画失败也能完成位置切换。 PMS: BUG-346777
6c7fc28 to
cc0e4cd
Compare
deepin pr auto review这段代码主要实现了 Dock 面板在切换位置(如从底部切换到左侧)时的平滑过渡动画。它通过引入“待处理位置”机制,在隐藏动画播放完毕后再实际应用新位置,从而避免了视觉上的突变。 以下是对该代码的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与改进建议代码片段建议 1:在 C++ 中添加断言和注释 void DockPanel::commitPositionChange()
{
if (!m_hasPendingPosition) {
return;
}
// Defensive check: Ensure SETTINGS is valid
Q_ASSERT(SETTINGS);
m_hasPendingPosition = false;
// Apply the pending position. This will trigger the positionChanged signal.
SETTINGS->setPosition(m_pendingPosition);
}建议 2:在 QML 中定义常量并封装逻辑 // 在文件顶部定义常量
readonly property int positionChangeFallbackInterval: 1000 // ms
// ...
Timer {
id: positionChangeFallbackTimer
interval: positionChangeFallbackInterval // 使用常量
running: false
repeat: false
onTriggered: {
console.warn("Position change fallback timer triggered. Animation might have stalled.");
Panel.commitPositionChange();
}
}
// ...
// 封装动画初始化逻辑
function resetTransformForAnimation() {
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;
}
}
// 在 handlePositionChangeAfterHide 中调用
function handlePositionChangeAfterHide() {
dockAnimation.positionForAnimation = Panel.position;
resetTransformForAnimation(); // 调用封装的函数
dockAnimation.startAnimation(true);
}建议 3:处理快速切换的逻辑(可选) 如果需要处理极快速的连续点击,可以在 void DockPanel::setPosition(const Position& position)
{
if (position == SETTINGS->position()) return;
// 如果当前正在切换位置(即 m_hasPendingPosition 为 true),
// 且新的请求位置与当前待处理的位置不同,则更新待处理位置。
// 这确保了动画只针对最终的目标位置播放一次。
if (m_hasPendingPosition && m_pendingPosition == position) {
return; // 已经在等待切换到这个位置了
}
m_pendingPosition = position;
m_hasPendingPosition = true;
Position oldPosition = SETTINGS->position();
Q_EMIT beforePositionChanged(oldPosition, position);
}总体而言,这段代码的改动逻辑清晰,有效地解决了位置切换时的动画问题。主要的改进空间在于细节的健壮性和代码的可维护性。 |
| } | ||
| } | ||
|
|
||
| Timer { |
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.
这个定时器能否去掉,如果真需要,dde-shell有个DS.singleshot定时执行一次的接口,
c++里数据被调用了setPosition之后,不应该依赖界面来驱动这个流程,不管有没有动画,它都应该执行下去,我们能不能将隐藏显示的动画,补充到另一个流程里,而不是直接嵌入到setPosition,影响这个流程,
如果真需要beforePosition这个值,可以在panel里加一个,类似张坤提的禁用动画的pr,
通过改进位置切换逻辑,实现平滑的过渡动画:
这样可以确保任务栏在位置切换时不会出现空白区域、内容溢出或突然跳变的问题。
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: