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.

通过改进位置切换逻辑,实现平滑的过渡动画:
1. 在切换位置时,先保存新位置并临时恢复到旧位置
2. 在旧位置播放隐藏动画
3. 切换到新位置后,从隐藏状态播放显示动画
4. 使用状态标志精确控制动画流程,避免闪烁和显示异常

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

Log: 修复任务栏位置切换时的显示异常
PMS: BUG-346777
@deepin-ci-robot
Copy link

[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.

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

@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

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要实现了 Dock(任务栏)在切换位置(如从底部切换到左侧)时的动画过渡逻辑,确保在 Dock 隐藏后再改变位置,然后再显示出来,从而避免视觉上的闪烁或布局错乱。

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

1. 语法逻辑

  • 状态管理逻辑清晰:引入 isPositionChanging 标志位配合 savedNewPositionpreviousPosition,有效地处理了异步动画过程中的状态流转。逻辑上遵循了“保存新位置 -> 恢复旧位置 -> 执行隐藏动画 -> 应用新位置 -> 执行显示动画”的流程,逻辑是自洽的。
  • 动画结束后的回调处理:在 dockAnimationonStopped 中处理 isPositionChanging 的逻辑非常关键,这确保了位置切换的原子性。
  • 潜在的状态竞争风险
    • ConnectionsonPositionChanged 中,代码尝试通过 isRestoringPosition 标志位来过滤由 Applet.position = previousPosition 触发的递归调用。虽然逻辑上可行,但在 QML 的信号绑定机制中,如果 Panel.position 的变化非常快,或者在动画未结束时再次触发位置变化,可能会导致状态机混乱(例如 savedNewPosition 被覆盖,或者 isRestoringPosition 标志位未正确重置)。
    • 建议:增加对 dockAnimation.isPositionChanging 的检查。如果已经在切换位置中,新的位置变更请求应该被忽略或排队,而不是直接覆盖当前的 savedNewPosition

2. 代码质量

  • 代码可读性:新增加的 handlePositionChangeAfterHide 函数封装了位置切换后的清理和显示逻辑,相比之前的内联代码,结构更清晰,易于维护。
  • 魔法值:代码中使用了 -1 作为 savedNewPosition 的无效值标记。
    • 建议:定义一个常量(例如 const int INVALID_POSITION = -1;)或者使用 undefined / null(如果属性类型允许),以提高代码可读性。
  • 组件职责MutuallyExclusiveMenuItem 组件的改动移除了复杂的 positionChangeCallback,将其逻辑上移到了 Connections 层级统一处理。这是一个很好的重构方向,使得 UI 组件更纯粹,不再关心复杂的动画流程。

3. 代码性能

  • 动画停止与重置:在 onPositionChanged 中调用了 dockAnimation.stop()hideShowAnimation.stop(),这是必要的,可以防止多个动画实例并发运行导致的资源浪费和视觉冲突。
  • 属性绑定MutuallyExclusiveMenuItem 中使用了 Qt.binding 恢复 checked 属性的绑定。这是正确的做法,避免了在 onTriggered 中手动赋值导致绑定永久失效的问题。
  • 频繁的属性读写:在 handlePositionChangeAfterHide 中,根据 useTransformBasedAnimation 修改 dockTransform.x/y。这通常在动画帧之外执行,性能影响可忽略。但在动画密集的场景下,应确保这些属性变化不会触发不必要的重排。

4. 代码安全

  • 空指针/未初始化检查:代码中对 savedNewPosition === -1 进行了检查,防止了在非正常流程下调用 handlePositionChangeAfterHide 导致的错误。
  • 边界情况处理
    • 建议:考虑用户在 Dock 隐藏动画进行期间快速切换位置的情况。目前的逻辑是 stop() 动画然后开始新的流程。虽然安全,但体验上可能会“跳变”。
    • 建议:在 dockAnimation.startAnimation(true) 之前,确保 dock 的可见性状态符合预期,防止在某些极端状态下(如系统休眠唤醒后) Dock 处于不可见但 isShowingtrue 的不一致状态。

总结与改进建议代码

总体而言,这段代码修复了 Dock 切换位置时的闪烁问题,逻辑较为严谨。主要改进点在于增强对并发状态变更的防御性编程。

改进建议代码片段(针对 onPositionChanged 的防御性增强):

Connections {
    id: positionChangeConnections
    property int previousPosition: Panel.position
    property int savedNewPosition: -1
    property bool isRestoringPosition: false

    function onPositionChanged() {
        // 1. 防御性检查:忽略由我们自己恢复操作触发的变化
        if (isRestoringPosition) {
            return;
        }

        // 2. 防御性检查:如果当前正在进行位置切换动画,忽略新的请求或更新目标位置
        // (这里选择忽略以保持状态机简单,也可以选择更新 savedNewPosition)
        if (dockAnimation.isPositionChanging) {
            console.warn("Position change ignored: animation already in progress.");
            return;
        }

        // 保存新位置
        savedNewPosition = Panel.position;

        // 标记开始恢复操作
        isRestoringPosition = true;

        // 恢复到旧位置以触发隐藏动画
        Applet.position = previousPosition;

        // 恢复操作完成(注意:Applet.position 的赋值是同步的,但信号触发是异步的,
        // 由于上面有 isRestoringPosition 守卫,这里立即重置是安全的)
        isRestoringPosition = false;

        // 停止现有动画
        dockAnimation.stop();
        hideShowAnimation.stop();

        // 标记开始位置变更流程
        dockAnimation.isPositionChanging = true;

        // 执行隐藏动画
        if (Panel.hideState === Dock.Hide && !dock.visible) {
            dockAnimation.isPositionChanging = false;
            handlePositionChangeAfterHide();
        } else {
            dockAnimation.startAnimation(false);
        }
    }
    
    // ... handlePositionChangeAfterHide 保持不变 ...
}

补充说明
MutuallyExclusiveMenuItem 中,Component.onCompleted 里建立绑定是好的,但为了更稳健,可以在 onTriggered 中添加对 prop 类型的检查,确保只有特定属性(如 "position")才需要这种复杂的动画流程,其他属性(如图标大小)可以直接切换。不过根据 diff 上下文,似乎这部分逻辑已经被移到了上层 Connections 统一处理,所以目前的改动是合理的。

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.

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.

2 participants