From a68ec13320e8c0a4a6f8694b488c132a2dd619bd 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 | 34 ++----------------- .../DankBar/Widgets/CpuTemperature.qml | 1 - .../DankBar/Widgets/GpuTemperature.qml | 1 - quickshell/Services/PopoutService.qml | 12 +++---- 5 files changed, 24 insertions(+), 43 deletions(-) diff --git a/quickshell/Modals/ProcessListModal.qml b/quickshell/Modals/ProcessListModal.qml index a313fbd69..0fe2576f8 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 c06b9e88b..7adfbb71f 100644 --- a/quickshell/Modules/DankBar/DankBarContent.qml +++ b/quickshell/Modules/DankBar/DankBarContent.qml @@ -1180,22 +1180,7 @@ Item { parentScreen: barWindow.screen widgetData: parent.widgetData onCpuTempClicked: { - processListPopoutLoader.active = true; - if (!processListPopoutLoader.item) { - return; - } - const effectiveBarConfig = topBarContent.barConfig; - const barPosition = barWindow.axis?.edge === "left" ? 2 : (barWindow.axis?.edge === "right" ? 3 : (barWindow.axis?.edge === "top" ? 0 : 1)); - if (processListPopoutLoader.item.setBarContext) { - processListPopoutLoader.item.setBarContext(barPosition, effectiveBarConfig?.bottomGap ?? 0); - } - if (processListPopoutLoader.item.setTriggerPosition) { - const globalPos = cpuTempWidget.mapToItem(null, 0, 0); - const pos = SettingsData.getPopupTriggerPosition(globalPos, barWindow.screen, barWindow.effectiveBarThickness, cpuTempWidget.width, effectiveBarConfig?.spacing ?? 4, barPosition, effectiveBarConfig); - const widgetSection = topBarContent.getWidgetSection(parent) || "right"; - processListPopoutLoader.item.setTriggerPosition(pos.x, pos.y, pos.width, widgetSection, barWindow.screen, barPosition, barWindow.effectiveBarThickness, effectiveBarConfig?.spacing ?? 4, effectiveBarConfig); - } - PopoutManager.requestPopout(processListPopoutLoader.item, undefined, "cpu_temp"); + PopoutService.toggleProcessListModal(1); } } } @@ -1213,22 +1198,7 @@ Item { parentScreen: barWindow.screen widgetData: parent.widgetData onGpuTempClicked: { - processListPopoutLoader.active = true; - if (!processListPopoutLoader.item) { - return; - } - const effectiveBarConfig = topBarContent.barConfig; - const barPosition = barWindow.axis?.edge === "left" ? 2 : (barWindow.axis?.edge === "right" ? 3 : (barWindow.axis?.edge === "top" ? 0 : 1)); - if (processListPopoutLoader.item.setBarContext) { - processListPopoutLoader.item.setBarContext(barPosition, effectiveBarConfig?.bottomGap ?? 0); - } - if (processListPopoutLoader.item.setTriggerPosition) { - const globalPos = gpuTempWidget.mapToItem(null, 0, 0); - const pos = SettingsData.getPopupTriggerPosition(globalPos, barWindow.screen, barWindow.effectiveBarThickness, gpuTempWidget.width, effectiveBarConfig?.spacing ?? 4, barPosition, effectiveBarConfig); - const widgetSection = topBarContent.getWidgetSection(parent) || "right"; - processListPopoutLoader.item.setTriggerPosition(pos.x, pos.y, pos.width, widgetSection, barWindow.screen, barPosition, barWindow.effectiveBarThickness, effectiveBarConfig?.spacing ?? 4, effectiveBarConfig); - } - PopoutManager.requestPopout(processListPopoutLoader.item, undefined, "gpu_temp"); + PopoutService.toggleProcessListModal(3); } } } 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 b6c1cb163..1c38666d8 100644 --- a/quickshell/Services/PopoutService.qml +++ b/quickshell/Services/PopoutService.qml @@ -708,12 +708,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)); } } @@ -728,12 +728,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 3019d7f7ee1266cb15ebf24d9f64a02bfaf7c5d3 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 0fe2576f8..1bf775b7d 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; } @@ -88,11 +100,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 1c38666d8..fca37ba26 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 @@ -712,8 +714,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)); } } @@ -732,8 +734,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 2e60b85fa46f4c7739609c9e6658a2b1e51d23e2 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 1bf775b7d..730de86a9 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() {