From 8916b14f0e0d93a0e7ad7c7b25a3a4747647b294 Mon Sep 17 00:00:00 2001 From: Nate Graham Date: Thu, 22 Apr 2021 10:01:14 -0600 Subject: [PATCH] [applets/battery] Clarify power management inhibition UI The power management inhibition UI is currently somewhat confusing to users because it's not clear that the checkbox is a local override for the permanent settings set elsewhere in System Settings. A good sign that the UI is sub-optimal is that we refer to it as "power management inhibition" internally and in our developer conversations, but the UI expresses the opposite: *allowing* power management, not inhibiting it. This conflicts in the user's mind with the UI in System Settings that is also expressed in terms of allowing it. It is further confused by the fact that the message about apps suppressing power management is phrased in terms of being an temporary override, and that we also show an unrelated message about the battery charge limit (if set) in the same place where we're notifying the user about power management inhibition. Let's clarify this UI by doing the following: - Re-word the checkbox's label to emphasize that it's a local override for the settings that were set elsewhere - Move the charge level message into the battery item itself, since that's what it affects - Slightly re-word the message for when apps are inhibiting power management to emphasize that power management can be inhibited automatically by apps in additionto manually by the user BUG: 435827 FIXED-IN: 5.22 --- .../package/contents/ui/BatteryItem.qml | 13 +++++ .../package/contents/ui/PopupDialog.qml | 4 +- .../contents/ui/PowerManagementItem.qml | 47 ++++++++++--------- .../package/contents/ui/batterymonitor.qml | 30 ++++++------ 4 files changed, 54 insertions(+), 40 deletions(-) diff --git a/applets/batterymonitor/package/contents/ui/BatteryItem.qml b/applets/batterymonitor/package/contents/ui/BatteryItem.qml index fd21f175a..4818a8a90 100644 --- a/applets/batterymonitor/package/contents/ui/BatteryItem.qml +++ b/applets/batterymonitor/package/contents/ui/BatteryItem.qml @@ -208,6 +208,19 @@ Item { opacity: 0.5 sourceComponent: batteryDetails } + + InhibitionHint { + anchors { + left: parent.left + leftMargin: batteryIcon.width + PlasmaCore.Units.gridUnit + right: parent.right + } + readonly property var chargeStopThreshold: pmSource.data["Battery"] ? pmSource.data["Battery"]["Charge Stop Threshold"] : undefined + readonly property bool pluggedIn: pmSource.data["AC Adapter"] !== undefined && pmSource.data["AC Adapter"]["Plugged in"] + visible: pluggedIn && typeof chargeStopThreshold === "number" && chargeStopThreshold > 0 && chargeStopThreshold < 100 + iconSource: "kt-speed-limits" // FIXME good icon + text: i18n("Your battery is configured to only charge up to %1%.", chargeStopThreshold || 0) + } } } diff --git a/applets/batterymonitor/package/contents/ui/PopupDialog.qml b/applets/batterymonitor/package/contents/ui/PopupDialog.qml index c962aaa86..68d8a7bc7 100644 --- a/applets/batterymonitor/package/contents/ui/PopupDialog.qml +++ b/applets/batterymonitor/package/contents/ui/PopupDialog.qml @@ -35,14 +35,14 @@ PlasmaComponents3.Page { property bool isBrightnessAvailable property bool isKeyboardBrightnessAvailable - signal powermanagementChanged(bool checked) + signal powermanagementChanged(bool disabled) header: PlasmaExtras.PlasmoidHeading { PowerManagementItem { id: pmSwitch width: parent.width pluggedIn: dialog.pluggedIn - onEnabledChanged: powermanagementChanged(enabled) + onDisabledChanged: powermanagementChanged(disabled) KeyNavigation.tab: batteryList KeyNavigation.backtab: keyboardBrightnessSlider } diff --git a/applets/batterymonitor/package/contents/ui/PowerManagementItem.qml b/applets/batterymonitor/package/contents/ui/PowerManagementItem.qml index 93e99e27f..1a69b8a03 100644 --- a/applets/batterymonitor/package/contents/ui/PowerManagementItem.qml +++ b/applets/batterymonitor/package/contents/ui/PowerManagementItem.qml @@ -25,8 +25,8 @@ import org.kde.plasma.components 3.0 as PlasmaComponents3 import org.kde.kquickcontrolsaddons 2.0 ColumnLayout { - id: powerManagementHeadingColumn - property alias enabled: pmCheckBox.checked + id: powerManagement + property alias disabled: pmCheckBox.checked property bool pluggedIn spacing: 0 @@ -38,8 +38,8 @@ ColumnLayout { PlasmaComponents3.CheckBox { id: pmCheckBox Layout.fillWidth: true - text: i18nc("Minimize the length of this string as much as possible", "Allow automatic sleep and screen locking") - checked: true + text: i18nc("Minimize the length of this string as much as possible", "Inhibit automatic sleep and screen locking") + checked: false } } @@ -50,42 +50,43 @@ ColumnLayout { InhibitionHint { Layout.fillWidth: true - readonly property var chargeStopThreshold: pmSource.data["Battery"] ? pmSource.data["Battery"]["Charge Stop Threshold"] : undefined - visible: powerManagementHeadingColumn.pluggedIn && typeof chargeStopThreshold === "number" && chargeStopThreshold > 0 && chargeStopThreshold < 100 - iconSource: "kt-speed-limits" // FIXME good icon - text: i18n("Your battery is configured to only charge up to %1%.", chargeStopThreshold || 0) + visible: pmSource.data["PowerDevil"] && pmSource.data["PowerDevil"]["Is Lid Present"] && !pmSource.data["PowerDevil"]["Triggers Lid Action"] ? true : false + iconSource: "computer-laptop" + text: i18nc("Minimize the length of this string as much as possible", "Your notebook is configured not to sleep when closing the lid while an external monitor is connected.") } + + // UI to display when there is only one inhibition InhibitionHint { + // Don't need to show the inhibitions when power management + // isn't enabled anyway + visible: inhibitions.length === 1 && !powerManagement.disabled Layout.fillWidth: true - visible: pmSource.data["PowerDevil"] && pmSource.data["PowerDevil"]["Is Lid Present"] && !pmSource.data["PowerDevil"]["Triggers Lid Action"] ? true : false - iconSource: "computer-laptop" - text: i18nc("Minimize the length of this string as much as possible", "Your notebook is configured not to sleep when closing the lid while an external monitor is connected.") + iconSource: inhibitions[0].Icon || "" + text: inhibitions[0].Reason ? + i18n("%1 is inhibiting sleep and screen locking (%2)", inhibitions[0].Name, inhibitions[0].Reason) + : i18n("%1 is inhibiting sleep and screen locking (unknown reason)", inhibitions[0].Name) } + + // UI to display when there is more than one inhibition PlasmaComponents3.Label { id: inhibitionExplanation Layout.fillWidth: true // Don't need to show the inhibitions when power management // isn't enabled anyway - visible: inhibitions.length > 0 && pmCheckBox.checked + visible: inhibitions.length > 1 && !powerManagement.disabled font: PlasmaCore.Theme.smallestFont wrapMode: Text.WordWrap elide: Text.ElideRight maximumLineCount: 3 - text: { - if (inhibitions.length === 1) { - return i18n("An application is preventing sleep and screen locking:") - } else if (inhibitions.length > 1) { - return i18np("%1 application is preventing sleep and screen locking:", - "%1 applications are preventing sleep and screen locking:", - inhibitions.length) - } else { - return "" - } - } + text: i18np("%1 application is inhibiting sleep and screen locking:", + "%1 applications are inhibiting sleep and screen locking:", + inhibitions.length) } Repeater { + visible: inhibitions.length > 1 + model: inhibitionExplanation.visible ? inhibitions.length : null InhibitionHint { diff --git a/applets/batterymonitor/package/contents/ui/batterymonitor.qml b/applets/batterymonitor/package/contents/ui/batterymonitor.qml index e47564e8f..71ba2d3ce 100644 --- a/applets/batterymonitor/package/contents/ui/batterymonitor.qml +++ b/applets/batterymonitor/package/contents/ui/batterymonitor.qml @@ -223,39 +223,39 @@ Item { property int cookie2: -1 onPowermanagementChanged: { var service = pmSource.serviceForSource("PowerDevil"); - if (checked) { - var op1 = service.operationDescription("stopSuppressingSleep"); - op1.cookie = cookie1; - var op2 = service.operationDescription("stopSuppressingScreenPowerManagement"); - op2.cookie = cookie2; + if (disabled) { + var reason = i18n("The battery applet has enabled system-wide inhibition"); + var op1 = service.operationDescription("beginSuppressingSleep"); + op1.reason = reason; + var op2 = service.operationDescription("beginSuppressingScreenPowerManagement"); + op2.reason = reason; var job1 = service.startOperationCall(op1); job1.finished.connect(function(job) { - cookie1 = -1; + cookie1 = job.result; }); var job2 = service.startOperationCall(op2); job2.finished.connect(function(job) { - cookie2 = -1; + cookie2 = job.result; }); } else { - var reason = i18n("The battery applet has enabled system-wide inhibition"); - var op1 = service.operationDescription("beginSuppressingSleep"); - op1.reason = reason; - var op2 = service.operationDescription("beginSuppressingScreenPowerManagement"); - op2.reason = reason; + var op1 = service.operationDescription("stopSuppressingSleep"); + op1.cookie = cookie1; + var op2 = service.operationDescription("stopSuppressingScreenPowerManagement"); + op2.cookie = cookie2; var job1 = service.startOperationCall(op1); job1.finished.connect(function(job) { - cookie1 = job.result; + cookie1 = -1; }); var job2 = service.startOperationCall(op2); job2.finished.connect(function(job) { - cookie2 = job.result; + cookie2 = -1; }); } - batterymonitor.powermanagementDisabled = !checked + batterymonitor.powermanagementDisabled = disabled } } }