Skip to content

fix: resolve Tab key focus navigation issues in notification center #1369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
deepin-bot merged 1 commit into from
Dec 17, 2025
Merged

fix: resolve Tab key focus navigation issues in notification center #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions panels/notification/center/GroupNotify.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ NotifyItem {
implicitHeight: impl.implicitHeight

signal collapse()
signal gotoNextItem() // Signal to navigate to next notify item
signal gotoPrevItem() // Signal to navigate to previous notify item

// Focus the first button for Tab navigation into group
function focusFirstButton() {
foldBtn.forceActiveFocus()
return true
}

Control {
id: impl
Expand All @@ -34,28 +42,62 @@ NotifyItem {
}

AnimationSettingButton {
id: foldBtn
Layout.alignment: Qt.AlignRight
activeFocusOnTab: false
focusBorderVisible: activeFocus
icon.name: "fold"
text: qsTr("Fold")
Keys.onTabPressed: function(event) {
groupMoreBtn.forceActiveFocus()
event.accepted = true
}
Keys.onBacktabPressed: function(event) {
root.gotoPrevItem()
event.accepted = true
}
onClicked: {
console.log("collapse")
root.collapse()
}
}
AnimationSettingButton {
id: groupMoreBtn
Layout.alignment: Qt.AlignRight
activeFocusOnTab: false
focusBorderVisible: activeFocus
icon.name: "more"
text: qsTr("More")
Keys.onTabPressed: function(event) {
groupClearBtn.forceActiveFocus()
event.accepted = true
}
Keys.onBacktabPressed: function(event) {
foldBtn.forceActiveFocus()
event.accepted = true
}
onClicked: function () {
console.log("group setting", root.appName)
let pos = mapToItem(root, Qt.point(width / 2, height))
root.setting(pos)
}
}
AnimationSettingButton {
id: groupClearBtn
Layout.alignment: Qt.AlignRight
activeFocusOnTab: false
focusBorderVisible: activeFocus
icon.name: "clean-group"
text: qsTr("Clear All")
Keys.onTabPressed: function(event) {
groupClearBtn.focus = false // Clear focus before signal to prevent focus state residue
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting focus to false directly on a control before triggering navigation may not properly clear the focus state. Consider using Qt.callLater to ensure the focus state is cleared after the event is processed, similar to how other navigation code uses Qt.callLater for focus operations.

Copilot uses AI. Check for mistakes.
root.gotoNextItem()
event.accepted = true
}
Keys.onBacktabPressed: function(event) {
groupMoreBtn.forceActiveFocus()
event.accepted = true
}
onClicked: function () {
root.remove()
}
Expand Down
10 changes: 10 additions & 0 deletions panels/notification/center/NormalNotify.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ NotifyItem {
implicitWidth: impl.implicitWidth
implicitHeight: impl.implicitHeight

signal gotoNextItem()
signal gotoPrevItem()

function focusFirstButton() {
return notifyContent.focusFirstButton()
}

Control {
id: impl
anchors.fill: parent

contentItem: NotifyItemContent {
id: notifyContent
width: parent.width
appName: root.appName
iconName: root.iconName
Expand All @@ -42,6 +50,8 @@ NotifyItem {
onActionInvoked: function (actionId) {
root.actionInvoked(actionId)
}
onGotoNextItem: root.gotoNextItem()
onGotoPrevItem: root.gotoPrevItem()
}
}
}
9 changes: 9 additions & 0 deletions panels/notification/center/NotifyCenter.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ FocusScope {
width: NotifyStyle.contentItem.width
notifyModel: notifyModel
z: 1
onGotoFirstNotify: {
if (view.viewCount === 0 || !view.focusItemAtIndex(0)) header.focusFirstButton()
Comment on lines +39 to +40
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition relies on focusItemAtIndex returning false when it can't focus, but that function always returns true when the retry logic is initiated. This means if view.viewCount is greater than 0 but the item at index 0 doesn't exist yet (delegate not created), the header won't be focused even though the retry might fail. Consider checking view.viewCount first and only calling focusItemAtIndex if count is greater than 0, or fix focusItemAtIndex to return a meaningful value.

Copilot uses AI. Check for mistakes.
}
onGotoLastNotify: {
if (view.viewCount === 0) header.focusLastButton()
else view.focusLastItem()
}
}

NotifyView {
Expand All @@ -51,6 +58,8 @@ FocusScope {

height: Math.min(maxViewHeight, viewHeight)
notifyModel: notifyModel
onGotoHeaderFirst: header.focusFirstButton()
onGotoHeaderLast: header.focusLastButton()
}

DropShadowText {
Expand Down
69 changes: 67 additions & 2 deletions panels/notification/center/NotifyHeader.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,38 @@ import org.deepin.ds.notificationcenter

FocusScope {
id: root
activeFocusOnTab: true

required property NotifyModel notifyModel

signal gotoFirstNotify() // Signal to Tab to first notify item
signal gotoLastNotify() // Signal to Shift+Tab to last notify item

// Forward focus to first button when FocusScope receives focus
onActiveFocusChanged: {
if (activeFocus && !collapseBtn.activeFocus && !moreBtn.activeFocus && !clearAllBtn.activeFocus) {
if (collapseBtn.visible) {
collapseBtn.forceActiveFocus()
} else {
moreBtn.forceActiveFocus()
}
}
}

// Focus the first visible button in header for Tab navigation
function focusFirstButton() {
if (collapseBtn.visible) {
collapseBtn.forceActiveFocus()
} else {
moreBtn.forceActiveFocus()
}
}

// Focus the last button in header for Shift+Tab navigation
function focusLastButton() {
clearAllBtn.forceActiveFocus()
}

RowLayout {
anchors.fill: parent
NotifyHeaderTitleText {
Expand All @@ -29,35 +58,71 @@ FocusScope {
}

AnimationSettingButton {
id: collapseBtn
objectName: "collapse"
focus: true
visible: !notifyModel.collapse
Layout.alignment: Qt.AlignRight
activeFocusOnTab: false
focusBorderVisible: activeFocus
icon.name: "fold"
text: qsTr("Fold")
Keys.onTabPressed: function(event) {
moreBtn.forceActiveFocus()
event.accepted = true
}
Keys.onBacktabPressed: function(event) {
root.gotoLastNotify()
event.accepted = true
}
onClicked: function () {
console.log("Collapse all notify")
notifyModel.collapseAllApp()
}
}

AnimationSettingButton {
id: moreBtn
objectName: "more"
focus: true
Layout.alignment: Qt.AlignRight
activeFocusOnTab: false
focusBorderVisible: activeFocus
icon.name: "more"
text: qsTr("More")
Keys.onTabPressed: function(event) {
clearAllBtn.forceActiveFocus()
event.accepted = true
}
Keys.onBacktabPressed: function(event) {
if (collapseBtn.visible) {
collapseBtn.forceActiveFocus()
} else {
root.gotoLastNotify()
}
event.accepted = true
}
onClicked: function () {
console.log("Notify setting")
NotifyAccessor.openNotificationSetting()
}
}

AnimationSettingButton {
id: clearAllBtn
objectName: "closeAllNotify"
activeFocusOnTab: false
focusBorderVisible: activeFocus
icon.name: "clean-all"
text: qsTr("Clear All")
Layout.alignment: Qt.AlignRight
Keys.onTabPressed: function(event) {
clearAllBtn.focus = false // Clear focus before signal to prevent focus state residue
root.gotoFirstNotify()
event.accepted = true
}
Keys.onBacktabPressed: function(event) {
moreBtn.forceActiveFocus()
event.accepted = true
}
onClicked: function () {
console.log("Clear all notify")
notifyModel.clear()
Expand Down
39 changes: 37 additions & 2 deletions panels/notification/center/NotifyView.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,60 @@ Control {
id: root
focus: true

// Maximum retry attempts for focus operations when delegate creation is pending
readonly property int maxFocusRetries: 5

required property NotifyModel notifyModel
property alias viewPanelShown: view.panelShown
readonly property real viewHeight: view.contentHeight
readonly property int viewCount: view.count

signal gotoHeaderFirst() // Signal to cycle Tab back to header first button
signal gotoHeaderLast() // Signal to cycle Shift+Tab back to header last button

NotifySetting {
id: notifySetting
notifyModel: root.notifyModel
}

// Focus notify item at specified index with retry logic for delegate creation
function focusItemAtIndex(idx) {
if (idx < 0 || idx >= view.count) return false
view.currentIndex = idx
view.positionViewAtIndex(idx, ListView.Contain)
function tryFocus(retries) {
let item = view.itemAtIndex(idx)
if (item && item.enabled) {
item.forceActiveFocus()
} else if (retries > 0) {
Qt.callLater(function() { tryFocus(retries - 1) })
}
Comment on lines +36 to +43
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nested function tryFocus is defined inside focusItemAtIndex but may cause confusion about scope. Consider extracting this as a separate function or using a more conventional pattern to make the retry logic clearer and more maintainable.

Copilot uses AI. Check for mistakes.
}
Qt.callLater(function() { tryFocus(root.maxFocusRetries) })
return true
Comment on lines +32 to +46
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The focusItemAtIndex function returns true even when the item cannot be found and retry logic is triggered. This doesn't accurately reflect whether focus was successfully set. Consider tracking whether the retry succeeded and updating the return value accordingly, or document that the return value only indicates whether the attempt was initiated.

Copilot uses AI. Check for mistakes.
}

// Focus the last notify item for Shift+Tab cycling from header
function focusLastItem() {
if (view.count > 0) {
focusItemAtIndex(view.count - 1)
}
}

contentItem: ListView {
id: view
spacing: 10
snapMode: ListView.SnapToItem
// activeFocusOnTab: true
keyNavigationEnabled: false
activeFocusOnTab: false
ScrollBar.vertical: ScrollBar { }
property int nextIndex: -1
property bool panelShown: false


// Forward signals from delegate to root for Tab cycling
function gotoHeaderFirst() { root.gotoHeaderFirst() }
function gotoHeaderLast() { root.gotoHeaderLast() }

onNextIndexChanged: {
if (nextIndex >= 0 && count > 0) {
currentIndex = nextIndex
Expand Down
Loading