feat: replace modal user-confirm dialog with persistent flashing button

Both clients used a modal OK dialog for a Sud step's WAIT_USER confirmation (e.g. "Bitte Malz einfuellen und bestaetigen"). Replaced with a persistent "Confirm" control in the top toolbar/header instead: flashes yellow with the step's message while pending, and modally disables the rest of the GUI (main content plus Start/Pause/Stop/ New/Save/Load, and Connect) until it's clicked - forcing the pending step to be dealt with first. Disabled and blank otherwise. client/brewpi_gui.py: set_user_confirm_pending() drives a QTimer blink (same pattern as the existing "Cool down" indicator) and disables centralwidget + the relevant toolbar QActions individually, so the new button (a toolbar sibling) stays enabled; Start/Pause/Stop are restored via update_sud_actions() rather than a blanket re-enable. web/app.js: setUserConfirmPending() toggles a 'confirm-pending' class (CSS @keyframes flash, matching the file's existing animation-by-class convention) and a 'modally-disabled' class on #layout; also wired into ws.onclose so a dropped connection doesn't leave the page stuck modally disabled on a confirm that'll never resolve. Also fixes a message-ordering race in both clients (found testing the web version on an iPad, connecting fresh while a step was already WAIT_USER): a step's UserMessage is set once when the step starts, well before State later flips to WAIT_USER, so an already-connected client sees them at genuinely different times - but a client connecting fresh gets both bundled into one replayed snapshot, where State's key can sort before UserMessage's. Processing State first checked the pending guard before that same message had set the text, silently failing it and leaving the button looking merely disabled. Both onSudChanged()/ on_sud_changed() now force UserMessage (and, in the web client, Json) to process before State/Step regardless of the bundled dict's key order, same pattern already used for the pre-existing Json-before-Step case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M2ierBoxW3v7nUbDw3M2pE
2026-07-06 11:57:23 +02:00
parent 6d1429daca
commit 3b34f52bb0
4 changed files with 168 additions and 54 deletions
@@ -422,7 +422,7 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow):
 		self.sud_empty = True
 		self.sud_state = None
 		self.sud_user_message = None
-		self._confirm_box = None
+		self.user_confirm_pending = False
 		# Global, not Sud-specific - from the 'System' channel's one-time
 		# startup message. warp_factor defaults to 1.0 (no scaling) until
 		# that arrives, or for a server that doesn't send it at all.
@@ -545,6 +545,24 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow):
 		self.cooling_blink_timer = QtCore.QTimer(self)
 		self.cooling_blink_timer.timeout.connect(self.on_cooling_blink)

+		# Persistent user-confirm control (a Sud step's WAIT_USER, e.g.
+		# "Bitte Malz einfuellen und bestaetigen") - lives in the top
+		# toolbar rather than a transient dialog, so it's visible
+		# regardless of which tab is active. Flashes yellow with the
+		# step's message while pending (same blink-timer pattern as
+		# status_label_cooling above); disabled and blank otherwise.
+		# set_user_confirm_pending() also disables the rest of the GUI
+		# while pending, forcing this button to be dealt with first.
+		self.toolBar.addSeparator()
+		self.label_user_confirm = QtWidgets.QLabel()
+		self.toolBar.addWidget(self.label_user_confirm)
+		self.btn_user_confirm = QtWidgets.QPushButton("Confirm")
+		self.btn_user_confirm.setEnabled(False)
+		self.btn_user_confirm.clicked.connect(self.on_action_user_confirm)
+		self.toolBar.addWidget(self.btn_user_confirm)
+		self.user_confirm_blink_timer = QtCore.QTimer(self)
+		self.user_confirm_blink_timer.timeout.connect(self.on_user_confirm_blink)
+
 		self.update_sud_actions()
 		self.btn_connect.clicked.connect(self.on_btn_connect_clicked)
 		self.actionStart.triggered.connect(self.on_action_sud_start)
@@ -727,7 +745,7 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow):
 			self.sud_empty = True
 			self.sud_state = None
 			self.sud_user_message = None
-			self._close_confirm_dialog()
+			self.set_user_confirm_pending(False)
 			for w in (self.Slider_temp_soll, self.doubleSpinBox_heatrate_soll,
 					  self.Slider_pwr_soll, self.Slider_speed_soll):
 				w.setEnabled(True)
@@ -860,24 +878,47 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow):
 	def on_action_sud_stop(self):
 		self.msg_sud.send({'Stop': True})

-	def show_user_message(self, message):
-		if self._confirm_box is not None:
+	def set_user_confirm_pending(self, pending, message=''):
+		if pending == self.user_confirm_pending:
 			return
-		dlg = QtWidgets.QDialog(self)
-		dlg.setWindowTitle("Sud")
-		dlg.setWindowModality(QtCore.Qt.NonModal)
-		layout = QtWidgets.QVBoxLayout(dlg)
-		layout.addWidget(QtWidgets.QLabel(message))
-		btn = QtWidgets.QPushButton("OK")
-		btn.clicked.connect(lambda: self.msg_sud.send({'Confirm': True}))
-		layout.addWidget(btn)
-		self._confirm_box = dlg
-		dlg.show()
+		self.user_confirm_pending = pending
+		if pending:
+			self.label_user_confirm.setText(message)
+			self.btn_user_confirm.setEnabled(True)
+			self.btn_user_confirm.setStyleSheet("background-color: yellow; font-weight: bold;")
+			self.user_confirm_blink_timer.start(500)
+			# Modally disabled: nothing else in the GUI should be usable
+			# until the pending step is actually confirmed - centralwidget
+			# covers every tab's content; the toolbar/menu actions are
+			# disabled individually (rather than disabling self.toolBar
+			# itself) so btn_user_confirm, a sibling widget on the same
+			# toolbar, stays enabled.
+			self.centralwidget.setEnabled(False)
+			for action in (self.actionStart, self.actionPause, self.actionStop,
+						   self.actionSudNew, self.actionSudSave, self.actionSudLoad):
+				action.setEnabled(False)
+		else:
+			self.user_confirm_blink_timer.stop()
+			self.btn_user_confirm.setStyleSheet("")
+			self.btn_user_confirm.setEnabled(False)
+			self.label_user_confirm.setText('')
+			self.centralwidget.setEnabled(True)
+			self.actionSudNew.setEnabled(True)
+			self.actionSudSave.setEnabled(True)
+			self.actionSudLoad.setEnabled(True)
+			# Start/Pause/Stop depend on sud_state/sud_empty, not just
+			# "was disabled for confirm" - let the usual logic pick the
+			# right one back up rather than assuming all three should
+			# simply return to enabled.
+			self.update_sud_actions()

-	def _close_confirm_dialog(self):
-		if self._confirm_box is not None:
-			self._confirm_box.close()
-			self._confirm_box = None
+	def on_user_confirm_blink(self):
+		yellow = self.btn_user_confirm.styleSheet() == ""
+		self.btn_user_confirm.setStyleSheet(
+			"background-color: yellow; font-weight: bold;" if yellow else "")
+
+	def on_action_user_confirm(self):
+		self.msg_sud.send({'Confirm': True})

 	def on_action_sud_new(self):
 		# Sends an empty schedule via the same Load path Load Sud uses, with
@@ -1013,7 +1054,22 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow):
 		# Receiving anything at all on this channel means a Sud is loaded
 		# server-side - the schedule it's running may still be coming.
 		self.sud_loaded = True
-		for key in msg:
+		# UserMessage must be processed before State: a step's user_message
+		# is set once, when the step starts (components/sud.py's
+		# _advance()), well before state later flips to WAIT_USER
+		# (_finish_step()) - so UserMessage's key was inserted into the
+		# server's global_state earlier in its lifetime and would precede
+		# State there. A client connecting fresh while a step is already
+		# WAIT_USER gets both bundled into this same replay dict; processing
+		# State first would check self.sud_user_message before this very
+		# message has set it, silently failing set_user_confirm_pending()'s
+		# guard and leaving the Confirm button looking merely disabled
+		# instead of pending - see web/app.js's onSudChanged() for the twin
+		# fix (same server, same race).
+		ordered_keys = ['UserMessage'] + [k for k in msg if k != 'UserMessage']
+		for key in ordered_keys:
+			if key not in msg:
+				continue
 			if "Json" in key:
 				if self.sud_save_path:
 					with open(self.sud_save_path, 'w') as f:
@@ -1078,9 +1134,9 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow):
 						self.forecast_plot.show_computing(self.sud_name)

 				if self.sud_state == SUD_WAIT_USER_STATE and prev_state != SUD_WAIT_USER_STATE and self.sud_user_message:
-					self.show_user_message(self.sud_user_message)
+					self.set_user_confirm_pending(True, self.sud_user_message)
 				elif prev_state == SUD_WAIT_USER_STATE and self.sud_state != SUD_WAIT_USER_STATE:
-					self._close_confirm_dialog()
+					self.set_user_confirm_pending(False)
 				self._update_step_plates()
 			elif "UserMessage" in key:
 				self.sud_user_message = msg['UserMessage']
@@ -582,12 +582,34 @@ function updateClosedLoopStatus() {
 	statusEl.className = `panel-status ${closedLoop ? 'status-connected' : 'status-disconnected'}`;
 }

-// Mirrors client/brewpi_gui.py's show_user_message(): the Sud is already
-// blocked in WAIT_USER server-side - dismissing this dialog (OK or Escape,
-// both fire the dialog's 'close' event) is what unblocks it.
-function showUserMessage(message) {
-	document.getElementById('sud-message-text').textContent = message;
-	document.getElementById('sud-message-dialog').showModal();
+// Mirrors client/brewpi_gui.py's set_user_confirm_pending(): a persistent
+// header control rather than a modal dialog. While pending, the Confirm
+// button flashes yellow with the step's message and #layout (everything
+// except the header) is modally disabled via the 'modally-disabled' class
+// (opacity + pointer-events:none - see style.css) so the pending step has
+// to be dealt with before anything else. The Sud is already blocked in
+// WAIT_USER server-side; clicking Confirm is what unblocks it.
+let userConfirmPending = false;
+const CONFIRM_DISABLE_IDS = ['btn-connect', 'btn-sud-start', 'btn-sud-pause', 'btn-sud-stop',
+	'btn-sud-new', 'btn-sud-load', 'btn-sud-save'];
+
+function setUserConfirmPending(pending, message = '') {
+	if (pending === userConfirmPending) return;
+	userConfirmPending = pending;
+	const btn = document.getElementById('btn-user-confirm');
+	document.getElementById('user-confirm-text').textContent = pending ? message : '';
+	btn.disabled = !pending;
+	btn.classList.toggle('confirm-pending', pending);
+	document.getElementById('layout').classList.toggle('modally-disabled', pending);
+	for (const id of CONFIRM_DISABLE_IDS) {
+		document.getElementById(id).disabled = pending;
+	}
+	if (!pending) {
+		// Start/Pause/Stop depend on sudState/sudEmpty/connected, not just
+		// "was disabled for confirm" - let the usual logic pick the right
+		// one back up rather than assuming all should simply re-enable.
+		updateSudActions();
+	}
 }

 function downloadJson(doc) {
@@ -755,7 +777,19 @@ function onSudChanged(msg) {
 	// Processing Step first would set sudStepIndex before the schedule
 	// exists, then Json would reset it to null (isNewSchedule=true on
 	// first/reloaded connect) with no subsequent Step to restore it.
-	const keys = ['Json', ...Object.keys(msg).filter(k => k !== 'Json')];
+	//
+	// UserMessage must likewise be processed before State: a step's
+	// user_message is set once, when the step starts (components/sud.py's
+	// _advance()), well before state later flips to WAIT_USER
+	// (_finish_step()) - so UserMessage's key was inserted into
+	// global_state earlier in the server's lifetime and would otherwise
+	// precede State here too. A client connecting fresh mid-confirm gets
+	// both bundled into this same replay message; processing State first
+	// would check sudUserMessage before this very message has set it,
+	// silently failing setUserConfirmPending()'s guard and leaving the
+	// Confirm button looking merely disabled instead of pending - the
+	// same race the old modal dialog had, just never noticed until now.
+	const keys = ['Json', 'UserMessage', ...Object.keys(msg).filter(k => k !== 'Json' && k !== 'UserMessage')];
 	for (const key of keys) {
 		if (!(key in msg)) continue;
 		if (key === 'Json') {
@@ -791,10 +825,9 @@ function onSudChanged(msg) {
 			sudState = newState;
 			updateControlsEnabled();
 			if (newState === WAIT_USER_STATE && prevState !== WAIT_USER_STATE && sudUserMessage) {
-				showUserMessage(sudUserMessage);
+				setUserConfirmPending(true, sudUserMessage);
 			} else if (prevState === WAIT_USER_STATE && newState !== WAIT_USER_STATE) {
-				const dlg = document.getElementById('sud-message-dialog');
-				if (dlg.open) dlg.close();
+				setUserConfirmPending(false);
 			}
 			updateStepPlates();
 			updateSudActions();
@@ -945,6 +978,11 @@ function connect() {
 		for (const id of ['temp-soll', 'heatrate-soll', 'heater-power', 'stirrer-speed', 'closed-loop']) {
 			document.getElementById(id).disabled = false;
 		}
+		// Nothing reported over a dead connection can be trusted anymore -
+		// don't leave the page modally stuck on a pending confirm that'll
+		// never resolve now (mirrors client/brewpi_gui.py's disconnect
+		// handling).
+		setUserConfirmPending(false);
 		setConnected(false);
 	};
 	ws.onmessage = (event) => {
@@ -1053,8 +1091,7 @@ document.getElementById('btn-sud-pause').addEventListener('click', () => {
 document.getElementById('btn-sud-stop').addEventListener('click', () => {
 	sendMsg('Sud', {Stop: true});
 });
-document.getElementById('sud-message-dialog').addEventListener('cancel', e => e.preventDefault());
-document.getElementById('sud-message-ok').addEventListener('click', () => {
+document.getElementById('btn-user-confirm').addEventListener('click', () => {
 	sendMsg('Sud', {Confirm: true});
 });

@@ -30,6 +30,10 @@
 		<input type="file" id="sud-file-input" accept="application/json" class="hidden">
 		<button id="btn-sud-save">Save&hellip;</button>
 	</div>
+	<div id="user-confirm-row">
+		<span id="user-confirm-text"></span>
+		<button id="btn-user-confirm" type="button" disabled>Confirm</button>
+	</div>
 </header>

 <main id="layout">
@@ -286,13 +290,6 @@

 </main>

-<dialog id="sud-message-dialog">
-	<p id="sud-message-text"></p>
-	<form>
-		<button id="sud-message-ok" type="button">OK</button>
-	</form>
-</dialog>
-
 <footer>
 	<div id="sud-status-line"></div>
 	<div id="env-status-line"></div>
@@ -11,6 +11,7 @@
    --orange:      #ffa726;
    --orange-dim:  #6d3b00;
    --red:         #f44336;
+    --yellow:      #ffeb3b;
    --led-off:     #444444;
    --led-green:       var(--green);
    --led-green-dim:   var(--green-dim);
@@ -80,6 +81,43 @@ header#connection-bar {
 }
 #host { width: 18em; }

+/* --- User-confirm control (replaces a modal OK dialog) --- */
+
+#user-confirm-row {
+    display: flex;
+    align-items: center;
+    gap: 0.6em;
+}
+#user-confirm-text {
+    color: var(--yellow);
+    font-weight: bold;
+}
+#btn-user-confirm {
+    font-weight: bold;
+    border: none;
+    border-radius: 4px;
+    padding: 0.4em 1em;
+    cursor: pointer;
+    background: var(--surface-2);
+    color: var(--text-muted);
+}
+#btn-user-confirm.confirm-pending {
+    color: #000;
+    cursor: pointer;
+    animation: confirm-flash 1s step-start infinite;
+}
+@keyframes confirm-flash {
+    0%, 49%   { background: var(--surface-2); }
+    50%, 100% { background: var(--yellow); }
+}
+
+/* Modally disabled while a user-confirm is pending - #user-confirm-row
+   (and its Confirm button) stays outside #layout so it's untouched. */
+#layout.modally-disabled {
+    opacity: 0.5;
+    pointer-events: none;
+}
+
 .status-connected    { color: var(--green); font-weight: bold; }
 .status-disconnected { color: var(--text-muted); }

@@ -392,20 +430,6 @@ header#connection-bar {
 }
 .step-plate .span2 { grid-column: span 2; }

-/* --- Message dialog --- */
-
-#sud-message-dialog {
-    background: var(--surface);
-    color: var(--text);
-    border: 1px solid var(--border);
-    border-radius: 8px;
-    padding: 1.2em 1.6em;
-    max-width: 24em;
-    box-shadow: 0 8px 32px rgba(0,0,0,0.7);
-}
-#sud-message-dialog::backdrop { background: rgba(0,0,0,0.65); }
-#sud-message-dialog form { margin-top: 1em; text-align: right; }
-
 /* --- Footer --- */

 footer {