fix(ComboBox): size popup before it is mapped - #712
Merged
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR prevents Popup.Window ComboBox popups from being mapped at a collapsed height by realizing and measuring a delegate before opening, deriving the list height from that measurement, and using stable arrow-control dimensions. Sequence diagram for sizing the ComboBox popup before mappingsequenceDiagram
participant ComboBox
participant Popup
participant ArrowListView
participant ItemsView
participant Delegate
participant PopupWindow
ComboBox->>Popup: implicitHeight
Popup->>ArrowListView: set temporary viewport
ArrowListView->>ItemsView: forceLayout()
ItemsView->>Delegate: instantiate first item
ArrowListView->>ArrowListView: itemDelegateHeight
ArrowListView->>ArrowListView: sizedHeight
ArrowListView-->>Popup: calculated list height
Popup-->>PopupWindow: set final height
PopupWindow->>PopupWindow: setVisible(true)
Flow diagram for calculating the ComboBox popup heightflowchart TD
A[ComboBox popup opens] --> B{First delegate measured?}
B -- No --> C[Use temporary viewport: 2 × itemHeight]
C --> D["ItemsView.forceLayout()"]
D --> E[Read itemDelegateHeight]
B -- Yes --> E
E --> F[Calculate sizedHeight]
F --> G[Add top and bottom padding]
G --> H[Map Popup.Window at final height]
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}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="qt6/src/qml/ArrowListView.qml" line_range="20" />
<code_context>
property int itemHeight: DS.Style.arrowListView.itemHeight
property alias view: itemsView
+ readonly property real itemDelegateHeight: {
+ const item = itemsView.itemAtIndex(0)
+ return itemsView.contentHeight > 0 && item && item.implicitHeight > 0 ? item.implicitHeight : itemHeight
+ }
+ readonly property real sizedHeight: itemsView.count > 0
</code_context>
<issue_to_address>
**issue (bug_risk):** `itemDelegateHeight` measures `item.implicitHeight` instead of the delegate's actual `height`. A delegate that assigns an explicit height (for example, `height: 44`) while leaving its implicit height at zero falls back to the style `itemHeight`, so `sizedHeight` computes the popup using the wrong row size.
**Triggers:** When a ComboBox uses a custom delegate with an explicit `height` that differs from the style row height.
**Suggested fix:** Use the realized delegate's actual `height` when it is valid, falling back to `implicitHeight` or `itemHeight` only when necessary.
```suggestion
return item && item.height > 0 ? item.height : item && item.implicitHeight > 0 ? item.implicitHeight : itemHeight
```
</issue_to_address>
### Comment 2
<location path="qt6/src/qml/ArrowListView.qml" line_range="28-31" />
<code_context>
visible: itemsView.interactive
Layout.alignment: Qt.AlignHCenter
Layout.fillWidth: true
- Layout.preferredHeight: implicitHeight
+ Layout.preferredHeight: DS.Style.arrowListView.stepButtonIconSize.height
view: itemsView
stepSize: control.itemHeight
</code_context>
<issue_to_address>
**issue (bug_risk):** The arrow Loader rows are allocated `stepButtonIconSize.height` (12px in the checked style), while the loaded `ActionButton` is explicitly sized to `stepButtonSize.height` (16px). The ColumnLayout therefore gives each arrow only the icon height, shrinking or clipping the button and making the popup's arrow-space calculation too small.
**Triggers:** When the style has different `stepButtonSize.height` and `stepButtonIconSize.height`, as in the checked FlowStyle configuration.
**Suggested fix:** Use the actual step-button layout height, such as `DS.Style.arrowListView.stepButtonSize.height`, consistently in both the Loader layout and `sizedHeight` calculation.
</issue_to_address>
mhduiy
force-pushed
the
bug-370839-combobox-popup-size
branch
from
September 29, 2026 02:33
e10a8a5 to
7725739
Compare
18202781743
reviewed
Sep 29, 2026
mhduiy
force-pushed
the
bug-370839-combobox-popup-size
branch
2 times, most recently
from
September 29, 2026 03:38
41ee9e1 to
fb47527
Compare
1. Size the window popup from the model and the measured delegate height, instead of the content item, which has not been laid out yet when the window is created. 2. Measure the delegate height in ArrowListView by realizing one item while the popup is still closed, and pin the scroll-arrow height so the layout settles in one pass. 3. Keep the style item height only as the viewport that makes that first measurement possible. Log: ComboBox popups appear at their final size instead of being resized after being mapped. Influence: Removes the popup size jump with Window popup type. fix(ComboBox): 弹窗按最终尺寸显示 1. 窗口弹窗改用模型与实测行高计算尺寸，不再使用尚未布局的内容项。 2. ArrowListView 在弹窗关闭时提前实例化一项以量出行高，并固定箭头条高度，使布局一次到位。 3. 样式行高仅作为首次测量所需的引导视口。 Log: 下拉弹窗显示时即为其最终尺寸，不再在显示后改变尺寸。 PMS: BUG-370839 Influence: 修复 Window 类型弹窗的尺寸跳动。
mhduiy
force-pushed
the
bug-370839-combobox-popup-size
branch
3 times, most recently
from
September 29, 2026 07:20
cf0e406 to
5214691
Compare
Contributor
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法和逻辑均无问题，边界条件处理完善 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 建议将注释修改为更准确的表达，如"2 * itemHeight is a temporary viewport so the list can realize one row and be measured." 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 对于 ComboBox 场景，条目数量通常有限，forceLayout() 的性能影响可接受。如需进一步优化，可考虑异步延迟布局方案 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 本次修改为纯 UI 布局计算代码，不涉及用户输入处理、网络操作、文件系统操作、命令执行等安全敏感操作，无需安全加固 💡 改进建议代码示例// ComboBox.qml - 建议修改注释为更准确的表达
// 2 * itemHeight is a temporary viewport so the list can realize one row and be measured.
implicitHeight: (contentItem.sizedHeight > 0 ? contentItem.sizedHeight : 2 * DS.Style.arrowListView.itemHeight)
+ topPadding + bottomPadding本报告由 AI 代码审查工具自动生成 |
18202781743
approved these changes
Sep 29, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy 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 |
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.
问题
Popup.Window模式下,弹窗窗口是在内容布局之前创建并映射的:QQuickPopupPrivate::adjustPopupItemParentAndWindow()先按popupItem当时的尺寸建窗并setVisible(true),之后才定位、才布局内容。打开前列表一个 delegate 都没有(contentHeight == 0),隐式高只剩 padding = 20px,窗口就先以 20px 显示,等内容量出来再 resize 到真实高度,并且位置也随之被重算 —— 表现为"先很扁、突然变高、y 跳"。改动
ArrowListView暴露sizedHeight:整列高度 = 实测行高 ×min(行数, maxVisibleItems)+ 滚动时的上下箭头条。行高取自已实例化的第一个 delegate;在还没有 delegate 时退回itemHeight(size hint),并用contentHeight > 0守卫保证只在布局完成后取值。onHeightChanged/onCountChanged→forceLayout()):ListView 在弹窗窗口显示前不会自己创建 delegate,这一步让真实行高在窗口映射之前就可得到。Layout.preferredHeight固定为stepButtonIconSize.height(即它实际布局到的高度):箭头是懒加载的,原来会让布局多走一遍、尺寸变两次。ComboBox的 popup 用sizedHeight设置implicitHeight;测量完成前用一个临时视口(2 × itemHeight)让列表有高度、能把第一项造出来。验证
本地复现(offscreen,窗口首个事件):
改动规模:2 文件 +17 −2。
已知边界
DPopupWindowHandle::adjustPopupPosition()的夹取与 Qt positioner 互相回写),属于另一个问题。PMS: BUG-370839