-
Notifications
You must be signed in to change notification settings - Fork 28
fix: should also update geometry when tips type changed #392
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures tooltip geometry is updated when the tips widget type switches between single-line and multi-line, even if the text content is unchanged, and removes now-redundant manual width handling in the Bluetooth item. Sequence diagram for Bluetooth tooltip refresh with updated TipsWidgetsequenceDiagram
actor User
participant BluetoothItem
participant TipsWidget
User->>BluetoothItem: hover triggers refreshTips()
BluetoothItem->>TipsWidget: setText(tipsText)
activate TipsWidget
TipsWidget->>TipsWidget: compute typeChanged based on m_type
TipsWidget->>TipsWidget: m_type = SingleLine
alt text unchanged
TipsWidget->>TipsWidget: updateGeometry()
TipsWidget->>TipsWidget: update()
TipsWidget-->>BluetoothItem: return
else text changed
TipsWidget->>TipsWidget: m_text = newText
TipsWidget->>TipsWidget: updateGeometry()
TipsWidget->>TipsWidget: update()
TipsWidget-->>BluetoothItem: return
end
deactivate TipsWidget
BluetoothItem->>BluetoothItem: m_quickPanel->setDescription(description)
BluetoothItem-->>User: updated tooltip and quick panel shown
Class diagram for updated TipsWidget and BluetoothItemclassDiagram
class TipsWidget {
enum Type
Type m_type
QString m_text
QStringList m_textList
+setText(text: QString): void
+setTextList(textList: QStringList): void
+updateGeometry(): void
+update(): void
}
class BluetoothItem {
+refreshTips(): void
QWidget m_tipsLabel
QWidget m_quickPanel
}
BluetoothItem --> TipsWidget : uses m_tipsLabel as tooltip widget
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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 - here's some feedback:
- In
setTextList, thetypeChangedcheck usesm_type != TipsWidget::SingleLine, which means switching fromSingleLinetoMultiLinewill incorrectly settypeChangedtofalse; compare against the new type (or the previous type) instead so type changes are detected correctly. - Both
setTextandsetTextListduplicate the sametypeChanged/updateGeometry()/update()logic; consider capturing the previous type once and refactoring this into a small helper or shared block to avoid divergence between the two paths.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `setTextList`, the `typeChanged` check uses `m_type != TipsWidget::SingleLine`, which means switching from `SingleLine` to `MultiLine` will incorrectly set `typeChanged` to `false`; compare against the new type (or the previous type) instead so type changes are detected correctly.
- Both `setText` and `setTextList` duplicate the same `typeChanged`/`updateGeometry()`/`update()` logic; consider capturing the previous type once and refactoring this into a small helper or shared block to avoid divergence between the two paths.
## Individual Comments
### Comment 1
<location> `plugins/dde-dock/widgets/tipswidget.cpp:22-25` </location>
<code_context>
void TipsWidget::setText(const QString &text)
{
+ bool typeChanged = m_type != TipsWidget::SingleLine;
m_type = TipsWidget::SingleLine;
// 如果传递的是富文本,获取富文本中的纯文本内容进行显示
</code_context>
<issue_to_address>
**issue (bug_risk):** The type change detection in setTextList compares against SingleLine instead of MultiLine, which breaks the intended geometry update logic.
In `setTextList`, `typeChanged` is computed against `TipsWidget::SingleLine`, but the new type is `TipsWidget::MultiLine`. As a result, transitions from `SingleLine` to `MultiLine` don’t trigger `updateGeometry()` / `update()` even though the type changed. This should match `setText` and use `bool typeChanged = (m_type != TipsWidget::MultiLine);` so changes to `MultiLine` are detected correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
目前在 tips 控件的类型(单行/多行)变化时,若原本控件内存储的文案没有 变化,则不一定会重新计算界面宽度.此提交新增了类型变化的检查. PMS: BUG-335551 Log:
deepin pr auto review我来分析一下这两个文件的修改:
建议改进:
这些修改总体上是积极的,提高了代码的可维护性和界面的响应性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, wjyrich 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 |
目前在 tips 控件的类型(单行/多行)变化时,若原本控件内存储的文案没有变化,则不一定会重新计算界面宽度.此提交新增了类型变化的检查.
Summary by Sourcery
Ensure tips widgets recompute and update their layout when switching between single-line and multi-line modes, and simplify Bluetooth tooltip width handling by relying on the tips widget geometry instead of manual sizing.
Bug Fixes:
Enhancements: