Notifications: Don't display 'acknowledge all' after clicking 'silenc… #1349

DanielMcInnes · 2024-07-23T07:13:47Z

…e alarm'

blammit · 2024-07-25T04:11:22Z

pages/NotificationsPage.qml

-		onClicked: Global.notifications.acknowledgeAll()
+		onClicked: {
+			// break bindings so that color, text and image don't change while fading out
+			backgroundColor = backgroundColor


Won't this cause problems the next time an alarm or alert occurs, and the binding is required again?

Would it be easier to create two separate buttons, one for alerts and one for alarms?

Yes it does cause problems. Updated as suggested.

…e alarm' Fixes #1348

DanielMcInnes · 2024-07-30T04:09:44Z

components/NotificationButton.qml

+		right: parent ? parent.rightSideRow.right : undefined
+		verticalCenter: parent.verticalCenter
+	}
+	parent: !!Global.pageManager ? Global.pageManager.statusBar : undefined


The button used to be parented to Global.pageManager.statusBar.rightSideRow, I had to change to to Global.pageManager.statusBar and anchor it to the RHS of Global.pageManager.statusBar.rightSideRow, as Row doesn't re-layout itself in a timely manner when its children's visibility changes.

What happens if other buttons (aside from Display Sleep etc) get added to the rightSideRow?

If we want to add other stuff to rightSideRow, we will have to revisit this. It would depend how the new button is supposed to behave.

What about moving the buttons to StatusBar.qml and defining the NotificationButton component inline (like how StatusBarButton is defined there)? That way the buttons are defined directly within the layout in StatusBar.qml, and we wouldn't need awkward alias properties like Global.pageManager.statusBar.rightSideRow.

Updated as suggested.

blammit

In mock mode, I pressed Shift+N to generate a new alarm. gui-v2 jumped to the Notifications page and showed the "Silence alarms" button, but then it quickly disappeared and was replaced by "Acknowledge alerts". I think I'd expect "Silence alarms" to take precedence if both alerts and alarms are present.

Also, if I click "Silence alarms" or "Acknowledge alerts", the button jumps slightly to the left before it disappears.

blammit · 2024-07-31T01:31:17Z

components/NotificationButton.qml

+	opacity: enabled ? 1 : 0
+	Behavior on opacity { OpacityAnimator { duration: Theme.animation_toastNotification_fade_duration } }
+
+	contentItem: Row {


Button already supports showing an icon and text side-by-side, so it should be possible to do:

NotificationButton { icon.source: "qrc:/images/icon_alarm_snooze_24.svg" text: "Silence alarm" }

instead of overriding the contentItem.

updated as suggested

This has the side effect of the button text color changing while the button is pressed, as per our default Button behaviour. Looks ok to me.

blammit · 2024-07-31T01:40:50Z

components/NotificationButton.qml

+		right: parent ? parent.rightSideRow.right : undefined
+		verticalCenter: parent.verticalCenter
+	}
+	parent: !!Global.pageManager ? Global.pageManager.statusBar : undefined


What about moving the buttons to StatusBar.qml and defining the NotificationButton component inline (like how StatusBarButton is defined there)? That way the buttons are defined directly within the layout in StatusBar.qml, and we wouldn't need awkward alias properties like Global.pageManager.statusBar.rightSideRow.

DanielMcInnes · 2024-07-31T04:10:25Z

In mock mode, I pressed Shift+N to generate a new alarm. gui-v2 jumped to the Notifications page and showed the "Silence alarms" button, but then it quickly disappeared and was replaced by "Acknowledge alerts". I think I'd expect "Silence alarms" to take precedence if both alerts and alarms are present.

Also, if I click "Silence alarms" or "Acknowledge alerts", the button jumps slightly to the left before it disappears.

Re. the "Silence alarms" button disappearing and being replaced by "Acknowledge alerts", I saw this as well, it is caused by a timer in the mock NotificationsImpl that sets .../Active to null. If you turn off the timers by pressing 'T', you shouldn't see it any more.

Re. the button jumping slightly to the left, I don't see this. I'm hoping it is caused by the mock timers deactivating alarms - would you mind disabling the mock timers and checking again?

blammit · 2024-08-01T04:06:47Z

Re. the button jumping slightly to the left, I don't see this. I'm hoping it is caused by the mock timers deactivating alarms - would you mind disabling the mock timers and checking again?

It still occurs. But if it doesn't occur with real alarms on device/wasm, then it LGTM.

DanielMcInnes · 2024-08-02T04:35:04Z

Re. the button jumping slightly to the left, I don't see this. I'm hoping it is caused by the mock timers deactivating alarms - would you mind disabling the mock timers and checking again?

It still occurs. But if it doesn't occur with real alarms on device/wasm, then it LGTM.

I managed to reproduce this on a cerbo. Fixed now.

blammit

Works well, some minor comments. LGTM

