feat: set focus to first expanded notification item #1316
Merged
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAutomatically move focus to the first expanded notification item by computing its index and updating ListView’s currentIndex when an app group is expanded. Sequence diagram for expanding an app group and setting focus to first notification itemsequenceDiagram
participant User as actor User
participant "AppGroupDelegate"
participant "notifyModel"
participant "root.view (ListView)"
User->>"AppGroupDelegate": Expand app group
"AppGroupDelegate"->>"notifyModel": expandApp(expandIndex)
alt root.view is available
"AppGroupDelegate"->>"root.view (ListView)": Set currentIndex = expandIndex + 1
end
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 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> `panels/notification/center/NotifyViewDelegate.qml:169-172` </location>
<code_context>
+ notifyModel.expandApp(expandIndex)
+ // 设置ListView的currentIndex到新展开的首条通知项 (将焦点设置到新展开的通知的第一项)
+ if (root.view) {
+ let firstExpandedIndex = expandIndex + 1
+ root.view.currentIndex = firstExpandedIndex
+ }
}
</code_context>
<issue_to_address>
**suggestion:** Consider edge cases where expandIndex + 1 may exceed the model bounds.
Ensure firstExpandedIndex does not exceed the model's length before assigning it to root.view.currentIndex to prevent out-of-bounds errors.
```suggestion
if (root.view) {
let firstExpandedIndex = expandIndex + 1
if (firstExpandedIndex < notifyModel.count) {
root.view.currentIndex = firstExpandedIndex
}
}
```
</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
169
to
182
| if (root.view) { | ||
| let firstExpandedIndex = expandIndex + 1 | ||
| root.view.currentIndex = firstExpandedIndex | ||
| } |
There was a problem hiding this comment.
suggestion: Consider edge cases where expandIndex + 1 may exceed the model bounds.
Ensure firstExpandedIndex does not exceed the model's length before assigning it to root.view.currentIndex to prevent out-of-bounds errors.
Suggested change
| if (root.view) { | |
| let firstExpandedIndex = expandIndex + 1 | |
| root.view.currentIndex = firstExpandedIndex | |
| } | |
| if (root.view) { | |
| let firstExpandedIndex = expandIndex + 1 | |
| if (firstExpandedIndex < notifyModel.count) { | |
| root.view.currentIndex = firstExpandedIndex | |
| } | |
| } |
18202781743
reviewed
Oct 27, 2025
| { | ||
| console.log("expand") | ||
| notifyModel.expandApp(model.index) | ||
| let expandIndex = model.index |
Contributor
There was a problem hiding this comment.
这个需不需要弄一个全局nextIndex的呀，我看展开，折叠，删除，都可能会需要currentIndex指向正确的index，
Contributor
Author
There was a problem hiding this comment.
似乎不需要，他折叠和删除，默认listview的焦点行为会给到下一个，焦点不会乱跳
wjyrich
force-pushed
the
fix-bug-284987-2
branch
2 times, most recently
from
33af38e to
ce6eac8
Compare
18202781743
approved these changes
Oct 28, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
1. Add logic to automatically set ListView currentIndex to the first notification item when expanding an app group 2. This improves keyboard navigation by moving focus to the newly expanded content 3. Calculate the correct index by adding 1 to the expanded app index 4. Only apply the focus change when the root view is available feat: 设置焦点到新展开的首条通知项 1. 添加逻辑在展开应用组时自动将 ListView 当前索引设置到第一条通知项 2. 通过将焦点移动到新展开的内容来改进键盘导航体验 3. 通过展开应用索引加1计算正确的索引位置 4. 仅在根视图可用时应用焦点更改 5.修复了删除后，焦点给到下一项 PMS: BUG-284987
wjyrich
force-pushed
the
fix-bug-284987-2
branch
from
ce6eac8 to
c3a358e
Compare
deepin pr auto review我来帮你审查这段代码的修改。主要改动涉及通知列表项的删除和展开操作时的索引处理。让我从几个方面分析：
改进建议：
property int nextIndex: -1
onNextIndexChanged: {
if (nextIndex >= 0 && count > 0) {
// 确保nextIndex在有效范围内
currentIndex = Math.min(nextIndex, count - 1)
}
}
onRemove: function () {
console.log("remove normal", model.id)
if (!model.id) return // 添加id有效性检查
let removeIndex = index
notifyModel.remove(model.id)
// 确保删除后选中项的合理性
root.view.nextIndex = removeIndex >= root.view.count ? removeIndex - 1 : removeIndex
}
onDismiss: function () {
console.log("dismiss normal", model.id)
if (!model.id) return // 添加id有效性检查
let dismissIndex = index
notifyModel.remove(model.id)
// 确保删除后选中项的合理性
root.view.nextIndex = dismissIndex >= root.view.count ? dismissIndex - 1 : dismissIndex
}
onExpand: function () {
console.log("expand")
if (!model.index && model.index !== 0) return // 添加index有效性检查
let expandIndex = model.index
notifyModel.expandApp(expandIndex)
// 确保展开后的索引在有效范围内
root.view.nextIndex = Math.min(expandIndex + 1, root.view.count - 1)
}主要改进点：
这些改进可以使代码更加健壮，提高用户体验，并减少潜在的bug。 |
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.
feat: 设置焦点到新展开的首条通知项
PMS: BUG-284987
Summary by Sourcery
Set ListView.currentIndex to the first item of an expanded notification group when the root view is available to streamline keyboard navigation after expanding an app group
New Features:
Enhancements: