Skip to content

fix(dock): Launcher popup menu in small window mode fails to update with system font size changes#1483

Open
electricface wants to merge 1 commit intolinuxdeepin:masterfrom
electricface:swt/fix-bug335169-way4
Open

fix(dock): Launcher popup menu in small window mode fails to update with system font size changes#1483
electricface wants to merge 1 commit intolinuxdeepin:masterfrom
electricface:swt/fix-bug335169-way4

Conversation

@electricface
Copy link
Member

@electricface electricface commented Mar 6, 2026

  • Add font propagation support to PopupWindow
  • Add a font Q_PROPERTY to PopupWindow that
    propagates the font to all QQuickControl descendants via
    QQuickControlPrivate::updateFontRecur(), mirroring the behavior
    of QQuickApplicationWindow.
  • Bind PanelPopupWindow.font to D.DTK.fontManager.t6 so that all
    popup content (tooltips, menus, panels) automatically inherits
    and dynamically responds to system font changes without requiring
    per-applet fixes.

Log: Fix launcher popup menu in small window mode fails to update
with system font size changes
PMS: BUG-335169

Summary by Sourcery

Propagate system font settings to popup windows so launcher and panel popups update correctly when font size changes.

Bug Fixes:

  • Ensure launcher popup menus in small window mode respond to system font size changes by inheriting updated fonts from the system.

Enhancements:

  • Introduce a font property on PopupWindow that propagates its font to descendant controls, aligning popup font behavior with application windows.

Build:

  • Add Qt Quick Templates 2 as a required dependency and link its private module for font propagation support in popup windows.

with system font size changes

- Add font propagation support to PopupWindow
- Add a `font` Q_PROPERTY to PopupWindow that
propagates the font to all QQuickControl descendants via
QQuickControlPrivate::updateFontRecur(), mirroring the behavior
of QQuickApplicationWindow.
- Bind PanelPopupWindow.font to D.DTK.fontManager.t6 so that all
popup content (tooltips, menus, panels) automatically inherits
and dynamically responds to system font changes without requiring
per-applet fixes.

Log: Fix launcher popup menu in small window mode fails to update
with system font size changes
PMS: BUG-335169
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: electricface

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

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 6, 2026

Reviewer's Guide

Adds font propagation support to PopupWindow so that popup-based UIs (panel popups, menus, tooltips) inherit and dynamically react to system font changes, wiring this through a new font Q_PROPERTY and using Qt Quick Templates2 private APIs, plus the necessary build-system updates and QML bindings.

Sequence diagram for font change propagation to popup controls

sequenceDiagram
    participant User
    participant SystemSettings
    participant DDTKFontManager as DDTK_fontManager
    participant PanelPopupQML as PanelPopupWindow_qml
    participant PopupWindow
    participant ContentItem as QQuickItem_contentItem
    participant QQuickControlPrivate

    User->>SystemSettings: changeFontSize(newSize)
    SystemSettings-->>DDTKFontManager: update t6.font(newSize)
    DDTKFontManager-->>PanelPopupQML: t6.family, t6.pixelSize changed
    PanelPopupQML-->>PopupWindow: update font.family, font.pixelSize via binding
    PopupWindow->>PopupWindow: setFont(QFont from QML)
    PopupWindow->>PopupWindow: resolve with QGuiApplication::font()
    PopupWindow->>PopupWindow: propagateFontToContentItem(m_font)
    PopupWindow->>ContentItem: contentItem()
    PopupWindow->>QQuickControlPrivate: updateFontRecur(contentItem, m_font)
    QQuickControlPrivate-->>ContentItem: applyFontToControlsRecursively(m_font)
    ContentItem-->>User: popup menu re-rendered with new font size
Loading

Class diagram for updated PopupWindow font propagation

classDiagram
    class QQuickWindowQmlImpl

    class PopupWindow {
        <<QML_NAMED_ELEMENT_PopupWindow>>
        +PopupWindow(parent: QWindow*)
        +QFont font() const
        +void setFont(font: QFont const&)
        +void resetFont()
        +void mouseReleaseEvent(event: QMouseEvent*)
        +void mousePressEvent(event: QMouseEvent*)
        +void mouseMoveEvent(event: QMouseEvent*)
        +void fontChanged()
        -void propagateFontToContentItem(font: QFont const&)
        -bool m_dragging
        -bool m_pressing
        -QFont m_font
        -bool m_hasExplicitFont
    }

    PopupWindow --|> QQuickWindowQmlImpl
Loading

Architecture/flow diagram for font propagation across components

flowchart LR
    SystemFonts["System font settings"] --> DDTKFontManager["D.DTK.fontManager.t6"]
    DDTKFontManager --> PanelPopupWindowQML["PanelPopupWindow_qml<br/>font.family,font.pixelSize bindings"]
    PanelPopupWindowQML --> PopupWindow["PopupWindow<br/>Q_PROPERTY font"]
    PopupWindow --> QQuickItemContentItem["QQuickItem contentItem()"]
    QQuickItemContentItem --> QQuickControlPrivate["QQuickControlPrivate::updateFontRecur"]
    QQuickControlPrivate --> PopupControls["QQuickControl descendants<br/>(menus, tooltips, panels)"]

    User["User"] --> SystemFonts
    PopupControls --> User
Loading

File-Level Changes

Change Details Files
Add a font Q_PROPERTY to PopupWindow and propagate its value to all QQuickControl descendants so popup content follows system font changes.
  • Introduce a QFont-backed font Q_PROPERTY on PopupWindow with getter, setter, resetter, and fontChanged signal.
  • Track explicit font vs default state via m_hasExplicitFont and resolve the assigned font against QGuiApplication::font() to avoid redundant updates.
  • Implement propagateFontToContentItem() that calls QQuickControlPrivate::updateFontRecur() starting from the window contentItem().
  • Include QQuickItem and qquickcontrol_p_p headers to support font propagation logic.
frame/popupwindow.h
frame/popupwindow.cpp
Bind PanelPopupWindow’s font to the global DTK font manager and wire up Qt Quick Templates2 private linkage in the build system.
  • Bind PanelPopupWindow.font.family and font.pixelSize to D.DTK.fontManager.t6 so popup UIs respond dynamically to system font changes without per-applet overrides.
  • Add Qt QuickTemplates2 to the top-level Qt find_package call and link QtQuickTemplates2Private in frame/CMakeLists.txt to access QQuickControlPrivate APIs used by PopupWindow.
frame/qml/PanelPopupWindow.qml
CMakeLists.txt
frame/CMakeLists.txt

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

@deepin-ci-robot
Copy link

deepin pr auto review

这份代码修改主要是为 PopupWindow 类增加了字体(Font)属性的支持,使其能够像标准的 Qt Quick 控件一样管理字体,并将字体设置传递给其内容项。

以下是对该 diff 的详细审查意见,涵盖语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 依赖引入 (CMakeLists.txt)

    • 正确性:在根目录 CMakeLists.txt 中添加了 QuickTemplates2,并在 frame/CMakeLists.txt 中链接了 QuickTemplates2Private。这是正确的,因为代码中使用了 QQuickControlPrivate,该类定义在 QtQuickTemplates2Private 模块中。
    • 建议:逻辑正确,确保了编译时能找到所需的头文件和符号。
  • 头文件引用 (popupwindow.cpp)

    • 正确性:引入了 <QtQuickTemplates2/private/qquickcontrol_p_p.h>。注意使用了 _p.h 后缀,这是 Qt 私有头文件的惯例。引入路径符合规范。
  • 字体属性实现 (popupwindow.h/cpp)

    • 逻辑一致性:代码实现了 READ, WRITE, RESET, NOTIFY 以及 FINAL 标志,符合 Qt 属性系统的标准模式。
    • setFont 逻辑
      • QFont resolved = font.resolve(QGuiApplication::font());:这行代码逻辑正确。它将传入的字体与应用程序默认字体进行合并(resolve),确保未设置的属性(如字重、斜体等)继承自默认值。
      • if (...) return;:检查新旧字体是否一致,避免不必要的操作和信号发射,逻辑合理。
    • resetFont 逻辑
      • 逻辑正确,将 m_hasExplicitFont 设为 false,并将字体重置为默认构造的 QFont(即默认字体)。
    • propagateFontToContentItem 逻辑
      • 使用 QQuickControlPrivate::updateFontRecur(ci, font) 递归更新字体。这是一个高效的实现,利用了 Qt 内部私有 API 来处理控件树中的字体继承和更新。

2. 代码质量

  • 封装与接口设计

    • 优点:通过添加 font 属性,PopupWindow 的接口更加符合 QML 的习惯,用户可以直接在 QML 中通过 font.family 等方式进行设置,如 PanelPopupWindow.qml 中所示。
    • 潜在问题:引入了 m_hasExplicitFont 成员变量,但在当前的 diff 代码中(setFontresetFont),这个变量被设置了,但在读取或逻辑判断中并没有被使用(例如,font() getter 只是简单返回 m_font)。
    • 改进建议:如果 m_hasExplicitFont 是为了将来区分"用户显式设置的字体"和"继承的默认字体",那么目前的实现是合理的预留。但如果短期内不需要此功能,建议暂时移除以减少状态维护的复杂度,或者在 font() getter 中添加逻辑,根据此标志决定返回 m_font 还是 QGuiApplication::font()
  • 代码风格

    • 命名规范:变量命名(如 m_font, m_hasExplicitFont)符合 Qt 的驼峰命名法和 m_ 前缀惯例。
    • 包含头文件popupwindow.h 中添加了 #include <QtGui/qfont.h>。虽然 QFont 通常通过 <QFont><QtGui> 引入,但直接引入具体类头文件在某些配置下能略微提高编译速度,这里没有问题。

