fix: font not updating in windowed launcher popup on system font change#1483
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 9, 2026
Merged
Conversation
Reviewer's GuideAdds 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 controlssequenceDiagram
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
Class diagram for updated PopupWindow font propagationclassDiagram
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
Architecture/flow diagram for font propagation across componentsflowchart 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
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:
- In
resetFont()you reset to a default-constructedQFont, which breaks the symmetry withsetFont()(which resolves againstQGuiApplication::font()); consider resetting toQGuiApplication::font()(or clearing the resolve mask appropriately) so that reset restores the effective system font rather than an uninitialized default. - The
m_hasExplicitFontflag 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).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
electricface
force-pushed
the
swt/fix-bug335169-way4
branch
from
d829f56 to
573168c
Compare
electricface
changed the title
electricface
force-pushed
the
swt/fix-bug335169-way4
branch
2 times, most recently
from
79c38ee to
0e7df7e
Compare
18202781743
reviewed
Mar 8, 2026
| DS_BEGIN_NAMESPACE | ||
| PopupWindow::PopupWindow(QWindow *parent) | ||
| : QQuickWindowQmlImpl(parent) | ||
| : QQuickApplicationWindow(parent) |
electricface
force-pushed
the
swt/fix-bug335169-way4
branch
from
0e7df7e to
3b008e3
Compare
electricface
changed the title
There was a problem hiding this comment.
Pull request overview
This PR updates the launcher/panel popup window implementation to participate in Qt Quick Controls’ standard font inheritance chain, fixing stale fonts in dynamically created controls after a system font size change.
Changes:
- Rebased
PopupWindowfromQQuickWindowQmlImpltoQQuickApplicationWindow. - Added explicit font settings to
PanelPopupWindow.qmlto align popup UI with DTK font settings. - Updated CMake dependencies to add Qt Quick Templates 2 support.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
frame/qml/PanelPopupWindow.qml |
Sets popup window font properties to DTK-managed font values. |
frame/popupwindow.h |
Changes the window base class to QQuickApplicationWindow and updates includes. |
frame/popupwindow.cpp |
Updates constructor/base calls and event handler base invocations accordingly. |
frame/CMakeLists.txt |
Links against Qt Quick Templates 2 private target to support new base class/include. |
CMakeLists.txt |
Adds QuickTemplates2 to the required Qt components. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
12
| #include <QtQuickTemplates2/private/qquickapplicationwindow_p.h> | ||
|
|
||
| DS_BEGIN_NAMESPACE | ||
| class PopupWindow : public QQuickWindowQmlImpl | ||
| class PopupWindow : public QQuickApplicationWindow | ||
| { |
There was a problem hiding this comment.
PopupWindow is only using the public QQuickApplicationWindow type, but the header includes the private Qt header QtQuickTemplates2/private/qquickapplicationwindow_p.h. This unnecessarily pulls in Qt private APIs/headers and forces consumers/builds to have the corresponding private dev package. Prefer including the public header for QQuickApplicationWindow (and only using the private header if you actually need private symbols).
Comment on lines
95
to
103
| target_link_libraries(dde-shell-frame | ||
| PUBLIC | ||
| Dtk${DTK_VERSION_MAJOR}::Core | ||
| Dtk${DTK_VERSION_MAJOR}::Gui | ||
| Qt${QT_VERSION_MAJOR}::Core | ||
| Qt${QT_VERSION_MAJOR}::Gui | ||
| Qt${QT_VERSION_MAJOR}::Quick | ||
| Qt${QT_VERSION_MAJOR}::QuickTemplates2Private | ||
| PRIVATE |
There was a problem hiding this comment.
Qt::QuickTemplates2Private is being added as a PUBLIC link dependency of dde-shell-frame. Since popupwindow.h is not part of PUBLIC_HEADERS, exporting a private Qt module through the public interface is likely unintended and can break downstream consumers at CMake configure time (private Qt targets often aren't available to consumers). Make this dependency PRIVATE and link the public Qt::QuickTemplates2 module as needed instead.
frame/qml/PanelPopupWindow.qml
Outdated
Comment on lines
+71
to
+72
| font.family: D.DTK.fontManager.t6.family | ||
| font.pixelSize: D.DTK.fontManager.t6.pixelSize |
There was a problem hiding this comment.
Only font.family and font.pixelSize are being set here, which can drop other attributes carried by DTK fonts (weight, style, letterSpacing, etc.) and makes the binding less consistent with other QML that assigns the whole DTK font (e.g. font: D.DTK.fontManager.t6). Consider binding the full font property to the DTK font instead of copying individual fields.
Suggested change
| font.family: D.DTK.fontManager.t6.family | |
| font.pixelSize: D.DTK.fontManager.t6.pixelSize | |
| font: D.DTK.fontManager.t6 |
system font change Rebase PopupWindow on QQuickApplicationWindow instead of QQuickWindowQmlImpl so that all QQuickControl descendants (including dynamically created context menus) participate in Qt's standard font inheritance chain. Previously, QQuickControlPrivate::resolveFont() fell back to QGuiApplication::font() for controls inside PopupWindow because the window was not recognized as a QQuickApplicationWindow. This meant controls created after a font change (e.g. AppItemMenu opened via right-click) always got the stale QGuiApplication font rather than DTK's custom font size. Log: Fix windowed launcher popup menu font not updating when system font size is changed PMS: BUG-335169
electricface
force-pushed
the
swt/fix-bug335169-way4
branch
from
3b008e3 to
3f3f6d7
Compare
deepin pr auto review这份代码 diff 主要对项目中的 以下是对语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见： 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与改进建议
总体而言，这次改动将基类从内部实现类迁移到了标准的公共类，方向是正确的，有助于代码的长期维护。但需要警惕对 Private API 的依赖，尽量解耦以保证跨版本的兼容性。 |
18202781743
approved these changes
Mar 9, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, electricface 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 |
Member
Author
|
/forcemerge |
|
This pr force merged! (status: behind) |
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
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.
Rebase PopupWindow on QQuickApplicationWindow instead of
QQuickWindowQmlImpl so that all QQuickControl descendants (including
dynamically created context menus) participate in Qt's standard
font inheritance chain.
Previously, QQuickControlPrivate::resolveFont() fell back to
QGuiApplication::font() for controls inside PopupWindow because
the window was not recognized as a QQuickApplicationWindow.
This meant controls created after a font change (e.g. AppItemMenu
opened via right-click) always got the stale QGuiApplication font
rather than DTK's custom font size.