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
5 times, most recently
from
September 10, 2026 07:40
4522a44 to
aaa04b4
Compare
Contributor
|
TAG Bot New tag: 6.7.49 |
18202781743
reviewed
Sep 21, 2026
| Q_INVOKABLE DTK_GUI_NAMESPACE::DDciIconPalette makeIconPalette(const QPalette &palette); | ||
| #else | ||
| Q_INVOKABLE DTK_GUI_NAMESPACE::DDciIconPalette makeIconPalette(const QQuickPalette *palette); | ||
| Q_INVOKABLE DTK_GUI_NAMESPACE::DDciIconPalette makeIconPaletteWithForeground(const QQuickPalette *palette, const QColor &foreground); |
18202781743
reviewed
Sep 21, 2026
| property bool contentFlow | ||
| property Component content | ||
| property D.Palette checkedTextColor: DS.Style.checkedButton.text | ||
| readonly property color resolvedTextColor: { |
Contributor
There was a problem hiding this comment.
这种应该不需要才对，不应该为了修这个问题而去引入，
Contributor
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码语法正确，逻辑清晰。修复方案通过移除 palette.windowText 写入操作并引入 makeIconPaletteWithForeground 函数显式传递前景色，有效断开了绑定环。ItemDelegate.qml 中新增的 resolvedTextColor 只读属性正确保留了原有的选中态文字颜色逻辑。C++ 函数 makeIconPaletteWithForeground 复用 makeIconPalette 后通过 foreground.isValid() 条件判断设置前景色，逻辑严谨。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码质量优秀。新增的 C++ 函数 makeIconPaletteWithForeground 有清晰的中文注释说明用途，函数命名语义明确。QML 中 resolvedTextColor 属性命名为只读属性，命名规范。所有 6 个 QML 文件的修改保持一致的替换模式（makeIconPalette → makeIconPaletteWithForeground），代码风格统一。无重复代码，无残留调试信息。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能表现良好。makeIconPaletteWithForeground 函数先调用 makeIconPalette 再设置前景色，仅增加一次条件判断和一次 setForeground 调用，开销极小。移除绑定环实际上消除了 QML 引擎对 palette.windowText 的反复求值，提升了运行时性能。resolvedTextColor 作为只读属性绑定，仅在依赖属性变化时重新求值，符合 QML 最佳实践。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 本次变更未涉及用户输入处理、网络通信、文件操作等安全敏感场景。C++ 函数中对 QColor 参数进行了 isValid() 校验，体现了良好的防御性编程习惯。无安全漏洞。 💡 改进建议代码示例// 本次提交为 Bug 修复，代码实现正确，无需修改
// makeIconPaletteWithForeground 实现简洁高效：
DDciIconPalette DQMLGlobalObject::makeIconPaletteWithForeground(const QQuickPalette *palette, const QColor &foreground)
{
DDciIconPalette iconPalette = makeIconPalette(palette);
if (foreground.isValid())
iconPalette.setForeground(foreground);
return iconPalette;
}本报告由 AI 代码审查工具自动生成 |
52cyb
force-pushed
the
master
branch
2 times, most recently
from
September 28, 2026 05:28
22f0873 to
93367f3
Compare
1. Remove state-dependent palette.windowText bindings from Button, MenuItem and ItemDelegate to avoid feeding ColorSelector output back into the control palette 2. Build DciIcon.palette from the control palette and override its foreground with the resolved state color, keeping the control palette as a read-only input 3. Make ItemDelegate text and indicator icons share the resolved DciIcon foreground without introducing additional properties 4. Reuse the control's resolved DciIcon.palette in IconButton, WindowButton and ButtonIndicator instead of rebuilding it from the base control palette 5. Preserve background, highlight and highlightForeground roles so future role-specific overrides can be propagated to child icons Influence: 1. Button, MenuItem and ItemDelegate no longer report binding loops for palette.windowText 2. Checked and highlighted text and icons retain their state foreground colors 3. Derived and indicator icons receive the same effective palette as their parent control 4. Verify light/dark theme switching and checked, highlighted and disabled states fix(palette): 使用解析后的图标调色板断开 windowText 绑定环 1. 移除 Button、MenuItem 和 ItemDelegate 中依赖控件状态的 palette.windowText 绑定，避免将 ColorSelector 输出回写到控件调色板 2. 基于控件调色板构造 DciIcon.palette，并在其中覆盖已解析的状态前景色， 使控件调色板保持为只读输入 3. ItemDelegate 的文本和指示图标共用 DciIcon 的有效前景色，不引入额外属性 4. IconButton、WindowButton 和 ButtonIndicator 复用控件已经解析的 DciIcon.palette，不再从基础控件调色板重新构造 5. 保留 background、highlight 和 highlightForeground 等角色，便于后续将 各角色的独立覆盖统一传递给子图标 Influence: 1. Button、MenuItem 和 ItemDelegate 不再报告 palette.windowText 绑定循环 2. 选中和高亮状态下的文本、图标继续使用正确的状态前景色 3. 派生图标和指示图标与父控件使用同一份有效调色板 4. 验证亮暗主题切换以及选中、高亮和禁用状态 PMS: TASK-39241
52cyb
changed the title
18202781743
reviewed
Sep 29, 2026
| D.DciIcon.palette: D.DTK.makeIconPalette(palette) | ||
| palette.windowText: D.ColorSelector.textColor | ||
| D.DciIcon.palette: { | ||
| const iconPalette = D.DTK.makeIconPalette(control.palette) |
18202781743
reviewed
Sep 29, 2026
| } | ||
| name: "arrow_ordinary_down" | ||
| palette: control.D.DTK.makeIconPalette(control.palette) | ||
| palette: control.D.DciIcon.palette |
Contributor
There was a problem hiding this comment.
这里不需要用control.D.DciIcon.palette吧，用原来的D.DTK.makeIconPalette(control.palette)应该也可以吧，control不一定是Button，有附加属性DciIcon.palette的值吧，
18202781743
reviewed
Sep 29, 2026
| D.DciIcon.theme: D.ColorSelector.controlTheme | ||
| D.DciIcon.palette: D.DTK.makeIconPalette(palette) | ||
| palette.windowText: D.ColorSelector.textColor | ||
| D.DciIcon.palette: { |
Contributor
There was a problem hiding this comment.
感觉这样改有点儿奇怪，这含义有点儿不同，D.DciIcon.palette针对的是dci图标的调色板，只对图标有效，
而palette.windowText针对的是qt的调色板，是通用的，
可能存在没有D.DciIcon.palette 但有palette.windowText的情况，
This branch has not been deployed
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
3 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.
MenuItem and ItemDelegate to avoid feeding ColorSelector output back
into the control palette
foreground with the resolved state color, keeping the control palette
as a read-only input
DciIcon foreground without introducing additional properties
WindowButton and ButtonIndicator instead of rebuilding it from the
base control palette
future role-specific overrides can be propagated to child icons
Influence:
palette.windowText
colors
their parent control
disabled states
fix(palette): 使用解析后的图标调色板断开 windowText 绑定环
palette.windowText 绑定，避免将 ColorSelector 输出回写到控件调色板
使控件调色板保持为只读输入
DciIcon.palette，不再从基础控件调色板重新构造
各角色的独立覆盖统一传递给子图标
Influence:
PMS: TASK-39241