fix: improve launcher view state reset logic #693
Closed
Conversation
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
1. Added delayed reset timer in FreeSortListView to handle empty model cases 2. Modified resetViewState() to check for empty model and use timer for delayed reset 3. Added positionViewAtIndex(0, ListView.Beginning) to ensure proper view positioning 4. Changed WindowedFrame's onVisibleChanged handler to reset appList both when showing and hiding 5. The timer ensures reset happens only when model has items, preventing issues with empty lists Log: Improved launcher view reset behavior when opening and closing Influence: 1. Test launcher opening with empty app list 2. Verify view resets to top when launcher is shown 3. Test search functionality and clearing on launcher hide 4. Verify keyboard focus behavior when launcher opens/closes 5. Test folder popup closes properly when launcher hides 6. Check smooth scrolling and positioning when resetting view fix: 改进启动器视图状态重置逻辑 1. 在 FreeSortListView 中添加延迟重置计时器以处理空模型情况 2. 修改 resetViewState() 方法检查空模型并使用计时器进行延迟重置 3. 添加 positionViewAtIndex(0, ListView.Beginning) 确保正确的视图定位 4. 修改 WindowedFrame 的 onVisibleChanged 处理程序，在显示和隐藏时都重 置 appList 5. 计时器确保仅在模型有项目时执行重置，避免空列表问题 Log: 改进启动器打开和关闭时的视图重置行为 Influence: 1. 测试空应用列表时启动器的打开 2. 验证启动器显示时视图是否重置到顶部 3. 测试搜索功能和在启动器隐藏时是否清除 4. 验证启动器打开/关闭时的键盘焦点行为 5. 测试文件夹弹窗在启动器隐藏时是否正确关闭 6. 检查重置视图时的平滑滚动和定位 PMS: BUG-348311
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: robertkill 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 |
Reviewer's GuideRefines launcher view reset behavior by introducing a delayed reset mechanism in FreeSortListView for empty models and adjusting WindowedFrame visibility handling so the app list view state is reset both when the launcher is shown and hidden, ensuring consistent positioning, focus, and cleanup. Sequence diagram for launcher visibility change and app list resetsequenceDiagram
actor User
participant LauncherController
participant WindowedFrame
participant AppList
participant BottomBar
participant BaseLayer
participant FolderGridViewPopup
User->>LauncherController: toggle visible true
LauncherController-->>WindowedFrame: onVisibleChanged(visible true)
WindowedFrame->>AppList: resetViewState()
AppList->>AppList: if model.count === 0
AppList->>AppList: start delayedResetTimer
AppList-->>WindowedFrame: return
AppList->>AppList: when model.count > 0
AppList->>AppList: stop delayedResetTimer
AppList->>AppList: perform resetViewState
AppList->>AppList: currentIndex = 0
AppList->>AppList: contentY = 0
AppList->>AppList: positionViewAtIndex(0, ListView.Beginning)
User->>LauncherController: toggle visible false
LauncherController-->>WindowedFrame: onVisibleChanged(visible false)
WindowedFrame->>BottomBar: searchEdit.text = ""
WindowedFrame->>BaseLayer: focus = true
WindowedFrame->>AppList: resetViewState()
WindowedFrame->>FolderGridViewPopup: close()
Class diagram for WindowedFrame and FreeSortListView changesclassDiagram
class WindowedFrame {
+LauncherController launcherController
+FreeSortListView appList
+BottomBar bottomBar
+Item baseLayer
+FolderGridViewPopup folderGridViewPopup
+onVisibleChanged()
}
class FreeSortListView {
+ListView listView
+Timer delayedResetTimer
+Timer enableAnimationTimer
+bool animationEnabled
+resetViewState()
}
class Timer {
+int interval
+bool repeat
+start()
+stop()
+onTriggered()
}
class ListView {
+int currentIndex
+real contentY
+bool highlightFollowsCurrentItem
+Model model
+positionViewAtIndex(index, mode)
}
class BottomBar {
+SearchEdit searchEdit
}
class SearchEdit {
+string text
}
class FolderGridViewPopup {
+close()
}
WindowedFrame --> FreeSortListView : uses
WindowedFrame --> BottomBar : uses
WindowedFrame --> FolderGridViewPopup : controls
FreeSortListView --> ListView : wraps
FreeSortListView --> Timer : delayedResetTimer
FreeSortListView --> Timer : enableAnimationTimer
BottomBar --> SearchEdit : contains
Flow diagram for FreeSortListView resetViewState and delayed timerflowchart TD
A[start resetViewState] --> B{model.count === 0}
B -->|yes| C[start delayedResetTimer]
C --> D[return]
B -->|no| E[store listView.highlightFollowsCurrentItem in wasFollowing]
E --> F[set listView.highlightFollowsCurrentItem = false]
F --> G[set listView.currentIndex = 0]
G --> H[set listView.contentY = 0]
H --> I[call listView.positionViewAtIndex 0, ListView.Beginning]
I --> J[restore listView.highlightFollowsCurrentItem from wasFollowing]
J --> K[end resetViewState]
subgraph DelayedResetTimer_loop
L[Timer triggered] --> M{model.count > 0}
M -->|no| L
M -->|yes| N[stop delayedResetTimer]
N --> O[call resetViewState]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码主要修改了 1.
|
![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 delayedResetTimer currently polls
listView.model.countevery 50ms; consider instead reacting to a signal (e.g.,onCountChangedoronModelChanged) so reset is event-driven rather than periodic polling. - Both the timer and
resetViewState()assumelistView.modelis non-null and has acountproperty; adding defensive checks forlistView.model && typeof listView.model.count === 'number'would make this more robust against model changes. - Since
Component.onCompletedcallsresetViewState()immediately, it might be worth confirming that the model is already populated at that time for all call sites, or gating the call behind the same delayed-reset logic to avoid transient empty-model behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The delayedResetTimer currently polls `listView.model.count` every 50ms; consider instead reacting to a signal (e.g., `onCountChanged` or `onModelChanged`) so reset is event-driven rather than periodic polling.
- Both the timer and `resetViewState()` assume `listView.model` is non-null and has a `count` property; adding defensive checks for `listView.model && typeof listView.model.count === 'number'` would make this more robust against model changes.
- Since `Component.onCompleted` calls `resetViewState()` immediately, it might be worth confirming that the model is already populated at that time for all call sites, or gating the call behind the same delayed-reset logic to avoid transient empty-model behavior.
## Individual Comments
### Comment 1
<location> `qml/windowed/FreeSortListView.qml:30-34` </location>
<code_context>
}
}
+
+ Timer {
+ id: delayedResetTimer
+ interval: 50
+ repeat: true
+ onTriggered: {
+ if (listView.model.count > 0) {
+ delayedResetTimer.stop()
</code_context>
<issue_to_address>
**suggestion (performance):** Polling with a repeating timer for `model.count > 0` could be brittle and wasteful.
This 50ms repeating timer is effectively a tight polling loop on `model.count`. If the model is late to populate or never does, it runs indefinitely and assumes `model.count` is always readable. Prefer an event-driven approach, e.g. listening to a `model.countChanged`-style signal, or using a one-shot `Timer` you restart from `resetViewState()` while `count === 0` to avoid continuous polling.
Suggested implementation:
```
// One-shot timer used as a deferred reset trigger. It is started from
// resetViewState() while the model is still empty, instead of polling
// continuously with a repeating timer.
Timer {
id: delayedResetTimer
interval: 50
repeat: false
onTriggered: {
// Only reset once the model is populated; otherwise the caller
// (resetViewState) may choose to reschedule this timer.
if (listView.model && listView.model.count > 0) {
resetViewState()
}
}
}
```
To fully implement the suggested behavior (avoiding continuous polling and only using a one-shot timer while `count === 0`), you should also update `resetViewState()` in this file:
1. At the beginning of `resetViewState()`, add logic like:
- If `!listView.model || listView.model.count === 0`, call `delayedResetTimer.start()` and `return` early.
- Otherwise, perform the normal reset logic immediately.
2. Ensure that no other code calls `delayedResetTimer.start()` in a loop or independently of `resetViewState()`, so the timer is only used as a deferred one-shot when the model is empty.
This will make the reset behavior event-driven from the caller’s perspective and avoid having an always-repeating 50ms polling timer.
</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
+30
to
+34
| Timer { | ||
| id: delayedResetTimer | ||
| interval: 50 | ||
| repeat: true | ||
| onTriggered: { |
There was a problem hiding this comment.
suggestion (performance): Polling with a repeating timer for model.count > 0 could be brittle and wasteful.
This 50ms repeating timer is effectively a tight polling loop on model.count. If the model is late to populate or never does, it runs indefinitely and assumes model.count is always readable. Prefer an event-driven approach, e.g. listening to a model.countChanged-style signal, or using a one-shot Timer you restart from resetViewState() while count === 0 to avoid continuous polling.
Suggested implementation:
// One-shot timer used as a deferred reset trigger. It is started from
// resetViewState() while the model is still empty, instead of polling
// continuously with a repeating timer.
Timer {
id: delayedResetTimer
interval: 50
repeat: false
onTriggered: {
// Only reset once the model is populated; otherwise the caller
// (resetViewState) may choose to reschedule this timer.
if (listView.model && listView.model.count > 0) {
resetViewState()
}
}
}
To fully implement the suggested behavior (avoiding continuous polling and only using a one-shot timer while count === 0), you should also update resetViewState() in this file:
-
At the beginning of
resetViewState(), add logic like:- If
!listView.model || listView.model.count === 0, calldelayedResetTimer.start()andreturnearly. - Otherwise, perform the normal reset logic immediately.
- If
-
Ensure that no other code calls
delayedResetTimer.start()in a loop or independently ofresetViewState(), so the timer is only used as a deferred one-shot when the model is empty.
This will make the reset behavior event-driven from the caller’s perspective and avoid having an always-repeating 50ms polling timer.
BLumia
reviewed
Jan 26, 2026
| } | ||
|
|
||
| function resetViewState() { | ||
| if (listView.model.count === 0) { |
Member
There was a problem hiding this comment.
这个好像不对？listView.model 是 FreeSortProxyModel，不是 DelegateModel，没有 count 属性
Contributor
Author
|
其它PR修复 |
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.
Log: Improved launcher view reset behavior when opening and closing
Influence:
fix: 改进启动器视图状态重置逻辑
Log: 改进启动器打开和关闭时的视图重置行为
Influence:
PMS: BUG-348311
Summary by Sourcery
Improve launcher app list view state reset behavior when the launcher is shown or hidden.
Bug Fixes:
Enhancements: