Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gugullll 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 GuideThe lockscreen power panel is refactored around ShutdownView, combining power and session actions while wiring each operation to GreeterProxy/UserModel state. Opening the panel now focuses the first visible, enabled action, with updated cross-action keyboard navigation, dismissal behavior, build configuration, and translations. Sequence diagram for opening the lockscreen power panelsequenceDiagram
participant User
participant ControlAction
participant ShutdownView
participant GreeterProxy
participant UserModel
User->>ControlAction: onClicked
ControlAction->>ShutdownView: focusFirstButton()
ShutdownView->>GreeterProxy: isLocked
ShutdownView->>GreeterProxy: canPowerOff
ShutdownView->>GreeterProxy: canReboot
ShutdownView->>GreeterProxy: canSuspend
ShutdownView->>GreeterProxy: canHibernate
ShutdownView->>UserModel: count
ShutdownView-->>User: Focus first visible and enabled action
Flow diagram for consolidated lockscreen power actionsflowchart LR
ControlAction --> ShutdownView
ShutdownView --> PowerOff["GreeterProxy.powerOff()"]
ShutdownView --> Reboot["GreeterProxy.reboot()"]
ShutdownView --> Suspend["GreeterProxy.suspend()"]
ShutdownView --> Hibernate["GreeterProxy.hibernate()"]
ShutdownView --> Lock["GreeterProxy.lock()"]
ShutdownView --> SwitchUser["UserModel.count and switchUser()"]
ShutdownView --> Logout["GreeterProxy.logout()"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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/plugins/lockscreen/qml/ControlAction.qml" line_range="82-90" />
<code_context>
- Item {
- id: powerList
+ ShutdownView {
+ id: powerShutdownView
parent: rootItem
visible: powerItem.expand
width: rootItem.width
height: rootItem.height
x: 0
y: 0
-
- // Click outside the PowerList to close
- MouseArea {
- anchors.fill: parent
- onClicked: {
- powerItem.expand = false
- innerPowerList.loopInside = false
- }
- }
-
- PowerList {
- id: innerPowerList
- width: rootItem.width
- height: 140
- x: 0
- y: rootItem.height / 5 * 2
- }
+ onOutsideClicked: powerItem.expand = false
}
onClicked: {
</code_context>
<issue_to_address>
**issue (bug_risk):** The `ShutdownView` embedded in `ControlAction` emits `switchUser()` when its switch-user button is clicked, but this instance has no `onSwitchUser` handler connected to the existing `otherUserRequested` or login switch-user flow. Clicking Switch User in this power panel therefore performs no action.
**Triggers:** When the power panel is opened while the greeter is unlocked.
**Suggested fix:** Connect `onSwitchUser` to the appropriate `otherUserRequested()`/user-switch handler, or keep the switch-user action only in the top-level `LoginView` shutdown view.
</issue_to_address>
### Comment 2
<location path="src/plugins/lockscreen/qml/ShutdownView.qml" line_range="83" />
<code_context>
+ text: qsTr("Hibernate")
+ icon.name: "login_hibernate"
+ onClicked: GreeterProxy.hibernate()
+ KeyNavigation.tab: lockBtn
+ KeyNavigation.backtab: suspendBtn
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** The fixed `KeyNavigation` links target buttons that can be invisible or disabled: in the locked state `hibernateBtn` navigates to invisible `lockBtn`, and in the unlocked state `lockBtn` navigates through potentially disabled `switchBtn`. Keyboard focus consequently stops or leaves the intended actionable-button sequence instead of skipping unavailable buttons.
**Triggers:** When a power capability is unavailable, or when the greeter is locked and the lock/switch/logout buttons are hidden.
**Suggested fix:** Build navigation dynamically from the same visible-and-enabled button list used by `focusFirstButton()`, or explicitly link each button to the next available target in both directions.
</issue_to_address>d42cb24 to
2a212d4
Compare
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugins/lockscreen/qml/ShutdownView.qml" line_range="53-54" />
<code_context>
- text: qsTr("Shut Down")
- icon.name: "login_shutdown"
- onClicked: GreeterProxy.powerOff()
- KeyNavigation.tab: rebootBtn
- KeyNavigation.backtab: hibernateBtn
- Keys.onBacktabPressed: function(event) {
</code_context>
<issue_to_address>
**issue (bug_risk):** The tab cycle points directly to buttons that can be hidden or disabled instead of dynamically skipping them. For example, `hibernateBtn` tabs to hidden `lockBtn` while locked, and `logoutBtn` backtabs to disabled or hidden power buttons, so keyboard focus stops or lands on an unavailable action.
**Triggers:** When a capability disables a power button, or when `GreeterProxy.isLocked` hides the session buttons.
**Suggested fix:** Use navigation logic that selects the next visible and enabled button, or update the navigation targets whenever button visibility or enabled state changes.
</issue_to_address>Merge PowerList into ShutdownView with shutdown, reboot, suspend, hibernate and lock/switch/logout buttons. Adjust focus logic to skip hidden or disabled buttons when opening the power panel. 将关机面板重构成ShutdownView,支持关机/重启/挂起/休眠 及锁屏,切换用户/注销操作,移除原PowerList组件。打开 电源面板时聚焦逻辑会跳过不可见或不可用的按钮。 Log: 重构锁屏关机面板并增加电源操作按钮 PMS: TASK-390751 Influence: 锁屏关机界面提供关机、重启、挂起、休眠等完整 电源操作;电源面板打开时聚焦第一个可见且可用的按钮。交互 和键盘焦点导航相应调整。
2a212d4 to
415a0c3
Compare
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugins/lockscreen/qml/ShutdownView.qml" line_range="53-114" />
<code_context>
+ KeyNavigation.tab: rebootBtn
</code_context>
<issue_to_address>
**issue (bug_risk):** Keyboard navigation points directly to buttons that are hidden when disabled or when the lock state changes, so Tab or Backtab stops instead of moving to the next available action. For example, when switching users is unavailable, `lockBtn` navigates to the invisible `switchBtn` rather than `logoutBtn`, and `logoutBtn` navigates to the potentially invisible `powerOffBtn`.
**Triggers:** When one or more power capabilities are unavailable or `UserModel.count` is less than two.
**Suggested fix:** Update the tab and backtab targets dynamically, or implement navigation that searches for the next visible and enabled button before assigning focus.
</issue_to_address>| KeyNavigation.tab: rebootBtn | ||
| KeyNavigation.backtab: logoutBtn | ||
| } | ||
|
|
||
| ShutdownButton { | ||
| id: rebootBtn | ||
| enabled: GreeterProxy.canReboot | ||
| text: qsTr("Reboot") | ||
| icon.name: "login_reboot" | ||
| onClicked: GreeterProxy.reboot() | ||
| KeyNavigation.tab: suspendBtn | ||
| KeyNavigation.backtab: powerOffBtn | ||
| } | ||
|
|
||
| ShutdownButton { | ||
| id: suspendBtn | ||
| enabled: GreeterProxy.canSuspend | ||
| text: qsTr("Suspend") | ||
| icon.name: "login_suspend" | ||
| onClicked: GreeterProxy.suspend() | ||
| KeyNavigation.tab: hibernateBtn | ||
| KeyNavigation.backtab: rebootBtn | ||
| } | ||
|
|
||
| ShutdownButton { | ||
| id: hibernateBtn | ||
| enabled: GreeterProxy.canHibernate | ||
| text: qsTr("Hibernate") | ||
| icon.name: "login_hibernate" | ||
| onClicked: GreeterProxy.hibernate() | ||
| KeyNavigation.tab: lockBtn | ||
| KeyNavigation.backtab: suspendBtn | ||
| } | ||
|
|
||
| ShutdownButton { | ||
| id: lockBtn | ||
| visible: !GreeterProxy.isLocked | ||
| text: qsTr("lock") | ||
| icon.name: "login_lock" | ||
| onClicked: GreeterProxy.lock() | ||
| KeyNavigation.tab: switchBtn | ||
| KeyNavigation.backtab: hibernateBtn | ||
| } | ||
|
|
||
| ShutdownButton { | ||
| id: switchBtn | ||
| visible: !GreeterProxy.isLocked | ||
| text: qsTr("switch user") | ||
| icon.name: "login_switchuser" | ||
| enabled: UserModel.count > 1 | ||
| onClicked: root.switchUser() | ||
| KeyNavigation.tab: logoutBtn | ||
| KeyNavigation.backtab: lockBtn | ||
| } | ||
|
|
||
| ShutdownButton { | ||
| id: logoutBtn | ||
| visible: !GreeterProxy.isLocked | ||
| text: qsTr("Logout") | ||
| icon.name: "login_logout" | ||
| onClicked: GreeterProxy.logout() | ||
| KeyNavigation.tab: powerOffBtn |
There was a problem hiding this comment.
issue (bug_risk): Keyboard navigation points directly to buttons that are hidden when disabled or when the lock state changes, so Tab or Backtab stops instead of moving to the next available action. For example, when switching users is unavailable, lockBtn navigates to the invisible switchBtn rather than logoutBtn, and logoutBtn navigates to the potentially invisible powerOffBtn.
Triggers: When one or more power capabilities are unavailable or UserModel.count is less than two.
Suggested fix: Update the tab and backtab targets dynamically, or implement navigation that searches for the next visible and enabled button before assigning focus.
Merge PowerList into ShutdownView with shutdown, reboot, suspend, hibernate and lock/switch/logout buttons. Adjust focus logic to skip hidden or disabled buttons when opening the power panel.
将关机面板重构成ShutdownView,支持关机/重启/挂起/休眠
及锁屏,切换用户/注销操作,移除原PowerList组件。打开
电源面板时聚焦逻辑会跳过不可见或不可用的按钮。
Log: 重构锁屏关机面板并增加电源操作按钮
PMS: TASK-390751
Influence: 锁屏关机界面提供关机、重启、挂起、休眠等完整
电源操作;电源面板打开时聚焦第一个可见且可用的按钮。交互
和键盘焦点导航相应调整。
Summary by Sourcery
Consolidate lock-screen power controls into ShutdownView and provide complete power-action access with robust focus handling.
New Features:
Bug Fixes:
Enhancements:
Chores: