Fix: Optimize tooltip positioning logic.#571
Open
JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
Open
Fix: Optimize tooltip positioning logic.#571JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
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.
18202781743
reviewed
Feb 10, 2026
qt6/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.
你这是自己写了个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，我们目前可以不处理，
JWWTSL
force-pushed
the
origin/solve-bug
branch
from
d34b765 to
85e00ca
Compare
18202781743
reviewed
Feb 24, 2026
qt6/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.
对于这个AlertToolTip可以重写一个,如果我们实在不能使用Popup达到这个效果的话,但那个动画我们需要保留,
建议用Control去模拟,这样的话,对应的contentItem和background以及一些边距都能跟之前的保持一致,这种enter和exit动画看能不能用hovered状态去实现,
Contributor
|
TAG Bot New tag: 6.7.34 |
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
Contributor
deepin pr auto reviewGit Diff 代码审查报告：AlertToolTip.qml代码变更概述这段代码将 语法逻辑分析优点
问题与建议
代码质量评估优点
问题与建议
代码性能考虑优点
问题与建议
代码安全审查优点
问题与建议
改进建议Control {
id: control
property Item target
property string text
property alias font: contentText.font
property int timeout: 0
// 使用双下划线表示私有属性
readonly property real __naturalWidth: Math.max(DS.Style.alertToolTip.width,
contentText.implicitWidth + DS.Style.alertToolTip.horizontalPadding * 2)
readonly property real __connectorPositionRatio: 0.75 // 将魔法数字定义为常量
x: 0
y: __displayY
z: D.DTK.TopOrder
topPadding: DS.Style.alertToolTip.verticalPadding
bottomPadding: DS.Style.alertToolTip.verticalPadding
leftPadding: DS.Style.alertToolTip.horizontalPadding
rightPadding: DS.Style.alertToolTip.horizontalPadding
// 添加空值检查
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
// 使用双下划线表示私有属性和函数
property real __displayY: target ? target.height : 0
function __updateDisplayY() {
if (target) {
if (visible)
__displayY = target.height + DS.Style.control.spacing
else
__displayY = target.height
}
}
onVisibleChanged: __updateDisplayY()
onTargetChanged: __updateDisplayY()
// 添加空值检查
Connections {
target: control.target
function onHeightChanged() {
if (control.target)
control.__updateDisplayY()
}
}
Component.onCompleted: __updateDisplayY()
Behavior on __displayY {
NumberAnimation { duration: 200 }
}
Timer {
interval: control.timeout
running: control.timeout > 0 && control.visible
onTriggered: control.visible = false
}
background: Item {
implicitWidth: DS.Style.alertToolTip.width
implicitHeight: DS.Style.alertToolTip.height
FloatingPanel {
anchors.fill: parent
radius: DS.Style.alertToolTip.radius
backgroundColor: DS.Style.alertToolTip.background
insideBorderColor: DS.Style.alertToolTip.insideBorder
outsideBorderColor: DS.Style.alertToolTip.outsideBorder
}
BoxShadow {
id: connector
property D.Palette dropShadowColor: DS.Style.alertToolTip.connecterdropShadow
property D.Palette backgroundColor: DS.Style.alertToolTip.connecterBackground
property D.Palette borderColor: DS.Style.control.border
// 使用定义的常量
y: -height * __connectorPositionRatio
width: DS.Style.alertToolTip.connectorWidth
height: DS.Style.alertToolTip.connectorHeight
shadowBlur: 4
shadowOffsetY: 2
shadowColor: D.ColorSelector.dropShadowColor
cornerRadius: DS.Style.control.radius
Rectangle {
anchors.fill: parent
color: connector.D.ColorSelector.backgroundColor
border.color: connector.D.ColorSelector.borderColor
border.width: 1
}
}
}
contentItem: Text {
id: contentText
property D.Palette textColor: DS.Style.alertToolTip.text
horizontalAlignment: Text.AlignLeft
verticalAlignment: Text.AlignVCenter
text: control.text
color: D.ColorSelector.textColor
wrapMode: Text.Wrap
}
}总结这段代码整体质量较高，实现了从
这些改进将提高代码的可读性、可维护性和健壮性。 |
18202781743
reviewed
Mar 3, 2026
| height: implicitHeight | ||
|
|
||
| // Animated y so tip stays attached to target; enter = from below target to final, exit = back. | ||
| property real _displayY: target ? target.height : 0 |
Contributor
There was a problem hiding this comment.
这个_displayY用绑定的方式吧,应该就不需要_updateDisplayY()这个函数去更新了,
18202781743
reviewed
Mar 3, 2026
| outsideBorderColor: DS.Style.alertToolTip.outsideBorder | ||
| } | ||
|
|
||
| BoxShadow { |
Contributor
There was a problem hiding this comment.
这个连接线,在逻辑上不属于background吧,还是保持原状,
18202781743
reviewed
Mar 3, 2026
| radius: DS.Style.alertToolTip.radius | ||
| readonly property real __naturalWidth: Math.max(DS.Style.alertToolTip.width, | ||
| contentText.implicitWidth + DS.Style.alertToolTip.horizontalPadding * 2) | ||
| implicitWidth: target ? Math.min(__naturalWidth, target.width) : __naturalWidth |
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
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: