Skip to content

Conversation

@wjyrich
Copy link
Contributor

@wjyrich wjyrich commented Sep 30, 2025

Fix time text flickering issue by ensuring time loader is only active when close button is not visible AND close placeholder is not hovered. Previously, the time text would disappear and reappear when hovering over the close button area due to incorrect activation logic.

The change adds an additional condition to check if closePlaceHolder.hovered is false before activating the time loader, providing more stable visual behavior during hover interactions.

fix: 修复悬停时时间文本闪烁问题

修复时间文本闪烁问题,通过确保时间加载器仅在关闭按钮不可见且关闭占位符未
被悬停时激活。之前当鼠标悬停在关闭按钮区域时,时间文本会因激活逻辑不正确
而消失和重新出现。

此更改添加了一个额外条件来检查 closePlaceHolder.hovered 是否为 false 后 再激活时间加载器,为悬停交互提供更稳定的视觉行为。

PMS: BUG-336145

Summary by Sourcery

Bug Fixes:

  • Prevent time text flickering when hovering over the close button area by refining the loader activation logic

@deepin-ci-robot
Copy link

deepin pr auto review

我对这个代码变更进行审查:

  1. 语法逻辑:
  • 语法正确,没有语法错误
  • 修改是合理的,增加了对 closePlaceHolder.hovered 的判断
  1. 代码质量:
  • 代码可读性良好,逻辑清晰
  • 使用了 Loader 的 active 属性来控制组件的加载,这是一种高效的做法
  • 命名规范一致,使用了驼峰命名法
  1. 代码性能:
  • 这个修改可能会带来轻微的性能提升,因为当 closePlaceHolder 被悬停时,可以避免不必要的 Loader 激活和组件渲染
  • 使用 active 而不是 visible 来控制组件加载是好的实践,可以减少资源占用
  1. 代码安全:
  • 没有明显的安全问题
  • 修改不会引入新的安全风险

改进建议:

  1. 可以考虑添加注释,说明为什么需要在 closePlaceHolder 悬停时保持时间显示,这样可以帮助其他开发者理解这个交互逻辑
  2. 如果 closePlaceHolder 和 time 的布局关系比较复杂,可以考虑使用更明确的布局约束来确保它们的位置关系
  3. 可以考虑将这个条件提取为一个单独的布尔属性,提高代码的可读性:
property bool showTime: !root.closeVisible && !closePlaceHolder.hovered

Loader {
    id: time
    active: showTime
    visible: active
    Layout.alignment: Qt.AlignRight
    sourceComponent: Text { ... }
}

这样的修改可以使代码更加清晰和易于维护,同时保持原有的功能和性能优势。

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 30, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates the time loader activation logic in NotifyItemContent.qml by adding a check that the close placeholder is not hovered, preventing flicker of the time text when hovering over the close button area.

Class diagram for NotifyItemContent.qml time loader activation logic

classDiagram
    class NotifyItem {
      +closeVisible: bool
    }
    class ClosePlaceHolder {
      +hovered: bool
    }
    class TimeLoader {
      +active: bool
      +visible: bool
    }
    NotifyItem --> ClosePlaceHolder : contains
    NotifyItem --> TimeLoader : contains
    TimeLoader : active = !NotifyItem.closeVisible && !ClosePlaceHolder.hovered
    TimeLoader : visible = active
Loading

File-Level Changes

Change Details Files
Refine time loader activation to include hover state
  • Augment active condition to require closePlaceHolder.hovered is false before activating the loader
panels/notification/plugin/NotifyItemContent.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 there - I've reviewed your changes and they look great!


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.

Loader {
id: time
active: !root.closeVisible
active: !root.closeVisible && !closePlaceHolder.hovered
Copy link
Contributor

Choose a reason for hiding this comment

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

关闭按钮有焦点是不是也不应该显示time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

目前关闭按钮有焦点的效果是就是这样 就是没有显示time了。

@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

Fix time text flickering issue by ensuring time loader is only active
when close button is not visible AND close placeholder is not hovered.
Previously, the time text would disappear and reappear when hovering
over the close button area due to incorrect activation logic.

The change adds an additional condition to check if
closePlaceHolder.hovered is false before activating the time loader,
providing more stable visual behavior during hover interactions.

fix: 修复悬停时时间文本闪烁问题

修复时间文本闪烁问题,通过确保时间加载器仅在关闭按钮不可见且关闭占位符未
被悬停时激活。之前当鼠标悬停在关闭按钮区域时,时间文本会因激活逻辑不正确
而消失和重新出现。

此更改添加了一个额外条件来检查 closePlaceHolder.hovered 是否为 false 后
再激活时间加载器,为悬停交互提供更稳定的视觉行为。

PMS: BUG-336145
@wjyrich
Copy link
Contributor Author

wjyrich commented Oct 9, 2025

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Oct 9, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit efa3dee into linuxdeepin:master Oct 9, 2025
8 of 11 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