fix(palette): remove palette.windowText writes to break binding loop - #682
Open
52cyb wants to merge 1 commit into
Open
fix(palette): remove palette.windowText writes to break binding loop#68252cyb wants to merge 1 commit into
52cyb wants to merge 1 commit into
Conversation
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 52cyb 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 GuideThe PR breaks palette.windowText binding loops by restoring its native inherited-foreground behavior, routing state colors directly to text and icon consumers, and adding an explicit icon-palette foreground helper. Review should verify binding-loop elimination, checked/hovered/pressed color correctness, and theme/accent responsiveness. Sequence diagram for explicit state foreground resolutionsequenceDiagram
participant Control
participant ColorSelector
participant DQMLGlobalObject
participant DciIcon
participant Text
Control->>ColorSelector: textColor
Control->>DQMLGlobalObject: makeIconPaletteWithForeground(palette, textColor)
DQMLGlobalObject->>DQMLGlobalObject: makeIconPalette(palette)
DQMLGlobalObject->>DciIcon: setForeground(textColor)
Control->>Text: color = textColor
Control->>DciIcon: palette = iconPalette
Flow diagram for restoring native windowText semanticsflowchart LR
State["Control state changes"] --> Selector["D.ColorSelector resolves state foreground"]
Selector --> Text["Text or Label reads state color directly"]
Selector --> Icons["makeIconPaletteWithForeground sets icon foreground"]
Palette["palette.windowText"] --> Native["Qt-native inherited general foreground"]
Native --> Default["Normal-state default foreground"]
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}
52cyb
force-pushed
the
master
branch
4 times, most recently
from
September 10, 2026 07:10
bf948e3 to
4522a44
Compare
1. Remove `palette.windowText: D.ColorSelector.*` bindings from Button, MenuItem and ItemDelegate, the three controls reporting "Binding loop detected for property windowText" 2. Let contentItem text read the state foreground color directly from `D.ColorSelector.textColor`; ItemDelegate exposes a new readonly `resolvedTextColor` for its checked text 3. Add `makeIconPaletteWithForeground(QQuickPalette*, QColor)` and switch the state-dependent icons (Button/MenuItem/ItemDelegate plus the inherited IconButton/WindowButton/ButtonIndicator) to it, passing their foreground explicitly 4. Keep other controls' `palette.windowText` bindings unchanged (scope limited to the three reported loops) Influence: 1. Open control center; no more "Binding loop detected for property windowText" from the three controls 2. Verify checked/highlighted text and icon foreground still turn white (HighlightedText) in light theme 3. Verify other controls (ToolButton/TitleBar/ActionButton...) unchanged 4. Verify light/dark theme switching fix(palette): 移除 palette.windowText 写入以断开绑定环 1. 移除 Button、MenuItem、ItemDelegate 三处 `palette.windowText: D.ColorSelector.*` 绑定，这三者报告 "Binding loop detected for property windowText" 2. contentItem 文字改直接读 `D.ColorSelector.textColor`；ItemDelegate 新增只读 `resolvedTextColor` 表达选中文字色 3. 新增 `makeIconPaletteWithForeground(QQuickPalette*, QColor)`，让三者及连带控件 （IconButton / WindowButton / ButtonIndicator，继承或关联 Button）的图标显式 传入前景色 4. 其余控件（ToolButton/TitleBar/ActionButton 等）的 `palette.windowText` 绑定 保持不变（范围限定到三处报告的环） Influence: 1. 打开控制中心，三个控件不再出现 "Binding loop detected for property windowText" 2. 验证浅色主题下选中/高亮文字与图标前景仍为白色（HighlightedText） 3. 验证其余控件（ToolButton/TitleBar/ActionButton 等）行为不变 4. 验证亮/暗主题切换 PMS: Task-392413
Contributor
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码语法正确，逻辑清晰。移除 palette.windowText 写入操作并通过 makeIconPaletteWithForeground 直接传入前景色的方案有效断开了绑定环。ItemDelegate.qml 中新增的 resolvedTextColor 只读属性正确复制了原有的文字颜色逻辑。C++ 端 makeIconPaletteWithForeground 函数通过 foreground.isValid() 检查确保传入颜色有效。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码结构清晰，注释完整。新增的 C++ 函数 makeIconPaletteWithForeground 附带清晰的中文注释说明用途。QML 变更在 6 个文件中保持一致的模式（Button/ButtonIndicator/IconButton/ItemDelegate/MenuItem/WindowButton），resolvedTextColor 属性命名清晰自解释。无重复代码，无残留调试代码。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，资源使用合理。移除绑定环实际上提升了性能，避免了 QML 引擎中无限绑定求值循环。makeIconPaletteWithForeground 在 makeIconPalette 基础上仅增加一次 QColor 赋值，开销可忽略。无不必要的计算或资源使用。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 本次变更纯涉及 UI 调色板逻辑，无用户输入处理、无网络操作、无文件系统访问、无硬编码凭证，不存在安全风险。 💡 改进建议代码示例// 本次变更已正确实现，无需修复
// makeIconPaletteWithForeground 实现简洁正确：
// DDciIconPalette DQMLGlobalObject::makeIconPaletteWithForeground(
// const QQuickPalette *palette, const QColor &foreground)
// {
// DDciIconPalette iconPalette = makeIconPalette(palette);
// if (foreground.isValid())
// iconPalette.setForeground(foreground);
// return iconPalette;
// }本报告由 AI 代码审查工具自动生成 |
Contributor
|
TAG Bot New tag: 6.7.49 |
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.
palette.windowText: D.ColorSelector.*bindings from Button, MenuItemand ItemDelegate, the three controls reporting "Binding loop detected for
property windowText"
D.ColorSelector.textColor; ItemDelegate exposes a new readonlyresolvedTextColorfor its checked textmakeIconPaletteWithForeground(QQuickPalette*, QColor)and switch thestate-dependent icons (Button/MenuItem/ItemDelegate plus the inherited
IconButton/WindowButton/ButtonIndicator) to it
palette.windowTextbindings unchanged (scope limited tothe three reported loops)
Influence:
from the three controls
(HighlightedText) in light theme
fix(palette): 移除 palette.windowText 写入以断开绑定环
palette.windowText: D.ColorSelector.*绑定，这三者报告 "Binding loop detected for property windowText"
D.ColorSelector.textColor；ItemDelegate 新增只读resolvedTextColor表达选中文字色makeIconPaletteWithForeground(QQuickPalette*, QColor)，让三者及连带控件（IconButton / WindowButton / ButtonIndicator，继承或关联 Button）的图标显式
传入前景色
palette.windowText绑定保持不变（范围限定到三处报告的环）
Influence:
PMS: Task-392413
Summary by Sourcery
Eliminate palette binding loops in core controls while preserving state-dependent text and icon foreground colors.
Bug Fixes:
palette.windowTextbindings from Button, MenuItem, and ItemDelegate to eliminate reported QML binding loops while preserving checked and highlighted text colors.Enhancements: