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.

@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

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 from b19836d to 6c7fc28 Compare January 26, 2026 14:38
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
@Ivy233 Ivy233 force-pushed the fix-dock-position-transform branch from 6c7fc28 to cc0e4cd Compare January 26, 2026 14:45
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要实现了 Dock 面板在切换位置(如从底部切换到左侧)时的平滑过渡动画。它通过引入“待处理位置”机制,在隐藏动画播放完毕后再实际应用新位置,从而避免了视觉上的突变。

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

1. 语法逻辑

  • 逻辑正确性

    • 改进点:在 DockPanel::setPosition 中,如果 position == SETTINGS->position() 则直接返回。这是好的,避免了不必要的信号发射和状态变更。
    • 潜在问题:在 DockPanel::commitPositionChange 中,直接调用了 SETTINGS->setPosition(m_pendingPosition)。这会触发 DockSettingspositionChanged 信号。如果此时 QML 端的 onPositionChanged 处理逻辑中再次调用 Panel.commitPositionChange(),可能会导致递归或重复操作(虽然目前的 QML 逻辑看起来是安全的,但防御性编程值得考虑)。
    • QML 动画逻辑onBeforePositionChanged 中判断 if (Panel.hideState === Dock.Hide && !dock.visible) 时直接提交更改。这里逻辑是合理的,但需确保 dock.visible 的状态与 hideState 完全同步,否则可能导致判断失误。
  • 状态一致性

    • 改进点:引入了 m_pendingPositionm_hasPendingPosition 两个成员变量来管理中间状态,这是一个清晰的状态机设计。
    • 风险:如果在动画播放期间,用户再次触发位置切换(例如快速点击),目前的逻辑可能会出现混乱。onBeforePositionChanged 中调用了 dockAnimation.stop(),这会中断当前动画。然而,m_hasPendingPosition 会在 setPosition 时被置为 true。如果快速连续调用 setPositionm_pendingPosition 会被覆盖,但 m_hasPendingPosition 保持为 true。这通常是可以接受的(以最后一次点击为准),但需要确认 positionChangeFallbackTimer 的处理逻辑是否与这种快速切换兼容。目前的代码中 positionChangeFallbackTimer 会被重启,这是合理的。

2. 代码质量

  • 可读性与维护性

    • 优点:函数命名清晰(如 commitPositionChange, beforePositionChanged),职责划分明确。
    • 改进点:QML 中的 onPositionChanged 逻辑变得比较复杂,混合了动画处理和位置变更逻辑。建议将 handlePositionChangeAfterHide 中的变换重置逻辑封装成一个独立的 JavaScript 函数或 QML 方法,以提高代码的模块化程度。
  • 魔法值

    • 改进点positionChangeFallbackTimerinterval: 1000 是一个硬编码的魔法值。建议将其定义为常量(例如在 QML 顶部或 C++ 头文件中),并添加注释说明为什么选择 1 秒(例如:考虑到最长动画时间 + 安全余量)。
  • 注释

    • 优点:关键步骤有注释(如 // Emit beforePositionChanged...)。
    • 建议dockpanel.cpp 中的 commitPositionChange 是一个关键动作,建议添加更详细的注释,说明它应该在何时被调用(例如:"Must be called after the hide animation completes to apply the new position")。

3. 代码性能

  • 信号与槽

    • 现状:使用了 Q_EMIT 和 QML 的 Connections,这是 Qt 的标准做法,性能开销在可接受范围内。
    • 潜在问题:在 onBeforePositionChanged 中,每次切换位置都会调用 dockAnimation.stop()hideShowAnimation.stop()。虽然这是必要的,但频繁的停止和启动动画可能会造成轻微的卡顿,尤其是在性能较低的设备上。
  • 定时器

    • 现状:引入了 positionChangeFallbackTimer 作为保底机制,防止动画卡死导致位置无法切换。
    • 建议:这个定时器的间隔时间(1000ms)可能略长。如果用户快速切换位置,会有明显的延迟感。建议评估实际动画的最长耗时,适当缩短此时间(例如 500ms),或者在动画正常结束的回调中确保定时器被停止(代码中已包含 positionChangeFallbackTimer.stop(),做得很好)。

4. 代码安全

  • 空指针与异常

    • 现状:代码中使用了 SETTINGS 指针,假设它始终有效。如果 SETTINGS 未初始化或已被销毁,会导致崩溃。
    • 建议:在 DockPanel::commitPositionChange 中,建议添加断言或空指针检查(尽管在单例模式下通常不需要,但为了代码健壮性)。
  • 线程安全

    • 现状DockPanel 继承自 QObject,通常运行在主线程。SETTINGS 的访问也通常在主线程。如果 SETTINGS 是跨线程共享的,需要加锁,但根据上下文推测这里应该没有问题。
  • 边界条件

    • 改进点:在 DockPanel::setPosition 中,m_pendingPosition = position 直接赋值是安全的,因为 Position 是枚举类型。
    • 建议:在 QML 的 handlePositionChangeAfterHide 中,使用了 Applet.position。请确保在 onPositionChanged 触发时,Applet.position 已经更新为新值,否则动画的起始位置计算会出错。

总结与改进建议代码片段

建议 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:处理快速切换的逻辑(可选)

如果需要处理极快速的连续点击,可以在 DockPanel::setPosition 中增加一个检查:

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 {
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,

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.

3 participants