fix: fix font not updating in windowed launcher popup on system font change#1483
Open
electricface wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: fix font not updating in windowed launcher popup on system font change#1483electricface wants to merge 1 commit intolinuxdeepin:masterfrom
electricface wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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.
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
d829f56 to
573168c
Compare
deepin pr auto review这份代码审查主要针对将 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与改进建议
总体而言，代码改动在语法上是正确的，但引入了对 Qt 私有 API 的依赖，这增加了维护成本和潜在的兼容性风险。建议在合并前充分测试，特别是跨 Qt 版本的兼容性测试。 |
electricface
changed the title
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
2 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.
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.