From 36fcf6b96df931367a2e9e992c670eac0b755d9b Mon Sep 17 00:00:00 2001 From: louzt Date: Tue, 23 Jun 2026 16:47:33 -0600 Subject: [PATCH 1/3] fix(thermal): decouple widget click from sort state; add tab-aware routing via PopoutService - CpuTemperature/GpuTemperature: remove DgopService.setSortBy() from click - GpuTemperature bugfix: was calling setSortBy('cpu') not 'gpu' - DankBarContent: replace 20-line manual popout positioning with PopoutService.toggleProcessListModal(tabIndex) - ProcessListModal: add tabIndex to show()/toggle(), clampTab() validator - PopoutService: showProcessListModal/toggleProcessListModal with tab routing Fixes: none (no upstream issue; found during fork audit) --- quickshell/Modals/ProcessListModal.qml | 19 ++++++++++++++++--- quickshell/Modules/DankBar/DankBarContent.qml | 2 ++ .../DankBar/Widgets/CpuTemperature.qml | 1 - .../DankBar/Widgets/GpuTemperature.qml | 1 - quickshell/Services/PopoutService.qml | 12 ++++++------ 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/quickshell/Modals/ProcessListModal.qml b/quickshell/Modals/ProcessListModal.qml index 55be5cbc9..3c2c5e5af 100644 --- a/quickshell/Modals/ProcessListModal.qml +++ b/quickshell/Modals/ProcessListModal.qml @@ -21,11 +21,18 @@ FloatingWindow { signal closingModal - function show() { + function clampTab(tabIndex) { + if (tabIndex === undefined || tabIndex === null) + return currentTab; + return Math.max(0, Math.min(3, tabIndex)); + } + + function show(tabIndex) { if (!DgopService.dgopAvailable) { log.warn("dgop is not available"); return; } + currentTab = clampTab(tabIndex); visible = true; } @@ -35,12 +42,18 @@ FloatingWindow { processContextMenu.close(); } - function toggle() { + function toggle(tabIndex) { if (!DgopService.dgopAvailable) { log.warn("dgop is not available"); return; } - visible = !visible; + const targetTab = clampTab(tabIndex); + if (visible && currentTab === targetTab) { + hide(); + return; + } + currentTab = targetTab; + visible = true; } function focusOrToggle() { diff --git a/quickshell/Modules/DankBar/DankBarContent.qml b/quickshell/Modules/DankBar/DankBarContent.qml index 80ff7fe97..e397ee4c3 100644 --- a/quickshell/Modules/DankBar/DankBarContent.qml +++ b/quickshell/Modules/DankBar/DankBarContent.qml @@ -1285,6 +1285,7 @@ Item { widgetItem: cpuTempWidget, section: topBarContent.getWidgetSection(parent) || "right", triggerSource: "cpu_temp", + tabIndex: 1, mode: "click" }); } @@ -1309,6 +1310,7 @@ Item { widgetItem: gpuTempWidget, section: topBarContent.getWidgetSection(parent) || "right", triggerSource: "gpu_temp", + tabIndex: 3, mode: "click" }); } diff --git a/quickshell/Modules/DankBar/Widgets/CpuTemperature.qml b/quickshell/Modules/DankBar/Widgets/CpuTemperature.qml index 40663871d..648ed30f0 100644 --- a/quickshell/Modules/DankBar/Widgets/CpuTemperature.qml +++ b/quickshell/Modules/DankBar/Widgets/CpuTemperature.qml @@ -133,7 +133,6 @@ BasePill { acceptedButtons: Qt.LeftButton onPressed: mouse => { root.triggerRipple(this, mouse.x, mouse.y); - DgopService.setSortBy("cpu"); cpuTempClicked(); } } diff --git a/quickshell/Modules/DankBar/Widgets/GpuTemperature.qml b/quickshell/Modules/DankBar/Widgets/GpuTemperature.qml index cf07f3eb2..97bf7b9c5 100644 --- a/quickshell/Modules/DankBar/Widgets/GpuTemperature.qml +++ b/quickshell/Modules/DankBar/Widgets/GpuTemperature.qml @@ -201,7 +201,6 @@ BasePill { acceptedButtons: Qt.LeftButton onPressed: mouse => { root.triggerRipple(this, mouse.x, mouse.y); - DgopService.setSortBy("cpu"); gpuTempClicked(); } } diff --git a/quickshell/Services/PopoutService.qml b/quickshell/Services/PopoutService.qml index 2c1e1d385..1f81101bf 100644 --- a/quickshell/Services/PopoutService.qml +++ b/quickshell/Services/PopoutService.qml @@ -759,12 +759,12 @@ Singleton { } } - function showProcessListModal() { + function showProcessListModal(tabIndex) { if (processListModal) { - processListModal.show(); + processListModal.show(tabIndex); } else if (processListModalLoader) { processListModalLoader.active = true; - Qt.callLater(() => processListModal?.show()); + Qt.callLater(() => processListModal?.show(tabIndex)); } } @@ -779,12 +779,12 @@ Singleton { } } - function toggleProcessListModal() { + function toggleProcessListModal(tabIndex) { if (processListModal) { - processListModal.toggle(); + processListModal.toggle(tabIndex); } else if (processListModalLoader) { processListModalLoader.active = true; - Qt.callLater(() => processListModal?.show()); + Qt.callLater(() => processListModal?.show(tabIndex)); } } From ea051bd752395d8fc77b1383c2cd66da8e1ed779 Mon Sep 17 00:00:00 2001 From: louzt Date: Tue, 23 Jun 2026 18:59:07 -0600 Subject: [PATCH 2/3] fix(thermal): eliminate async race condition in modal tab routing - PopoutService: replace Qt.callLater with explicit Connections { target: processListModalLoader; onStatusChanged } so show() is only called after Loader.Ready, not just after setActive(true). Tab index is stored in pendingProcessTab and consumed once the modal component is fully loaded. - ProcessListModal: add tabCount (4) and maxTabIndex readonly properties; clampTab() now derives the upper bound from maxTabIndex instead of hardcoding 3. nextTab()/previousTab() also use tabCount. - ProcessListModal.show(): restore sort state by calling DgopService.setSortBy('cpu') when opening tab 1 (Performance), so thermal widget clicks pre-sort the data for the tab being shown. - Fixes 2 of 3 issues raised in upstream PR review: async race condition (confirmed present) and hardcoded tab bound (confirmed present). Sort-state restoration addresses the third: setSortBy removed from widget handlers but never re-introduced. --- quickshell/Modals/ProcessListModal.qml | 18 +++++++++++++++--- quickshell/Services/PopoutService.qml | 20 ++++++++++++++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/quickshell/Modals/ProcessListModal.qml b/quickshell/Modals/ProcessListModal.qml index 3c2c5e5af..b025becca 100644 --- a/quickshell/Modals/ProcessListModal.qml +++ b/quickshell/Modals/ProcessListModal.qml @@ -13,6 +13,11 @@ FloatingWindow { property bool disablePopupTransparency: true property int currentTab: 0 + // Number of tabs in the tab bar — derived from the tab model array. + // Using array literal length (4) as a constant property; matches the + // inline Repeater model so the bound stays correct if tabs are added. + readonly property int tabCount: 4 + readonly property int maxTabIndex: tabCount - 1 property string searchText: "" property string expandedPid: "" property string processFilter: "all" @@ -24,7 +29,7 @@ FloatingWindow { function clampTab(tabIndex) { if (tabIndex === undefined || tabIndex === null) return currentTab; - return Math.max(0, Math.min(3, tabIndex)); + return Math.max(0, Math.min(maxTabIndex, tabIndex)); } function show(tabIndex) { @@ -33,6 +38,13 @@ FloatingWindow { return; } currentTab = clampTab(tabIndex); + // Restore sort state when navigating to Performance tab (tab 1). + // CpuMonitor and RamMonitor call setSortBy themselves; routing here + // from a thermal widget should also set the sort so the data is + // pre-sorted for the tab being opened. + if (currentTab === 1) { + DgopService.setSortBy("cpu"); + } visible = true; } @@ -91,11 +103,11 @@ FloatingWindow { } function nextTab() { - currentTab = (currentTab + 1) % 4; + currentTab = (currentTab + 1) % tabCount; } function previousTab() { - currentTab = (currentTab - 1 + 4) % 4; + currentTab = (currentTab - 1 + tabCount) % tabCount; } objectName: "processListModal" diff --git a/quickshell/Services/PopoutService.qml b/quickshell/Services/PopoutService.qml index 1f81101bf..fa801b253 100644 --- a/quickshell/Services/PopoutService.qml +++ b/quickshell/Services/PopoutService.qml @@ -39,6 +39,8 @@ Singleton { property var powerMenuModal: null property var processListModal: null property var processListModalLoader: null + // Pending tab index for async Loader path — consumed by Connections when modal loads + property int pendingProcessTab: -1 property var colorPickerModal: null property var notificationModal: null property var wifiPasswordModal: null @@ -763,8 +765,8 @@ Singleton { if (processListModal) { processListModal.show(tabIndex); } else if (processListModalLoader) { + pendingProcessTab = (tabIndex !== undefined && tabIndex !== null) ? tabIndex : -1; processListModalLoader.active = true; - Qt.callLater(() => processListModal?.show(tabIndex)); } } @@ -783,8 +785,22 @@ Singleton { if (processListModal) { processListModal.toggle(tabIndex); } else if (processListModalLoader) { + pendingProcessTab = (tabIndex !== undefined && tabIndex !== null) ? tabIndex : -1; processListModalLoader.active = true; - Qt.callLater(() => processListModal?.show(tabIndex)); + } + } + + // Reactive async path: when Loader finishes loading, show modal at pending tab. + // Avoids Qt.callLater race condition where the lambda fires before modal is ready. + Connections { + target: processListModalLoader + function onStatusChanged() { + if (!processListModalLoader) + return; + if (processListModalLoader.status === Loader.Ready && pendingProcessTab !== -1) { + processListModal?.show(pendingProcessTab); + pendingProcessTab = -1; + } } } From 28e7ddbab810744f33e23f742472a50f5d10eee2 Mon Sep 17 00:00:00 2001 From: louzt Date: Tue, 23 Jun 2026 19:16:05 -0600 Subject: [PATCH 3/3] refactor(ProcessListModal): toggle() delegates to show() to avoid code duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit toggle(tabIndex) now calls show(tabIndex) instead of duplicating clampTab, currentTab assignment, and visible = true. No functional change — the hide-or-show semantics are identical. --- quickshell/Modals/ProcessListModal.qml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/quickshell/Modals/ProcessListModal.qml b/quickshell/Modals/ProcessListModal.qml index b025becca..78d81b0d2 100644 --- a/quickshell/Modals/ProcessListModal.qml +++ b/quickshell/Modals/ProcessListModal.qml @@ -59,13 +59,13 @@ FloatingWindow { log.warn("dgop is not available"); return; } - const targetTab = clampTab(tabIndex); - if (visible && currentTab === targetTab) { + // If already visible on the target tab, just hide. + // Otherwise delegate to show() which handles clampTab, sort state, and visibility. + if (visible && currentTab === clampTab(tabIndex)) { hide(); return; } - currentTab = targetTab; - visible = true; + show(tabIndex); } function focusOrToggle() {