From 8a0a09dbb4aae709baed9809e36ae857a16a998a Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Tue, 18 Aug 2026 10:08:12 -0400 Subject: [PATCH] Fix System Health ledger rendering --- .../modules/settings/HealthCheckRow.qml | 17 +++- .../modules/settings/HealthPage.qml | 87 +++++++++++++++---- .../modules/settings/SettingsShell.qml | 6 +- tests/quickshell/health-ui-contract.sh | 40 ++++++--- 4 files changed, 116 insertions(+), 34 deletions(-) diff --git a/config/dot/quickshell/modules/settings/HealthCheckRow.qml b/config/dot/quickshell/modules/settings/HealthCheckRow.qml index eafd193..eb0020e 100644 --- a/config/dot/quickshell/modules/settings/HealthCheckRow.qml +++ b/config/dot/quickshell/modules/settings/HealthCheckRow.qml @@ -8,11 +8,22 @@ Item { required property var check property bool issue: false property bool divider: true + property int actionActivationCount: 0 signal actionRequested(var check) - objectName: `health-check-row:${root.check.id}:${root.issue ? "issue" : "ledger"}` + objectName: `health-check-row:${root.issue ? "issue" : "quiet"}:${root.check.id}` implicitHeight: 62 + // The isolated contract uses the same signal path as a pointer or keyboard + // activation instead of calling HealthPage's action handler directly. + function activateAction(): bool { + if (!actionButton.visible || !actionButton.enabled) + return false; + root.actionActivationCount += 1; + actionButton.clicked(); + return true; + } + function statusLabel(status: string): string { if (status === "ok") return "Healthy"; if (status === "warning") return "Needs attention"; @@ -78,7 +89,7 @@ Item { spacing: 12 Text { - objectName: `health-status-text:${root.check.id}:${root.issue ? "issue" : "ledger"}` + objectName: `health-status-text:${root.issue ? "issue" : "quiet"}:${root.check.id}` anchors.verticalCenter: parent.verticalCenter text: root.statusLabel(root.check.status) color: root.statusColor(root.check.status) @@ -89,7 +100,7 @@ Item { SettingsButton { id: actionButton - objectName: `health-row-action:${root.check.id}:${root.issue ? "issue" : "ledger"}` + objectName: `health-row-action:${root.issue ? "issue" : "quiet"}:${root.check.id}` visible: root.check.action !== undefined text: Health.repairingId === root.check.id ? "Working…" : (root.check.action?.label ?? "") enabled: visible && Health.repairingId !== root.check.id && !Health.busy diff --git a/config/dot/quickshell/modules/settings/HealthPage.qml b/config/dot/quickshell/modules/settings/HealthPage.qml index 0e22505..36ccdcd 100644 --- a/config/dot/quickshell/modules/settings/HealthPage.qml +++ b/config/dot/quickshell/modules/settings/HealthPage.qml @@ -51,7 +51,8 @@ SettingsPage { } function checksForGroup(group: string): var { - return Health.checks.filter(check => check.group === group); + return Health.checks.filter(check => check.group === group + && (check.status === "ok" || check.status === "unconfigured")); } function handleAction(check: var): void { @@ -101,28 +102,70 @@ SettingsPage { return matches; } + function activateRenderedAction(id: string): bool { + const suffix = `:${id}`; + const rows = root.descendants(root, "health-check-row:").filter(row => + row.visible && String(row.objectName).endsWith(suffix)); + return rows.length === 1 && rows[0].activateAction(); + } + + function renderedFocusChain(): var { + // Repeater delegates enter Qt's tab chain lazily after their enabled + // binding changes at the end of a scan. Touch each rendered action's + // real next-focus link before traversing from the first hero control. + const renderedActions = root.descendants(root, "health-row-action:").filter(item => + item.visible && item.enabled && item.activeFocusOnTab); + for (const action of renderedActions) + action.nextItemInFocusChain(true); + + const starts = root.descendants(root, "health-copy-report-button").filter(item => item.visible && item.enabled); + if (starts.length !== 1) + return []; + + const names = []; + const start = starts[0]; + let current = start; + for (let index = 0; index < 128; index++) { + const name = String(current.objectName || ""); + if (name !== "" && names.indexOf(name) < 0) + names.push(name); + current = current.nextItemInFocusChain(true); + if (!current || current === start) + break; + } + return names; + } + // Deterministic, read-only fixture seam used by the offscreen contract. function uiDiagnostics(): var { const summaries = root.descendants(root, "health-summary"); const rows = root.descendants(root, "health-check-row:").filter(row => row.visible); - const copyButtons = root.descendants(root, "health-copy-report-button"); - const refreshButtons = root.descendants(root, "health-refresh-button"); - const rowActions = root.descendants(root, "health-row-action:").filter(button => button.visible); const checkingLabels = root.descendants(root, "health-checking-label").filter(label => label.visible); + const confirmationSheets = root.descendants(root, "health-confirmation-sheet:").filter(sheet => sheet.visible); + const emptyGroups = root.descendants(root, "health-empty-group:").filter(label => label.visible); return { - issueIds: root.issueChecks.map(check => check.id), - desktopIds: root.checksForGroup("desktop-foundation").map(check => check.id), - integrationIds: root.checksForGroup("integrations").map(check => check.id), - statusLabels: Health.checks.map(check => root.statusLabel(check.status)), + renderedRows: rows.map(row => { + const objectName = String(row.objectName); + const parts = objectName.split(":"); + const statusTexts = root.descendants(row, "health-status-text:").filter(text => text.visible); + return { + objectName: objectName, + id: parts.slice(2).join(":"), + section: parts[1], + statusText: statusTexts.length === 1 ? statusTexts[0].text : "" + }; + }), summaryHeight: summaries.length > 0 ? summaries[0].height : 0, rowHeights: rows.map(row => row.height), checking: Health.busy, checkingText: checkingLabels.length > 0 ? checkingLabels[0].text : "", - copyFocusable: copyButtons.length > 0 && copyButtons[0].activeFocusOnTab, - refreshFocusable: refreshButtons.length > 0 && refreshButtons[0].activeFocusOnTab, - rowActionFocusable: rowActions.length > 0 && rowActions[0].activeFocusOnTab, - confirmationVisible: root.pendingConfirmation !== null, - confirmationId: root.pendingConfirmation ? root.pendingConfirmation.id : "" + focusChain: root.renderedFocusChain(), + activatedRows: rows.filter(row => row.actionActivationCount > 0).map(row => String(row.objectName)), + emptyQuietGroups: emptyGroups.map(label => String(label.objectName).slice("health-empty-group:".length)), + confirmationVisible: confirmationSheets.length === 1, + confirmationId: confirmationSheets.length === 1 + ? String(confirmationSheets[0].objectName).slice("health-confirmation-sheet:".length) + : "" }; } @@ -130,7 +173,7 @@ SettingsPage { header: Component { Rectangle { - objectName: "health-confirmation-sheet" + objectName: `health-confirmation-sheet:${root.pendingConfirmation ? root.pendingConfirmation.id : ""}` visible: root.pendingConfirmation !== null implicitHeight: visible ? confirmRow.implicitHeight + 28 : 0 radius: Theme.cardRadius @@ -295,6 +338,7 @@ SettingsPage { SettingsCard { required property var modelData + readonly property var quietChecks: root.checksForGroup(modelData.group) width: groupGrid.columns === 2 ? (groupGrid.width - groupGrid.columnSpacing) / 2 @@ -306,9 +350,22 @@ SettingsPage { id: groupRows width: parent.width + Text { + objectName: `health-empty-group:${modelData.group}` + width: parent.width + visible: quietChecks.length === 0 + text: "Items needing attention are listed above." + color: Theme.fgMuted + font.family: Theme.fontFamily + font.pixelSize: Theme.fontSizeSmall + topPadding: 8 + bottomPadding: 8 + wrapMode: Text.WordWrap + } + Repeater { id: groupRepeater - model: root.checksForGroup(modelData.group) + model: quietChecks HealthCheckRow { required property var modelData diff --git a/config/dot/quickshell/modules/settings/SettingsShell.qml b/config/dot/quickshell/modules/settings/SettingsShell.qml index 5fae1af..9e41d79 100644 --- a/config/dot/quickshell/modules/settings/SettingsShell.qml +++ b/config/dot/quickshell/modules/settings/SettingsShell.qml @@ -22,11 +22,7 @@ Rectangle { || !pageLoader.item || pageLoader.item.objectName !== "system-health-page") return false; - const check = Health.checks.find(candidate => candidate.id === id); - if (!check) - return false; - pageLoader.item.handleAction(check); - return true; + return pageLoader.item.activateRenderedAction(id); } color: Theme.bg diff --git a/tests/quickshell/health-ui-contract.sh b/tests/quickshell/health-ui-contract.sh index d8a4704..faf5406 100755 --- a/tests/quickshell/health-ui-contract.sh +++ b/tests/quickshell/health-ui-contract.sh @@ -120,9 +120,11 @@ ShellRoot { Health.consumeSnapshot(Quickshell.env("PANAMA_HEALTH_FIXTURE"), 100); } - Item { - width: 980 - height: 820 + FloatingWindow { + title: "Panama Health UI Contract" + visible: true + implicitWidth: 980 + implicitHeight: 820 SettingsShell { id: settingsShell @@ -185,12 +187,19 @@ run ipc show 2>/dev/null | rg -q '^target health-ui-test$' \ checking_state="$(run ipc call health-ui-test state)" jq -e ' - .issueIds == ["desktop.vicinae", "desktop.quickshell", "integration.calendar"] - and .desktopIds == ["desktop.vicinae", "desktop.quickshell"] - and .integrationIds == ["integration.bluebubbles", "integration.calendar"] - and .statusLabels == ["Needs attention", "Action required", "Healthy", "Not set up", "Needs attention", "Healthy"] + .renderedRows == [ + {objectName:"health-check-row:issue:desktop.vicinae", id:"desktop.vicinae", section:"issue", statusText:"Needs attention"}, + {objectName:"health-check-row:issue:desktop.quickshell", id:"desktop.quickshell", section:"issue", statusText:"Action required"}, + {objectName:"health-check-row:issue:integration.calendar", id:"integration.calendar", section:"issue", statusText:"Needs attention"}, + {objectName:"health-check-row:quiet:input.pipewire", id:"input.pipewire", section:"quiet", statusText:"Healthy"}, + {objectName:"health-check-row:quiet:integration.bluebubbles", id:"integration.bluebubbles", section:"quiet", statusText:"Not set up"}, + {objectName:"health-check-row:quiet:panama.caffeine", id:"panama.caffeine", section:"quiet", statusText:"Healthy"} + ] + and (.renderedRows | map(.id) | length) == 6 + and (.renderedRows | map(.id) | unique | length) == 6 + and .emptyQuietGroups == ["desktop-foundation"] and .summaryHeight == 126 - and (.rowHeights | length) == 9 + and (.rowHeights | length) == 6 and (.rowHeights | all(. >= 62)) and .checking == true and .checkingText == "Checking…" @@ -202,15 +211,24 @@ for _ in $(seq 1 40); do [[ "$(jq -r .checking <<<"$settled_state")" == "false" ]] && break sleep 0.1 done +sleep 0.1 +settled_state="$(run ipc call health-ui-test state)" [[ "$(jq -c .rowHeights <<<"$settled_state")" == "$checking_heights" ]] \ || fail 'row geometry changed after refresh settled' -jq -e '.copyFocusable == true and .refreshFocusable == true and .rowActionFocusable == true' \ - >/dev/null <<<"$settled_state" || fail 'keyboard focus does not reach hero and row actions' +jq -e ' + (.focusChain | index("health-copy-report-button")) != null + and (.focusChain | index("health-refresh-button")) != null + and (.focusChain | any(startswith("health-row-action:"))) +' >/dev/null <<<"$settled_state" || fail "actual focus-chain traversal does not reach hero and row actions: $settled_state" [[ "$(run ipc call health-ui-test request desktop.quickshell)" == "true" ]] \ || fail 'restart confirmation fixture could not be requested' confirmation_state="$(run ipc call health-ui-test state)" -jq -e '.confirmationVisible == true and .confirmationId == "desktop.quickshell"' \ +jq -e ' + .confirmationVisible == true + and .confirmationId == "desktop.quickshell" + and .activatedRows == ["health-check-row:issue:desktop.quickshell"] +' \ >/dev/null <<<"$confirmation_state" || fail 'Quickshell restart did not open confirmation sheet' if rg -i 'QQml|ReferenceError|TypeError|binding loop|failed to load component' "$shell_log"; then