3. 代码性能

  • 字体解析与比较
    • font.resolve()QFont 的比较操作涉及一定的计算,但通常非常快。
    • if (m_font.resolveMask() == resolved.resolveMask() && m_font == resolved) 这里的双重检查(先检查掩码再检查值)是一个很好的优化点,因为 resolveMask() 的比较比完整的字体对象比较要轻量。
  • 递归更新
    • QQuickControlPrivate::updateFontRecur 是递归操作。如果 contentItem 包含极其复杂的控件树,可能会有性能开销。但考虑到这是在用户修改字体属性时触发的(通常不是高频操作),这个性能损耗是可以接受的。

4. 代码安全

  • 私有 API 的使用

    • 风险提示:代码使用了 QtQuickTemplates2PrivateQQuickControlPrivate。这是 Qt 的私有 API。
    • 影响:Qt 的私有 API 在不同版本间(尤其是大版本更新,如 Qt 5 到 Qt 6,甚至 Qt 6.x 的小版本)可能会发生变化而不做通知。使用私有 API 会导致代码在未来升级 Qt 版本时面临较高的编译失败或运行时崩溃风险。
    • 建议:由于这是为了实现类似 QQuickWindowQQuickControl 的字体传播功能,而 Qt 官方可能未在公共 API 中提供直接支持,使用私有 API 可能是唯一的途径。建议在代码中添加明确的注释,标记此处使用了私有 API,以便后续维护者注意兼容性问题。
  • 空指针检查

    • if (QQuickItem *ci = contentItem()):在 propagateFontToContentItem 中正确检查了 contentItem() 是否返回空指针,这是安全的做法。

总结

该代码修改整体质量较高,逻辑清晰,正确实现了字体属性的添加和传播功能。主要需要注意以下几点:

  1. 依赖管理:正确添加了 CMake 依赖。
  2. 私有 API 风险:使用了 QQuickControlPrivate,需注意 Qt 版本升级时的兼容性风险。
  3. 未使用的变量m_hasExplicitFont 目前被设置但未在逻辑中深度使用,建议确认其必要性或完善其逻辑。
  4. QML 集成:在 QML 中的使用示例(PanelPopupWindow.qml)展示了正确的属性绑定方式。

改进建议代码片段 (针对 m_hasExplicitFont 的逻辑补充)

如果 m_hasExplicitFont 的意图是控制是否返回显式设置的字体,建议修改 font() 如下:

QFont PopupWindow::font() const
{
    // 如果没有显式设置字体,返回空字体(让 QML 引擎解析为默认值)或应用默认字体
    // 这取决于具体的设计需求。通常 QML 属性系统希望返回当前生效的值。
    // 目前的实现直接返回 m_font,这在 resetFont 后是默认 QFont,在 setFont 后是解析后的 QFont。
    // 这种行为是自洽的。
    return m_font; 
}

目前的实现中,m_hasExplicitFont 似乎更像是一个预留的状态位,如果只是为了标记"是否设置过",目前的实现是可以接受的,但可以注释说明其用途。

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 - I've left some high level feedback:

  • In resetFont() you reset to a default-constructed QFont, which breaks the symmetry with setFont() (which resolves against QGuiApplication::font()); consider resetting to QGuiApplication::font() (or clearing the resolve mask appropriately) so that reset restores the effective system font rather than an uninitialized default.
  • The m_hasExplicitFont flag is set but never read; either remove it to simplify the implementation or make use of it (for example to distinguish between explicit and inherited fonts when reacting to external font changes).
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `resetFont()` you reset to a default-constructed `QFont`, which breaks the symmetry with `setFont()` (which resolves against `QGuiApplication::font()`); consider resetting to `QGuiApplication::font()` (or clearing the resolve mask appropriately) so that reset restores the effective system font rather than an uninitialized default.
- The `m_hasExplicitFont` flag is set but never read; either remove it to simplify the implementation or make use of it (for example to distinguish between explicit and inherited fonts when reacting to external font changes).

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.

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.

2 participants