blammit · 2024-08-05T03:19:20Z

components/StatusBar.qml

+		}
+		enabled: notificationButtonsEnabled && !!Global.notifications && Global.notifications.alarm
+		backgroundColor: Theme.color_critical_background
+		display: C.AbstractButton.TextBesideIcon


TextBesideIcon is the default, so this is not needed

blammit · 2024-08-05T03:26:42Z

components/StatusBar.qml

+	Rectangle {
+		anchors.fill: rightSideRow
+		color: "pink"
+	}


Leftover debug?

ugh, whoops.

blammit · 2024-08-05T03:28:49Z

components/StatusBar.qml

@@ -14,7 +15,8 @@ Rectangle {

 	property int leftButton: VenusOS.StatusBar_LeftButton_None
 	property int rightButton: VenusOS.StatusBar_RightButton_None
-	property alias rightSideRow: rightSideRow
+	property bool notificationButtonsEnabled


I wonder if the value of notificationButtonsEnabled can just be set from within StatusBar.qml (e.g. if it could find the current page url somehow to check whether it's the NotificationsPage.qml) to avoid needing to set this from MainView.qml and needing the extra property in SwipePageModel. But no big deal either way.

updated as suggested

blammit · 2024-08-05T03:29:12Z

components/StatusBar.qml

@@ -5,6 +5,7 @@

 import QtQuick
 import QtQuick.Controls as C
+import QtQuick.Controls.impl as CP


No longer needed?

DanielMcInnes linked an issue Jul 23, 2024 that may be closed by this pull request

Notifications page: When "silence alarm" is clicked, the "Acknowledge all" button is briefly displayed #1348

Closed

blammit reviewed Jul 25, 2024

View reviewed changes

DanielMcInnes force-pushed the 1348-notifications-page-when-silence-alarm-is-clicked-the-acknowledge-all-button-is-briefly-displayed branch from 06240a9 to ce6985f Compare July 30, 2024 01:18

Notifications: Don't display 'acknowledge all' after clicking 'silenc…

9e1645b

…e alarm' Fixes #1348

DanielMcInnes force-pushed the 1348-notifications-page-when-silence-alarm-is-clicked-the-acknowledge-all-button-is-briefly-displayed branch from ce6985f to 9e1645b Compare July 30, 2024 04:04

DanielMcInnes commented Jul 30, 2024

View reviewed changes

blammit reviewed Jul 31, 2024

View reviewed changes

Refactor - move notification buttons into StatusBar

066f674

Refactor NotificationButton - don't override contentItem

89d9231

DanielMcInnes force-pushed the 1348-notifications-page-when-silence-alarm-is-clicked-the-acknowledge-all-button-is-briefly-displayed branch from e4a941b to 89d9231 Compare August 2, 2024 03:35

blammit approved these changes Aug 5, 2024

View reviewed changes

Disable ScreenBlanker while notification buttons are animating

09eab2c

DanielMcInnes force-pushed the 1348-notifications-page-when-silence-alarm-is-clicked-the-acknowledge-all-button-is-briefly-displayed branch from 7457eb4 to 09eab2c Compare August 5, 2024 04:36

DanielMcInnes merged commit 6d423dc into main Aug 6, 2024
2 checks passed

DanielMcInnes deleted the 1348-notifications-page-when-silence-alarm-is-clicked-the-acknowledge-all-button-is-briefly-displayed branch August 6, 2024 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notifications: Don't display 'acknowledge all' after clicking 'silenc… #1349

Notifications: Don't display 'acknowledge all' after clicking 'silenc… #1349

DanielMcInnes commented Jul 23, 2024

blammit Jul 25, 2024

DanielMcInnes Jul 30, 2024

DanielMcInnes Jul 30, 2024

chriadam Jul 30, 2024

DanielMcInnes Jul 30, 2024

blammit Jul 31, 2024

DanielMcInnes Jul 31, 2024

blammit left a comment

blammit Jul 31, 2024

DanielMcInnes Jul 31, 2024

DanielMcInnes Jul 31, 2024

blammit Jul 31, 2024

DanielMcInnes commented Jul 31, 2024

blammit commented Aug 1, 2024

DanielMcInnes commented Aug 2, 2024

blammit left a comment

blammit Aug 5, 2024

DanielMcInnes Aug 5, 2024

blammit Aug 5, 2024

DanielMcInnes Aug 5, 2024

blammit Aug 5, 2024

DanielMcInnes Aug 5, 2024

blammit Aug 5, 2024

DanielMcInnes Aug 5, 2024

Notifications: Don't display 'acknowledge all' after clicking 'silenc… #1349

Notifications: Don't display 'acknowledge all' after clicking 'silenc… #1349

Conversation

DanielMcInnes commented Jul 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blammit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielMcInnes commented Jul 31, 2024

blammit commented Aug 1, 2024

DanielMcInnes commented Aug 2, 2024

blammit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment