Skip to content

Conversation

@wjyrich
Copy link
Contributor

@wjyrich wjyrich commented Jun 13, 2025

as title.

PMS-BUG-313787

Summary by Sourcery

Disable window shadow and border during the dock hide/show animation

Bug Fixes:

  • Remove window shadow effect when hide/show animation is running to prevent residual shadows
  • Disable border width during hide/show animation to avoid visible lines

@sourcery-ai
Copy link

sourcery-ai bot commented Jun 13, 2025

Reviewer's Guide

Introduce conditional logic to D.DWindow properties in the dock panel to remove its shadow and border while the hide/show animation is active, preventing visual artifacts.

File-Level Changes

Change Details Files
Toggle windowEffect and borderWidth based on hideShowAnimation.running
  • Add windowEffect ternary to use EffectNoShadow when animation is running
  • Set borderWidth to 0 during animation and revert to 1 afterward
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 @wjyrich - I've reviewed your changes - here's some feedback:

  • Consider extracting the hideShowAnimation.running condition into a dedicated property (e.g., isAnimating) to avoid duplicating logic and improve readability.
  • Replace the numeric literal 0 for windowEffect with the corresponding named enum/constant to make the default effect clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the `hideShowAnimation.running` condition into a dedicated property (e.g., `isAnimating`) to avoid duplicating logic and improve readability.
- Replace the numeric literal `0` for `windowEffect` with the corresponding named enum/constant to make the default effect clearer.

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.

D.DWindow.enabled: true
D.DWindow.windowRadius: 0
D.DWindow.borderWidth: 1
//D.DWindow.windowEffect: hideShowAnimation.running ? D.PlatformHandle.EffectNoShadow : 0
Copy link
Contributor

Choose a reason for hiding this comment

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

是不是推错了?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

也不是, 因为windoweffect没有效果 想着kwin修复了 再修改,我去备注一下

Copy link
Contributor

@18202781743 18202781743 Jun 16, 2025

Choose a reason for hiding this comment

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

哦哦,这种可以给这个pr标记为草稿,
Uploading image.png…

@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • 注释中提到了一个未解决的问题,即动画结束后窗口阴影和边框宽度的问题。
  • 代码中使用了条件表达式来动态设置 D.DWindow.borderWidth,这可能是为了解决动画结束后的阴影残留问题。

是否建议立即修改:

  • 是,建议立即解决动画结束后的阴影残留问题,因为这可能会影响用户体验。
  • 是,建议移除或修复注释中的 TODO,并在解决后更新注释以反映问题的解决状态。

合并相似的问题:

  • 解决动画结束后的阴影残留问题,并确保 D.DWindow.borderWidth 的设置是正确的。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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-bot
Copy link

deepin-bot bot commented Jun 19, 2025

TAG Bot

New tag: 2.0.0
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1166

@deepin-bot
Copy link

deepin-bot bot commented Jun 24, 2025

TAG Bot

New tag: 2.0.1
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1169

@wjyrich
Copy link
Contributor Author

wjyrich commented Jun 27, 2025

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Jun 27, 2025

This pr force merged! (status: behind)

@deepin-bot deepin-bot bot merged commit 4c19b2d into linuxdeepin:master Jun 27, 2025
7 of 10 checks passed
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