Skip to content

sync: from linuxdeepin/dtkdeclarative#302

Merged
18202781743 merged 1 commit intomasterfrom
sync-pr-529-nosync
Sep 22, 2025
Merged

sync: from linuxdeepin/dtkdeclarative#302
18202781743 merged 1 commit intomasterfrom
sync-pr-529-nosync

Conversation

@deepin-ci-robot
Copy link
Contributor

@deepin-ci-robot deepin-ci-robot commented Sep 19, 2025

Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#529

Summary by Sourcery

Sync updates from linuxdeepin/dtkdeclarative: improve control item detection and signal connections in C++ backend, adjust QML placeholder styling, add dimension properties for flow components and tooltips, and set TitleBar objectName for identification

Enhancements:

  • Enhance DQuickControlColorSelector to recognize additional special-named items and conditionally connect paletteChanged and hoveredChanged signals only if present
  • Refine placeholderText palette in FlowStyle.qml with increased opacity for both normal and normalDark variants
  • Introduce width, height, and radius properties in FlowStyle.qml and apply them to AlertToolTip background for consistent sizing
  • Assign objectName "ColorSelectorMaster" to TitleBar.qml to support control item detection

Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#529
@deepin-ci-robot
Copy link
Contributor Author

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

代码审查报告

1. AlertToolTip.qml 的修改

优点:

  • 添加了样式属性(radius、width、height),使组件更加灵活和可配置

改进建议:

  1. 代码质量:建议为这些新增的样式属性添加注释,说明它们的用途和默认值
  2. 代码一致性:确保这些新属性与现有的 DS.Style.alertToolTip 中的其他属性命名风格保持一致
  3. 代码可维护性:考虑将这些属性集中在一个样式对象中,而不是分散在各个组件中

2. FlowStyle.qml 的修改

优点:

  • 调整了 placeholderText 的颜色值,提高了对比度,改善了可读性
  • 为 alertToolTip 添加了尺寸和圆角属性,增强了样式控制

改进建议:

  1. 代码性能:颜色值使用 Qt.rgba() 可能不如直接使用预定义的颜色常量高效
  2. 代码可维护性:建议将颜色值提取为命名常量,便于后续维护和修改
  3. 代码一致性:width、height 和 radius 属性的命名应与其他样式属性保持一致的风格

3. TitleBar.qml 的修改

优点:

  • 添加了 objectName,便于在测试和调试时识别组件

改进建议:

  1. 代码安全性:objectName 应该有更具体的命名,避免使用通用名称如 "ColorSelectorMaster",可以考虑使用 "TitleBarColorSelector"
  2. 代码一致性:确保 objectName 的命名符合项目的命名规范

4. dquickcontrolpalette.cpp 的修改

优点:

  • 增强了控件查找逻辑,支持通过 objectName 查找特定控件
  • 改进了信号连接机制,增加了信号存在性检查,提高了代码的健壮性

改进建议:

  1. 代码性能:specialObjectNameItems() 方法的实现需要检查,确保它不会成为性能瓶颈
  2. 代码可维护性:建议为特殊 objectName 的集合添加注释,说明其用途和来源
  3. 代码安全性:在连接信号前检查信号是否存在是好的实践,但建议将这种检查封装为一个辅助方法,提高代码复用性
  4. 代码逻辑:考虑将控件查找和信号连接的逻辑分离,提高代码的模块化程度

总体建议

  1. 代码风格:确保整个项目的命名风格和代码格式保持一致
  2. 代码文档:为新增的属性和方法添加适当的注释,说明其用途和参数
  3. 测试覆盖:对于修改的控件查找和信号连接逻辑,应添加单元测试确保其正确性
  4. 性能优化:对于频繁调用的方法(如控件查找),考虑性能优化和缓存机制

这些修改总体上是积极的,增强了代码的健壮性和可维护性。建议在实施上述改进后,进行全面测试以确保功能正常。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

  • In setControl you guard connections with indexOfSignal checks, but you never disconnect old connections when m_control changes—consider cleaning up previous connections to avoid duplicate slot invocations.
  • The specialObjectNameItems inclusion in findAndSetControlParent couples logic to object names; consider centralizing this list or adding a comment to explain why these names require special handling.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In setControl you guard connections with indexOfSignal checks, but you never disconnect old connections when m_control changes—consider cleaning up previous connections to avoid duplicate slot invocations.
- The specialObjectNameItems inclusion in findAndSetControlParent couples logic to object names; consider centralizing this list or adding a comment to explain why these names require special handling.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 19, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR synchronizes upstream declarative components by extending control detection and adding runtime signal checks in DQuickControlColorSelector, and refines QML styling defaults via updated placeholder colors, explicit sizing and shape properties, and component annotations for consistent theming.

File-Level Changes

Change Details Files
Enhanced control detection and added safety checks for signal connections in DQuickControlColorSelector
  • expanded parentItem check to include special object names
  • wrapped paletteChanged connect in a runtime signal availability check
  • wrapped hoveredChanged connect in a runtime signal availability check
src/private/dquickcontrolpalette.cpp
Updated placeholder text palette for improved contrast
  • changed placeholder normal color to darker tone with higher opacity
  • changed placeholder normalDark color to match increased opacity
qt6/src/qml/FlowStyle.qml
Introduced explicit sizing, shape properties, and component identifiers in QML styles
  • added width, height, and radius properties in FlowStyle
  • applied radius and implicit dimensions in AlertToolTip
  • set objectName property in TitleBar for theme identification
qt6/src/qml/FlowStyle.qml
qt6/src/qml/AlertToolTip.qml
qt6/src/qml/TitleBar.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@18202781743 18202781743 merged commit b932e78 into master Sep 22, 2025
13 of 14 checks passed
@18202781743 18202781743 deleted the sync-pr-529-nosync branch September 22, 2025 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants