Fix: Optimize tooltip positioning logic. #571
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JWWTSL The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideReplaces AlertToolTip’s ToolTip/Popup-based implementation with a regular Item-based layout in both Qt5 and Qt6 QML, so the tooltip remains within the visual tree, scrolls/clips with its target, and gains explicit sizing and timeout behavior. Sequence diagram for AlertToolTip visibility and timeout behaviorsequenceDiagram
actor User
participant TargetItem
participant AlertToolTip as AlertToolTip_Item
participant Timer as AlertToolTip_Timer
participant ContentView
User->>TargetItem: Hover or focus causes validation error
TargetItem-->>ContentView: Request to show AlertToolTip_Item
ContentView->>AlertToolTip: set target, text, timeout
ContentView->>AlertToolTip: visible = true
AlertToolTip->>AlertToolTip: compute y = target.height + spacing
AlertToolTip->>AlertToolTip: compute implicitWidth, implicitHeight
AlertToolTip->>Timer: interval = timeout
AlertToolTip->>Timer: running = timeout > 0 && visible
loop While ContentView scrolls
ContentView->>TargetItem: update position within visual tree
ContentView->>AlertToolTip: layout reflow
AlertToolTip->>AlertToolTip: y remains relative to target
end
Timer-->>AlertToolTip: onTriggered()
AlertToolTip->>AlertToolTip: visible = false
Class diagram for updated AlertToolTip Item-based structure (Qt5)classDiagram
class AlertToolTip_Qt5 {
<<Item>>
Item target
string text
font font
int timeout
real __naturalWidth
real x
real y
int z
real implicitWidth
real implicitHeight
real width
real height
}
class AlertToolTip_Timer_Qt5 {
<<Timer>>
int interval
bool running
onTriggered()
}
class AlertToolTip_Background_Qt5 {
<<Item>>
anchors.fill parent
}
class AlertToolTip_Text_Qt5 {
<<Text>>
D.Palette textColor
font font
string text
color color
wrapMode wrapMode
horizontalAlignment horizontalAlignment
verticalAlignment verticalAlignment
}
class AlertToolTip_Connector_Qt5 {
<<BoxShadow>>
D.Palette dropShadowColor
D.Palette backgroundColor
real y
real width
real height
real shadowBlur
real radius
real offsetX
real offsetY
real spread
}
AlertToolTip_Qt5 "1" o-- "1" AlertToolTip_Timer_Qt5 : contains
AlertToolTip_Qt5 "1" o-- "1" AlertToolTip_Background_Qt5 : contains
AlertToolTip_Qt5 "1" o-- "1" AlertToolTip_Text_Qt5 : contains
AlertToolTip_Qt5 "1" o-- "1" AlertToolTip_Connector_Qt5 : contains
Class diagram for updated AlertToolTip Item-based structure (Qt6)classDiagram
class AlertToolTip_Qt6 {
<<Item>>
Item target
string text
font font
int timeout
real __naturalWidth
real x
real y
int z
real implicitWidth
real implicitHeight
real width
real height
}
class AlertToolTip_Timer_Qt6 {
<<Timer>>
int interval
bool running
onTriggered()
}
class AlertToolTip_FloatingPanel_Qt6 {
<<FloatingPanel>>
anchors.fill parent
real radius
real implicitWidth
real implicitHeight
D.Palette backgroundColor
D.Palette insideBorderColor
D.Palette outsideBorderColor
}
class AlertToolTip_Text_Qt6 {
<<Text>>
D.Palette textColor
font font
string text
color color
wrapMode wrapMode
horizontalAlignment horizontalAlignment
verticalAlignment verticalAlignment
}
class AlertToolTip_Connector_Qt6 {
<<BoxShadow>>
D.Palette dropShadowColor
D.Palette backgroundColor
D.Palette borderColor
real y
real width
real height
real shadowBlur
real radius
real offsetX
real offsetY
real spread
}
AlertToolTip_Qt6 "1" o-- "1" AlertToolTip_Timer_Qt6 : contains
AlertToolTip_Qt6 "1" o-- "1" AlertToolTip_FloatingPanel_Qt6 : contains
AlertToolTip_Qt6 "1" o-- "1" AlertToolTip_Text_Qt6 : contains
AlertToolTip_Qt6 "1" o-- "1" AlertToolTip_Connector_Qt6 : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- With the switch from ToolTip(Popup) to Item, the previous enter/exit transitions were dropped; consider reintroducing a simple slide/fade-in/out via Behaviors or explicit animations on
y/opacityto preserve the prior interaction feel. - The new
Item-based tooltip now relies onvisibleinstead ofopen()/close()semantics fromToolTip; if existing callers expect the old API, consider providing small helper functions or a wrapper to keep the usage pattern consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- With the switch from ToolTip(Popup) to Item, the previous enter/exit transitions were dropped; consider reintroducing a simple slide/fade-in/out via Behaviors or explicit animations on `y`/`opacity` to preserve the prior interaction feel.
- The new `Item`-based tooltip now relies on `visible` instead of `open()/close()` semantics from `ToolTip`; if existing callers expect the old API, consider providing small helper functions or a wrapper to keep the usage pattern consistent.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
deepin pr auto review这份代码 diff 主要将 1. 语法逻辑与代码质量优点：
问题与建议：
2. 代码性能问题与建议：
3. 代码安全问题与建议：
4. 改进意见汇总针对 Qt6 版本的具体修改建议： // ... imports
Item {
id: control
property Item target
property string text
property alias font: contentText.font
property int timeout: 0
// 建议：如果 D.DTK.TopOrder 在旧版本不可用，确保有一个回退值或统一的常量
z: D.DTK.TopOrder
// 建议：使用 Binding 来延迟计算或确保逻辑清晰，但当前写法也可接受
x: 0
y: target ? target.height + DS.Style.control.spacing : 0
readonly property real __naturalWidth: Math.max(DS.Style.alertToolTip.width,
contentText.implicitWidth + DS.Style.alertToolTip.horizontalPadding * 2)
// 注意：这里隐含了如果 target 存在，宽度不能超过 target.width
implicitWidth: target ? Math.min(__naturalWidth, target.width) : __naturalWidth
implicitHeight: Math.max(DS.Style.alertToolTip.height,
contentText.implicitHeight + DS.Style.alertToolTip.verticalPadding * 2)
// 显式设置尺寸
width: implicitWidth
height: implicitHeight
// Timer 逻辑良好，无需修改
Timer {
interval: control.timeout
running: control.timeout > 0 && control.visible
onTriggered: control.visible = false
}
// 背景面板
FloatingPanel {
id: backgroundPanel
anchors.fill: parent
// ... 属性保持不变 ...
}
// 文本内容
Text {
id: contentText
// ... 属性保持不变 ...
// 建议：确保 wrapMode 配合 width 约束能正确换行
}
// 连接线
BoxShadow {
id: line
// ...
// y: - height * (0.75)
// 建议：确认视觉位置。由于背景是 anchors.fill parent，且 Text 有 topMargin。
// 如果背景有圆角，可能需要微调 y 值使其看起来是连接在背景顶部边缘的中间。
// 例如：y: -height / 2 可能更居中，具体取决于 height * 0.75 的几何含义。
}
}针对 Qt5 版本的具体修改建议： // ... imports
Item {
id: control
// ... properties ...
// 建议：统一 z-index 处理
z: 100 // 或者引入 DTK 常量
// ... layout logic ...
// 建议：移除多余的 Item 包装，直接使用 BoxShadow 作为兄弟元素，类似于 Qt6 版本的结构
// 除非这个 Item 容器有特殊的裁剪或定位需求。
// 当前代码：
// Item {
// anchors.fill: parent
// BoxShadow { ... } // 背景
// BoxShadow { ... } // 边框
// }
// 建议改为直接放置（如果不需要额外容器）：
BoxShadow {
id: backgroundShadow
anchors.fill: _background // 确保 _background 是正确定义的 Rectangle 或者 Item
// ...
}
// ... Text ...
// ... Connector BoxShadow ...
}总结这次改动整体是积极的，解决了
建议开发者重点测试连接线在不同 |
18202781743
reviewed
Feb 10, 2026
| ToolTip { | ||
| // Use Item instead of ToolTip(Popup) so it stays in the visual tree, | ||
| // scrolls with content and gets clipped properly. | ||
| Item { |
Contributor
There was a problem hiding this comment.
你这是自己写了个ToolTip呀，之后测试要是提了个ToolTip跟随移动的话怎么整，
src/qml/AlertToolTip.qml
Outdated
| ToolTip { | ||
| // Use Item instead of ToolTip(Popup) so it stays in the visual tree, | ||
| // scrolls with content and gets clipped properly. | ||
| Item { |
Contributor
There was a problem hiding this comment.
这个是dtk5的qml，我们目前可以不处理，
Log: The Popup is rendered on the window's Overlay layer and is not part of the content visual tree. As a result, the tooltip does not follow its anchor item during scrolling or window resizing, causing positional drift. To resolve this, AlertToolTip has been changed from a ToolTip (which uses Popup) to a regular Item, making it part of the content visual tree. This ensures it naturally scrolls with its parent, respects container clipping, and maintains correct positioning at all times. PMS: bug-341973
JWWTSL
force-pushed
the
origin/solve-bug
branch
from
d34b765 to
85e00ca
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Log: The Popup is rendered on the window's Overlay layer and is not part of the content visual tree. As a result, the tooltip does not follow its anchor item during scrolling or window resizing, causing positional drift. To resolve this, AlertToolTip has been changed from a ToolTip (which uses Popup) to a regular Item, making it part of the content visual tree. This ensures it naturally scrolls with its parent, respects container clipping, and maintains correct positioning at all times.
PMS: bug-341973
Summary by Sourcery
Replace the alert tooltip popup with an in-tree item to keep it correctly positioned with its target and respect container scrolling and clipping.
Bug Fixes:
Enhancements: