sync: from linuxdeepin/dtkdeclarative#303
Conversation
Synchronize source files from linuxdeepin/dtkdeclarative. Source-pull-request: linuxdeepin/dtkdeclarative#530
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot 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 |
deepin pr auto review这段代码是关于颜色选择的逻辑,我来分析一下它的语法逻辑、代码质量、性能和安全性:
改进建议:
static const qreal DARK_INACTIVE_ALPHA = 0.6;
static const qreal LIGHT_INACTIVE_ALPHA = 0.4;
总体而言,这段代码质量良好,逻辑清晰,并且已经考虑了一些性能优化。上述建议主要是为了进一步提高代码的可维护性和健壮性。 |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis pull request synchronizes upstream changes by introducing inactive-state color blending in the DQuickControlColorSelector::getColorOf method, allowing controls marked inactive to display a mask based on the window color and theme. Sequence diagram for inactive state color blending in getColorOfsequenceDiagram
participant Selector as DQuickControlColorSelector
participant Helper as DGuiApplicationHelper
participant State as DQMLGlobalObject::ControlState
participant Palette as QPalette
Note over Selector: getColorOf called
Selector->Helper: testAttribute(UseInactiveColorGroup)
alt UseInactiveColorGroup is true and State is InactiveState
Selector->Helper: standardPalette(State.controlTheme)
Helper->Palette: get Window color
Palette-->>Selector: windowColor
Selector->Palette: setAlphaF (0.6 or 0.4 based on theme)
Selector->Helper: blendColor(colorValue, inactiveMaskColor)
Helper-->>Selector: blendedColor
end
Selector-->>Caller: colorValue
Class diagram for updated DQuickControlColorSelector::getColorOf logicclassDiagram
class DQuickControlColorSelector {
+QColor getColorOf(const DQuickControlPalette *palette, const DQMLGlobalObject::ControlState *state)
}
class DGuiApplicationHelper {
+static bool testAttribute(Attribute)
+static QPalette standardPalette(Theme)
+static QColor blendColor(QColor, QColor)
UseInactiveColorGroup
DarkType
}
class DQMLGlobalObject {
InactiveState
ControlState
}
class QPalette {
+QColor color(ColorRole)
Window
}
DQuickControlColorSelector --> DGuiApplicationHelper : uses
DQuickControlColorSelector --> DQMLGlobalObject : uses
DQuickControlColorSelector --> QPalette : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/private/dquickcontrolpalette.cpp:846` </location>
<code_context>
}
+ // items with inactive state should use inactive color, it likes dtkgui's generatePaletteColor_helper.
+ static const auto useInactiveColor = DGuiApplicationHelper::testAttribute(DGuiApplicationHelper::UseInactiveColorGroup);
+ if (useInactiveColor && state->controlState == DQMLGlobalObject::InactiveState) {
+ const auto &palette = DGuiApplicationHelper::standardPalette(state->controlTheme);
+ const auto &windowColor = palette.color(QPalette::Window);
</code_context>
<issue_to_address>
**suggestion:** Consider whether static const is appropriate for useInactiveColor.
If the application's attribute can change at runtime, use a local variable instead to ensure the value is always current.
```suggestion
const auto useInactiveColor = DGuiApplicationHelper::testAttribute(DGuiApplicationHelper::UseInactiveColorGroup);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| // items with inactive state should use inactive color, it likes dtkgui's generatePaletteColor_helper. | ||
| static const auto useInactiveColor = DGuiApplicationHelper::testAttribute(DGuiApplicationHelper::UseInactiveColorGroup); |
There was a problem hiding this comment.
suggestion: Consider whether static const is appropriate for useInactiveColor.
If the application's attribute can change at runtime, use a local variable instead to ensure the value is always current.
| static const auto useInactiveColor = DGuiApplicationHelper::testAttribute(DGuiApplicationHelper::UseInactiveColorGroup); | |
| const auto useInactiveColor = DGuiApplicationHelper::testAttribute(DGuiApplicationHelper::UseInactiveColorGroup); |
Synchronize source files from linuxdeepin/dtkdeclarative.
Source-pull-request: linuxdeepin/dtkdeclarative#530
Summary by Sourcery
Enhancements: