Skip to content

Conversation

@deepin-ci-robot
Copy link
Contributor

@deepin-ci-robot deepin-ci-robot commented Aug 25, 2025

Synchronize source files from linuxdeepin/dtkdeclarative.

Source-pull-request: linuxdeepin/dtkdeclarative#521

Summary by Sourcery

Synchronize ComboBox QML component updates from upstream to improve flat styling, dynamic sizing, and popup item layout.

Enhancements:

  • Remove fixed delegate implicit width to allow flexible sizing
  • Add conditional implicitWidth and fill logic for flat TextField and background
  • Introduce popup left/right margins and dynamic implicitWidth based on flat style
  • Implement ArrowListView refreshContentItemWidth function to enforce uniform menu item widths on resize and model changes

@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

@sourcery-ai
Copy link

sourcery-ai bot commented Aug 25, 2025

Reviewer's Guide

This PR updates ComboBox.qml by introducing dynamic width management across delegate, text field, background, and popup components—enabling flat-mode sizing, adding popup margins, and ensuring list items auto-resize to fit view width.

Sequence diagram for dynamic width adjustment in ArrowListView

sequenceDiagram
    participant ComboBox
    participant Popup
    participant ArrowListView
    participant ListView
    participant MenuItem

    ComboBox->>Popup: Open popup
    Popup->>ArrowListView: Initialize contentItem
    ArrowListView->>ListView: Get item count
    ArrowListView->>ArrowListView: refreshContentItemWidth()
    loop For each item
        ArrowListView->>ListView: itemAtIndex(i)
        ArrowListView->>MenuItem: Set item.width = view.width
    end
    ArrowListView->>ArrowListView: onWidthChanged triggers refreshContentItemWidth()
    ArrowListView->>ArrowListView: Connections onCountChanged triggers refreshContentItemWidth()
Loading

File-Level Changes

Change Details Files
Removed fixed delegate width
  • Dropped implicitWidth binding to ListView.view.width in the MenuItem delegate
qt6/src/qml/ComboBox.qml
Adapted TextField layout and implicitWidth for flat mode
  • Changed Layout.fillWidth to depend on !control.flat
  • Calculated implicitWidth conditionally using contentWidth, background width, insets, and paddings
qt6/src/qml/ComboBox.qml
Dynamic background implicitWidth based on flat state
  • Set implicitWidth to use contentWidth plus paddings when flat, otherwise fall back to DS.Style.comboBox.width
qt6/src/qml/ComboBox.qml
Enhanced popup with margins and auto-resizing list items
  • Added leftMargin and rightMargin to Popup
  • Toggled popup implicitWidth between contentItem.implicitWidth and control.width based on flat mode
  • Introduced refreshContentItemWidth() to sync each ArrowListView item width with the view
  • Hooked widthChanged and view.countChanged events to refreshContentItemWidth()
qt6/src/qml/ComboBox.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

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 and they look great!

Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `qt6/src/qml/ComboBox.qml:107` </location>
<code_context>
-            Layout.fillWidth: true
+            Layout.fillWidth: !control.flat
             Layout.fillHeight: true
+            implicitWidth: control.flat ? contentWidth : implicitBackgroundWidth + leftInset + rightInset
+                   || contentWidth + leftPadding + rightPadding
             Layout.rightMargin: DS.Style.comboBox.spacing
             text: control.editable ? control.editText : control.displayText
</code_context>

<issue_to_address>
The implicitWidth assignment uses a chained logical OR, which may be error-prone.

Using '||' for fallback may result in unintended values if the first calculation is 0 or another falsy value. Please use a more explicit approach to ensure the correct width is assigned.
</issue_to_address>

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.

Comment on lines +107 to +109
implicitWidth: control.flat ? contentWidth : implicitBackgroundWidth + leftInset + rightInset
|| contentWidth + leftPadding + rightPadding
Copy link

Choose a reason for hiding this comment

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

issue: The implicitWidth assignment uses a chained logical OR, which may be error-prone.

Using '||' for fallback may result in unintended values if the first calculation is 0 or another falsy value. Please use a more explicit approach to ensure the correct width is assigned.

@deepin-ci-robot deepin-ci-robot force-pushed the sync-pr-521-nosync branch 2 times, most recently from 95ee1be to 62d12dd Compare August 25, 2025 11:30
Synchronize source files from linuxdeepin/dtkdeclarative.

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

deepin pr auto review

我对这个代码审查如下:

  1. ArrowListView.qml 改进建议:
  • 优点:添加了自动调整内容项宽度的功能,确保列表项宽度与视图一致
  • 改进建议:
    • refreshContentItemWidth()函数使用了简单的for循环,当项目数量多时可能影响性能
    • 建议使用更高效的方式,如绑定或Layout.fillWidth属性
    • 没有考虑内容项可能为null的情况,虽然已有检查,但可以添加更多错误处理
  1. ComboBox.qml 改进建议:
  • 优点:
    • 改进了宽度计算逻辑,考虑了不同状态下的宽度需求
    • 添加了对flat模式的特殊处理
    • 优化了布局参数
  • 改进建议:
    • implicitWidth的计算逻辑有些复杂,可以提取为单独的函数提高可读性
    • text属性中的三元表达式嵌套过深,难以维护
    • 建议添加注释说明复杂的布局逻辑
    • popup的leftMargin和rightMargin硬编码为相同值,考虑使用统一变量
  1. FlowStyle.qml 改进建议:
  • 优点:
    • 添加了margin属性,提高了样式配置的灵活性
  • 改进建议:
    • 建议为所有样式属性添加注释说明用途
    • 考虑将相关属性组织到更小的逻辑分组中

总体建议:

  1. 性能优化:

    • 避免在QML中使用大量循环,考虑使用绑定或其他QML特性
    • 对于复杂的计算,考虑使用C++后端
  2. 代码质量:

    • 提取重复代码为可复用组件
    • 添加适当的注释说明复杂逻辑
    • 简化嵌套的条件表达式
  3. 可维护性:

    • 使用命名常量代替魔法数字
    • 将相关属性组织到逻辑分组中
    • 考虑使用类型别名提高代码可读性
  4. 安全性:

    • 添加适当的边界检查
    • 考虑添加错误处理机制
    • 避免直接访问可能为null的属性

这些改进将使代码更加健壮、可维护,并提高性能。

@mhduiy mhduiy merged commit 8ee14f4 into master Aug 27, 2025
14 of 15 checks passed
@mhduiy mhduiy deleted the sync-pr-521-nosync branch August 27, 2025 03:11
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.

3 participants