fix(tooltip): 修复部分情况下 ToolTip 不显示的问题,并调整了偏移量 - #3508
Conversation
Reviewer's Guide调整工具提示打开逻辑,以正确显示启用控件的工具提示;改进对 ComboBox/ComboBoxItem 的祖先检测,从而正确识别工具提示归属;并微调基于鼠标的工具提示位置偏移/放置模式;其余 XAML 文件似乎是非功能性更改(可能是格式/元数据)。 更新后工具提示打开逻辑的序列图sequenceDiagram
participant Mouse
participant Tooltip
participant fe as FrameworkElement
participant Flyout as Flyout
Mouse->>Tooltip: OnOpening(fe)
alt [fe is null or not FrameworkElement]
Tooltip->>Tooltip: _Hush()
Tooltip-->>Mouse: return
else fe.IsEnabled
alt ReferenceEquals(_target, fe) and _flyout.IsOpen
Tooltip-->>Mouse: return
else not ReferenceEquals(_target, fe)
Tooltip->>Tooltip: _Hush()
end
Tooltip->>Tooltip: _latch.Stop()
Tooltip->>Tooltip: _PopUp(fe, _cursor)
Tooltip-->>Mouse: return
else not fe.IsEnabled
alt not ReferenceEquals(_target, fe)
Tooltip->>Tooltip: _Hush()
end
Tooltip->>Tooltip: _StartCycle(fe, _cursor)
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以:
Getting HelpOriginal review guide in EnglishReviewer's GuideAdjusts tooltip opening logic to correctly show tooltips for enabled controls, improves ancestor detection for ComboBox/ComboBoxItem so tooltip ownership is recognized, and tweaks mouse-based tooltip placement offsets/placement mode; remaining XAML files appear to have non-functional changes (likely formatting/metadata). Sequence diagram for updated tooltip opening logicsequenceDiagram
participant Mouse
participant Tooltip
participant fe as FrameworkElement
participant Flyout as Flyout
Mouse->>Tooltip: OnOpening(fe)
alt [fe is null or not FrameworkElement]
Tooltip->>Tooltip: _Hush()
Tooltip-->>Mouse: return
else fe.IsEnabled
alt ReferenceEquals(_target, fe) and _flyout.IsOpen
Tooltip-->>Mouse: return
else not ReferenceEquals(_target, fe)
Tooltip->>Tooltip: _Hush()
end
Tooltip->>Tooltip: _latch.Stop()
Tooltip->>Tooltip: _PopUp(fe, _cursor)
Tooltip-->>Mouse: return
else not fe.IsEnabled
alt not ReferenceEquals(_target, fe)
Tooltip->>Tooltip: _Hush()
end
Tooltip->>Tooltip: _StartCycle(fe, _cursor)
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我已经留下了一些高层次的反馈:
- 在
OnOpening中新的fe.IsEnabled分支和后面的部分逻辑重复(例如_Hush、_target赋值、_latch管理、_cursor),可以考虑重构以避免随着时间推移启用/禁用路径之间的行为出现偏差。 - 你移除了
_TryClaim中的_closing保护。如果_closing在其他地方仍然被使用,建议要么彻底移除该字段,要么说明为什么现在在关闭过程中允许可重入是安全的,以避免未来出现回归问题。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- 在 `OnOpening` 中新的 `fe.IsEnabled` 分支和后面的部分逻辑重复(例如 `_Hush`、`_target` 赋值、`_latch` 管理、`_cursor`),可以考虑重构以避免随着时间推移启用/禁用路径之间的行为出现偏差。
- 你移除了 `_TryClaim` 中的 `_closing` 保护。如果 `_closing` 在其他地方仍然被使用,建议要么彻底移除该字段,要么说明为什么现在在关闭过程中允许可重入是安全的,以避免未来出现回归问题。帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据这些反馈改进后续评审。
Original comment in English
Hey - I've left some high level feedback:
- The new
fe.IsEnabledbranch inOnOpeningduplicates some of the logic that follows (e.g.,_Hush,_targetassignment,_latchmanagement,_cursor), which could be refactored to avoid divergence between enabled/disabled paths over time. - You removed the
_closingguard in_TryClaim; if_closingis still used elsewhere, consider either removing the field entirely or documenting why re-entrancy during closing is now safe to avoid future regressions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `fe.IsEnabled` branch in `OnOpening` duplicates some of the logic that follows (e.g., `_Hush`, `_target` assignment, `_latch` management, `_cursor`), which could be refactored to avoid divergence between enabled/disabled paths over time.
- You removed the `_closing` guard in `_TryClaim`; if `_closing` is still used elsewhere, consider either removing the field entirely or documenting why re-entrancy during closing is now safe to avoid future regressions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e0b2c0db8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _flyout.Placement = PlacementMode.Left; | ||
| _flyout.PlacementRectangle = default; | ||
| _flyout.HorizontalOffset = Math.Round(pt.X + 15 + ToolTipService.GetHorizontalOffset(target)); | ||
| _flyout.VerticalOffset = Math.Round(pt.Y + 25 + ToolTipService.GetVerticalOffset(target)); | ||
| _flyout.VerticalOffset = Math.Round(pt.Y + 15 + ToolTipService.GetVerticalOffset(target)); |
There was a problem hiding this comment.
Keep mouse placement relative to the pointer
When ToolTipService.Placement is Mouse (the default for the controls whose explicit placements were removed in this commit), PlacementMode.Left anchors the popup to the target's left edge, while pt.X/pt.Y are still offsets measured from the pointer within the target. As a result, the tooltip is positioned relative to the target edge rather than the cursor and can appear substantially displaced or fail to follow the cursor, especially on wide controls; this branch should retain a relative/mouse anchor when applying pointer-relative offsets.
Useful? React with 👍 / 👎.
Summary by Sourcery
改进各类 UI 元素的工具提示显示行为和定位。
错误修复:
增强:
Original summary in English
Summary by Sourcery
Improve tooltip display behavior and positioning across various UI elements.
Bug Fixes:
Enhancements: