Skip to content

Conversation

@Ivy233
Copy link
Contributor

@Ivy233 Ivy233 commented Jan 19, 2026

通过改进位置切换逻辑,实现平滑的过渡动画:

  1. 在切换位置时,先保存新位置并临时恢复到旧位置
  2. 在旧位置播放隐藏动画
  3. 切换到新位置后,从隐藏状态播放显示动画
  4. 使用状态标志精确控制动画流程,避免闪烁和显示异常

这样可以确保任务栏在位置切换时不会出现空白区域、内容溢出或突然跳变的问题。

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:

  • Fix visual glitches such as blank areas and incorrect visibility when changing the dock/taskbar position, especially with auto-hide enabled.

Enhancements:

  • Refine dock animation flow to distinguish between normal show/hide and position-change transitions, coordinating them via explicit state flags.
  • Centralize dock position change handling in a dedicated connections helper that manages restoring the old position, applying the new one, and triggering the appropriate animations.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 19, 2026

Reviewer's Guide

Refines 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 flow

sequenceDiagram
    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
Loading

Updated class diagram for dockAnimation and positionChangeConnections

classDiagram
    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
Loading

Flow diagram for dock position change and animation states

flowchart 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
Loading

File-Level Changes

Change Details Files
Introduce state-driven dock animation flow with explicit handling for position-change sequences.
  • Add isPositionChanging flag to the dockAnimation object to distinguish between normal show/hide and position-change-driven animations.
  • Extend dockAnimation.onStopped to either continue the position-change workflow after a hide animation or trigger auto-hide after a show animation when appropriate.
  • Ensure running animations are stopped and relevant hide/show animations are coordinated around position changes.
panels/dock/package/main.qml
Rework position menu handling so it no longer couples the UI toggle directly to animation callbacks.
  • Simplify ActionButton.onTriggered to only update the underlying Applet property and keep the checked state bound to that property.
  • Remove the previous positionChangeCallback pattern that connected to dockAnimation.onStopped and started animations from the menu layer.
panels/dock/package/main.qml
Centralize and harden dock position-change logic using a dedicated Connections block that orchestrates hide-at-old / show-at-new behavior.
  • Track previousPosition, savedNewPosition, and an isRestoringPosition guard to safely perform temporary position restoration without re-entrancy issues.
  • On position change, temporarily restore the dock to its previous position, stop any running animations, mark a position-change in progress, and either directly finalize the change when the dock is already hidden or start a hide animation otherwise.
  • Implement handlePositionChangeAfterHide to apply the new position, update drag area anchors, close popups, preset transform offsets to the hidden state based on dock orientation, and then run the show animation from the correct off-screen position.
  • Initialize previousPosition on component completion so the first position change has a valid baseline.
panels/dock/package/main.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a 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.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.
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.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch from 9b5c6fe to fffc65c Compare January 20, 2026 06:41
@Ivy233 Ivy233 changed the title fix: 修复任务栏位置切换时的显示异常 fix: Fixed display issues when switching taskbar positions Jan 21, 2026
@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch from fffc65c to 72c38ac Compare January 21, 2026 02:20
@BLumia BLumia requested review from 18202781743 and BLumia January 22, 2026 05:21
@deepin-bot
Copy link

deepin-bot bot commented Jan 23, 2026

TAG Bot

New tag: 2.0.27
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1401

@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch from 72c38ac to b19836d Compare January 23, 2026 09:18
onTriggered: {
if (Applet[prop] === value) {
// Manually restore checked state since Qt already toggled it
checked = true;
Copy link
Contributor

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;
});

@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch 2 times, most recently from 6c7fc28 to cc0e4cd Compare January 26, 2026 14:45
}
}

Timer {
Copy link
Contributor

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,

@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch from cc0e4cd to 808d0fd Compare January 28, 2026 03:35
Q_SIGNALS:
void dockSizeChanged(uint size);
void hideModeChanged(HideMode mode);
void beforePositionChanged(Position oldPosition, Position newPosition);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch 2 times, most recently from e876c06 to 6123dbc Compare January 29, 2026 08:40
@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch from 6123dbc to 7f5936d Compare January 29, 2026 09:17
1. Lift positionForAnimation from dockAnimation to dock Window level
   - Move property from SequentialAnimation to Window scope
   - Update all references from dockAnimation.positionForAnimation to dock.positionForAnimation
   - Improve code organization and semantic clarity
2. Remove m_beforePosition member variable and beforePosition() getter
   - Directly emit beforePositionChanged(SETTINGS->position()) in setPosition()
   - Eliminate unnecessary intermediate storage
   - Simplify code and reduce memory footprint
3. Position change flow improvements
   - C++ directly commits position changes via SETTINGS->setPosition()
   - Animation logic triggered by beforePositionChanged signal
   - No dependency on QML animation callbacks

修复:重构任务栏位置管理并与动画流程解耦

1. 将 positionForAnimation 从 dockAnimation 提升到 dock Window 级别
   - 将属性从 SequentialAnimation 移至 Window 作用域
   - 更新所有引用从 dockAnimation.positionForAnimation 改为 dock.positionForAnimation
   - 改善代码组织和语义清晰度
2. 移除 m_beforePosition 成员变量和 beforePosition() getter 函数
   - 在 setPosition() 中直接发送 beforePositionChanged(SETTINGS->position())
   - 消除不必要的中间存储
   - 简化代码并减少内存占用
3. 位置变更流程改进
   - C++ 直接通过 SETTINGS->setPosition() 提交位置变更
   - 动画逻辑由 beforePositionChanged 信号触发
   - 不依赖 QML 动画回调

PMS: BUG-346777
@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch from 7f5936d to f921a24 Compare January 29, 2026 09:36
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, Ivy233

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要是对 Dock(任务栏)位置切换时的动画逻辑进行了重构。通过引入 beforePositionChanged 信号和 positionForAnimation 属性,实现了位置切换时更平滑的"先隐藏旧位置,再显示新位置"的动画效果。

以下是对这段代码的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

优点:

  • 信号与槽机制正确使用:在 dockpanel.cpp 中新增了 beforePositionChanged 信号,并在 C++ 端 setPosition 时优先发送该信号,逻辑清晰。
  • QML 属性绑定:在 main.qml 中引入了 positionForAnimation 变量,将动画逻辑依赖的属性与实际的 Panel.position 解耦。这使得在位置切换过程中,动画可以使用"旧位置"进行隐藏,而界面布局可以使用"新位置"进行准备,逻辑非常合理。
  • 状态管理:通过 dockAnimation.isPositionChanging 标志位,清晰地管理了位置切换过程中的中间状态,避免了状态混乱。

潜在问题与建议:

  • setTransformToHiddenPosition 中的逻辑判断
    main.qmlsetTransformToHiddenPosition 函数中:

    var hideOffset = (Panel.position === Dock.Left || Panel.position === Dock.Top) ? -Panel.dockSize : Panel.dockSize;

    这里直接使用了 Panel.position。考虑到这个函数是在 onStopped 信号中调用的,且此时 isPositionChangingtrue,意味着位置刚刚在 C++/后端发生了变化,Panel.position 已经是新位置了。
    建议:确认此处逻辑意图。如果是为了将 Transform 设置为"新位置下的隐藏状态",那么使用 Panel.position 是正确的。如果是为了重置到某个基准状态,可能需要重新审视。

  • hideShowAnimation.onStopped 中的嵌套逻辑
    代码中:

    if (isPositionChanging && !isShowing) {
        // ... 处理位置切换 ...
    } else if (isShowing) {
        // ... 处理自动隐藏 ...
    }

    逻辑分支处理得当,但要注意 isPositionChanging 标志位必须确保在所有分支(包括异常中断)中都能被正确重置,否则可能导致后续动画行为异常(例如无法正常显示)。

2. 代码质量

优点:

  • 删除了冗余的回调函数:在 MenuItemonTriggered 中,删除了原本复杂的 positionChangeCallback 闭包函数,改用统一的 Connections 监听 onBeforePositionChanged。这极大地提高了代码的可读性和可维护性,避免了在 QML 中手动连接/断开信号带来的风险。
  • 代码复用:将计算布局方向的逻辑统一为 useColumnLayout,并通过 Panel.rootObject 传递给子组件(如 TaskManager.qmlAppItem.qml),避免了在多处重复计算 position % 2

改进建议:

  • 魔法数字:代码中依然存在 position % 2 的计算。虽然封装了 useColumnLayout,但 position 本身通常是枚举值。如果枚举定义改变(例如 Top=0, Bottom=1, Left=2, Right=3),取模运算可能失效。
    建议:确保枚举定义严格遵循奇偶规则,或者将判断逻辑封装为 C++ 端的辅助函数(如 isVerticalPosition())。
  • 注释清晰度setTransformToHiddenPosition 函数名较长但准确。建议在关键动画状态切换处(如 isPositionChanging = false 的设置处)增加注释,说明此时动画处于什么阶段。

3. 代码性能

优点:

  • 减少不必要的属性绑定计算:通过引入中间变量 positionForAnimation,避免了在动画过程中频繁触发基于 Panel.position 的复杂绑定计算,直到动画真正需要更新时才刷新。

潜在问题与建议:

  • 频繁的属性读写:在动画过程中,dockTransform.x/ydock.width/height 会被频繁修改。这是动画的必然需求,但需确保这些属性没有绑定到极其耗时的计算逻辑上。
  • Connections 的作用域Connections 对象在 main.qml 中是顶层组件,这意味着它会一直监听。由于 Panel 是单例或全局对象,这通常是没问题的。但如果 Panel 对象频繁销毁重建,需注意 Connections 的生命周期管理。

4. 代码安全

优点:

  • 空指针/状态检查:在 onBeforePositionChanged 中检查了 Panel.hideState,防止在 Dock 已经隐藏时执行不必要的隐藏动画。
  • 防抖与竞态处理:在 onHideModeChanged 中增加了 if (!hideShowAnimation.running || !dock.visible) 判断,防止在动画运行期间重复触发导致闪烁或状态错乱。

改进建议:

  • positionForAnimation 的初始化:代码中 property int positionForAnimation: Panel.position 是在声明时初始化的。如果在组件初始化时 Panel 尚未完全准备好(虽然不太可能,但需考虑异步加载情况),可能会导致初始值错误。
    建议:确保 PanelWindow 完成初始化后再赋值,或者在 Component.onCompleted 中显式同步一次。
  • 信号发射顺序:在 dockpanel.cpp 中:
    Q_EMIT beforePositionChanged(SETTINGS->position());
    SETTINGS->setPosition(position);
    这个顺序非常关键。先发信号(携带旧值),再修改设置。这保证了 QML 端能捕捉到旧位置进行隐藏动画。请务必保持这个顺序,不要颠倒。

总结

这是一次高质量的代码重构。主要改进点在于:

  1. 解耦:将"动画显示的位置"与"实际配置的位置"解耦。
  2. 统一管理:将分散在各个控件(如 MenuItem)中的位置切换逻辑统一收归到 Connections 中处理。
  3. 状态机清晰:通过 isPositionChanging 明确了位置切换的生命周期。

最终建议
目前的逻辑主要处理了"位置切换"的动画。如果未来需要支持"模式切换"(如从时尚模式切换到高效模式)或"大小改变"的动画,建议参考本次重构的模式,统一在 Connections 中处理前置状态(beforeXxxChanged)和后置状态(onXxxChanged),以保持代码风格的一致性。

@BLumia BLumia merged commit 8683387 into linuxdeepin:master Jan 29, 2026
8 of 11 checks passed
@Ivy233 Ivy233 deleted the fix-dock-position-transform branch January 29, 2026 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants