perf(qml): lazy-load AlbumTitle collectionCombo for wide-screen startup#446
Merged
Conversation
Reviewer's GuideLazy-load the Y/M/D/All ComboBox in AlbumTitle.qml via a Loader that only instantiates after a narrow-width condition has ever been met, and update all interactions to go through the Loader item while keeping collection view selection and button group behavior in sync. Sequence diagram for lazy-loaded Y_M_D_All ComboBox via Loader in AlbumTitlesequenceDiagram
participant Window
participant AlbumTitle
participant collectionComboLoader
participant collectionCombo
participant GStatus
participant collectionBtnBox
Window->>AlbumTitle: onWidthChanged
alt [window.width <= showCollComboWidth]
AlbumTitle->>AlbumTitle: collectionComboEverNarrow = true
end
AlbumTitle->>collectionComboLoader: active = collectionComboEverNarrow && GStatus.currentViewType === Album.Types.ViewCollecttion && albumControl.getYears().length !== 0
alt [active becomes true]
collectionComboLoader->>collectionCombo: create ComboBox from sourceComponent
collectionCombo->>GStatus: Component.onCompleted connect currentCollecttionViewIndexChanged to updateIndex
GStatus-->>collectionCombo: currentCollecttionViewIndexChanged
collectionCombo->>collectionCombo: updateIndex()
collectionCombo->>collectionBtnBox: setChecked(collectionCombo.currentIndex)
end
GStatus-->>AlbumTitle: currentCollecttionViewIndex changed
AlbumTitle->>collectionCombo: updateIndex()
collectionCombo->>collectionBtnBox: setChecked(collectionCombo.currentIndex)
collectionBtnBox->>collectionCombo: setCurrentIndex(index)
collectionCombo->>collectionCombo: onCurrentIndexChanged
alt [!blocksignal]
collectionCombo->>AlbumTitle: collectionBtnClicked(currentIndex)
end
collectionCombo->>collectionBtnBox: setChecked(currentIndex)
note over Window,collectionComboLoader: Wide-screen startup (window.width > showCollComboWidth) keeps collectionComboLoader inactive, avoiding eager ComboBox instantiation
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="src/qml/AlbumTitle.qml" line_range="350-356" />
<code_context>
+ collectionCombo.currentIndex = index
+ }
- function updateIndex() {
- blocksignal = true
- collectionCombo.currentIndex = GStatus.currentCollecttionViewIndex
- blocksignal = false
- }
+ function updateIndex() {
+ blocksignal = true
+ collectionCombo.currentIndex = GStatus.currentCollecttionViewIndex
+ blocksignal = false
+ collectionBtnBox.setChecked(collectionCombo.currentIndex)
+ }
- onCurrentIndexChanged: {
- if (!blocksignal)
- collectionBtnClicked(currentIndex)
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid redundant setChecked calls in updateIndex/onCurrentIndexChanged.
Because `updateIndex` sets `collectionCombo.currentIndex`, `onCurrentIndexChanged` will still run and call `collectionBtnBox.setChecked(currentIndex)` again, causing the same work twice for each external index sync. Consider either dropping the `setChecked` call from `updateIndex` or guarding the `onCurrentIndexChanged` path with `blocksignal` so only one code path updates `collectionBtnBox`.
</issue_to_address>
### Comment 2
<location path="src/qml/AlbumTitle.qml" line_range="335-331" />
<code_context>
+ Loader {
</code_context>
<issue_to_address>
**suggestion:** Simplify visibility handling between Loader and inner ComboBox.
The Loader already controls visibility using these conditions, while the inner ComboBox re-evaluates them independently. Consider removing the ComboBox’s visibility expression (or binding it directly to `Loader.visible`) to avoid duplicated logic and the risk of the two sets of conditions drifting out of sync.
Suggested implementation:
```
Loader {
id: collectionComboLoader
Layout.minimumWidth: 100
Layout.maximumWidth: 120
Layout.fillWidth: true
// Ensure any inner ComboBox uses the Loader's visibility instead of duplicating conditions
onLoaded: {
if (item && item.visible !== undefined) {
item.visible = Qt.binding(function() { return collectionComboLoader.visible })
}
}
```
1. If the ComboBox inside this `Loader` currently has its own `visible: ...` expression (for example: `visible: window.width <= showCollComboWidth && collectionComboEverNarrow`), remove that expression so it becomes either:
- `visible: true` (letting the `onLoaded` binding above drive visibility), or
- omit `visible` entirely if it is not needed elsewhere.
2. If the ComboBox is defined via `sourceComponent` instead of `source`, and you prefer a purely declarative approach, you can instead:
- Add `visible: collectionComboLoader.visible` directly inside that `ComboBox { ... }` block,
- And then remove the `onLoaded` handler from the `Loader`.
3. Make sure the existing visibility / active logic remains only on the `Loader` (e.g. `active:` or `visible:` on `collectionComboLoader`) so there is a single source of truth for when the dropdown should appear.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Wrap the narrow-screen Y/M/D/All ComboBox in a Loader with a once-latch on collectionComboEverNarrow so wide-screen startup no longer eagerly instantiates a DTK ComboBox (popup/menu/listview). active avoids binding window.width to prevent hover-time rebuild jitter that lost highlight. 将窄屏年月日下拉框 ComboBox 包入 Loader，配合 collectionComboEverNarrow 单调锁，宽屏启动不再 eager 实例化 DTK ComboBox。active 不绑 window.width， 避免 hover 时重建抖动导致下拉菜单高亮丢失。 Log: 延迟加载标题栏年月日下拉框，优化宽屏启动 Influence: 宽屏启动减少一个 DTK ComboBox 实例化开销；窄屏功能与 hover 高亮行为不变。
pengfeixx
approved these changes
Jun 30, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pengfeixx, wyu71 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.
Wrap the narrow-screen Y/M/D/All ComboBox in a Loader with a once-latch on collectionComboEverNarrow so wide-screen startup no longer eagerly instantiates a DTK ComboBox (popup/menu/listview). active avoids binding window.width to prevent hover-time rebuild jitter that lost highlight.
将窄屏年月日下拉框 ComboBox 包入 Loader，配合 collectionComboEverNarrow 单调锁，宽屏启动不再 eager 实例化 DTK ComboBox。active 不绑 window.width， 避免 hover 时重建抖动导致下拉菜单高亮丢失。
Log: 延迟加载标题栏年月日下拉框，优化宽屏启动
Influence: 宽屏启动减少一个 DTK ComboBox 实例化开销；窄屏功能与 hover 高亮行为不变。
Summary by Sourcery
Lazy-load the narrow-screen Y/M/D/All ComboBox in the album title bar to avoid instantiating it on wide-screen startup while keeping narrow-screen behavior consistent.
Enhancements: