fix(view): defer visible date range refresh#463
Merged
Conversation
Coalesce viewport changes and avoid clearing dates before layout is ready. 合并视口变化，并避免在布局完成前错误清空日期。 Log: 修复合集可见日期范围刷新异常 Influence: 合集滚动和缩略图尺寸变化时的标题日期显示
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR defers and coalesces updates to the visible date range calculation in the album thumbnail list view, ensuring date labels update only after scroll/layout changes settle and correctly handle the empty model case. Sequence diagram for deferred visible date range refreshsequenceDiagram
actor User
participant ThumbnailListViewAlbum as ThumbnailListViewAlbum
participant vbar as ScrollBar
participant timeScopeTimer as Timer
User->>vbar: scroll
vbar->>ThumbnailListViewAlbum: onPositionChanged
ThumbnailListViewAlbum->>ThumbnailListViewAlbum: scheduleTimeScope()
ThumbnailListViewAlbum->>timeScopeTimer: restart()
timeScopeTimer-->>ThumbnailListViewAlbum: onTriggered
ThumbnailListViewAlbum->>ThumbnailListViewAlbum: totalTimeScope()
alt thumbnailModel.rowCount() > 0 and visilbeIndexs.length > 0
ThumbnailListViewAlbum->>ThumbnailListViewAlbum: timeChanged(str)
else thumbnailModel.rowCount() === 0
ThumbnailListViewAlbum->>ThumbnailListViewAlbum: timeChanged("")
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 当前代码已非常完善，无需进一步修改。 |
![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 1 issue, and left some high level feedback:
- The change from
elsetoelse if (thumbnailModel.rowCount() === 0)alters behavior when there are no visible indexes but the model is non-empty; please confirm this case is either impossible or explicitly handled elsewhere to avoid leaving a stale time label. - Consider avoiding the hard-coded
16ms timer interval (e.g., by deriving it from the display refresh rate or at least centralizing it as a named constant) to make the throttling intent clearer and easier to adjust later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change from `else` to `else if (thumbnailModel.rowCount() === 0)` alters behavior when there are no visible indexes but the model is non-empty; please confirm this case is either impossible or explicitly handled elsewhere to avoid leaving a stale time label.
- Consider avoiding the hard-coded `16` ms timer interval (e.g., by deriving it from the display refresh rate or at least centralizing it as a named constant) to make the throttling intent clearer and easier to adjust later.
## Individual Comments
### Comment 1
<location path="src/qml/Control/ListView/ThumbnailListViewAlbum.qml" line_range="108-118" />
<code_context>
+
+ Timer {
+ id: timeScopeTimer
+ interval: 16
+ repeat: false
+ onTriggered: totalTimeScope()
</code_context>
<issue_to_address>
**suggestion:** Consider avoiding the hard-coded 16ms timer interval or documenting the rationale.
This value assumes a ~60 FPS refresh rate and may not behave as intended on different displays. If you just need to batch layout/scroll changes to the next event loop turn, consider `Qt.callLater` or a 0ms single-shot `Timer` instead. If you keep the 16ms delay, define a named constant (e.g. `FRAME_INTERVAL_MS`) to make the intent explicit and avoid a magic number.
```suggestion
// Coalesce scroll and layout changes before querying the visible date range.
// Use a frame-based delay (~60 FPS) to batch updates with UI redraws.
readonly property int FRAME_INTERVAL_MS: 16
function scheduleTimeScope() {
timeScopeTimer.restart()
}
Timer {
id: timeScopeTimer
interval: FRAME_INTERVAL_MS
repeat: false
onTriggered: totalTimeScope()
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+108
to
+118
| // Coalesce scroll and layout changes before querying the visible date range. | ||
| function scheduleTimeScope() { | ||
| timeScopeTimer.restart() | ||
| } | ||
|
|
||
| Timer { | ||
| id: timeScopeTimer | ||
| interval: 16 | ||
| repeat: false | ||
| onTriggered: totalTimeScope() | ||
| } |
There was a problem hiding this comment.
suggestion: Consider avoiding the hard-coded 16ms timer interval or documenting the rationale.
This value assumes a ~60 FPS refresh rate and may not behave as intended on different displays. If you just need to batch layout/scroll changes to the next event loop turn, consider Qt.callLater or a 0ms single-shot Timer instead. If you keep the 16ms delay, define a named constant (e.g. FRAME_INTERVAL_MS) to make the intent explicit and avoid a magic number.
Suggested change
| // Coalesce scroll and layout changes before querying the visible date range. | |
| function scheduleTimeScope() { | |
| timeScopeTimer.restart() | |
| } | |
| Timer { | |
| id: timeScopeTimer | |
| interval: 16 | |
| repeat: false | |
| onTriggered: totalTimeScope() | |
| } | |
| // Coalesce scroll and layout changes before querying the visible date range. | |
| // Use a frame-based delay (~60 FPS) to batch updates with UI redraws. | |
| readonly property int FRAME_INTERVAL_MS: 16 | |
| function scheduleTimeScope() { | |
| timeScopeTimer.restart() | |
| } | |
| Timer { | |
| id: timeScopeTimer | |
| interval: FRAME_INTERVAL_MS | |
| repeat: false | |
| onTriggered: totalTimeScope() | |
| } |
pengfeixx
approved these changes
Jul 20, 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.
Coalesce viewport changes and avoid clearing dates before layout is ready. 合并视口变化，并避免在布局完成前错误清空日期。
Log: 修复合集可见日期范围刷新异常
Influence: 合集滚动和缩略图尺寸变化时的标题日期显示
Summary by Sourcery
Defer and coalesce visible date range updates in the album thumbnail list view to avoid incorrect clearing during scrolling or layout changes.
Bug Fixes:
Enhancements: