fix(thermal): decouple widget click from sort state; add tab-aware modal routing#2690
fix(thermal): decouple widget click from sort state; add tab-aware modal routing#2690louzt wants to merge 3 commits into
Conversation
…uting 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)
- 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.
…e duplication 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.
This comment was marked as spam.
This comment was marked as spam.
@Spoch-dev | Since this is a draft to benchmark the thermal routing behavior, I'm open to code suggestions. Specifically, what boundaries do you recommend for the |
Hey @louzt, we have no idea who that user is and why they went to all our open PR's to state "closing", that was not us so you can ignore them. Having said that, unless we're specifically prompted, we do not actively review Draft's unless we see it's going in the wrong direction or we have no plans to implement. @bbedward may have some feedback on what you asked the user there. |
|
Thanks for the clarification, @Purian23. Glad to know it was just noise. I was a bit skeptical at first about what could be wrong with the proposal, since I do my best to investigate thoroughly, structure valid points, and adapt on review each time i PR something, not equal as having the right maintainer vision and priors on your side ofc. To give you some context, I am a daily operator of DMS on my main workstation. Because I highly respect your core baseline and architectural choices, I maintain a local stack of experimental performance shims, layout optimizations, and custom daemon-routing implementations tailored to my workflows. Over time, pulling and adapting upstream commits to keep my tree aligned (until oficial release) gave me the confidence to continue contributing back cleanly. Since this specific PR introduces a semantic behavior change by routing CPU and GPU thermal clicks directly to the Performance and System tabs, I intentionally opened it as a Draft to benchmark the execution, and waits for conflicts to fix them // and invite design feedback from @bbedward or the team when time permits as other times. There is absolutely no rush. I will rebase my branch locally to resolve the current conflict in DankBarContent.qml so the code remains clean and building blocks don't drift. Anyways, this patch/code can serve as a solid historical reference for the repository or as a starting point to align with future layout iterations, even if the final state is close it and open other solution. Looking forward to any architecture notes whenever you guys have the bandwidth or interest in this or future PRs. |
|
I am fully aware of the current conflict in DankBarContent.qml caused by the new upstream openWidgetPopout abstraction, as well as the theoretical idempotency edge cases if a user spams fast consecutive clicks before the loader lifecycle finishes. I intentionally left this pull request as a Draft to sync on the architectural direction first. I wanted to gather feedback on whether you prefer thermal routing handled closer to the layout manager wrapper or inside PopoutService before spending time refactoring the alignment. Once we agree on the preferred pattern, I'll happily rebase, integrate with openWidgetPopout, and make the state machine bulletproof, and therefore, continue align other PRs slicing from my local hardenings on this and other topics after read other commits, issues and so on sporadically. |
fix(thermal): decouple widget click from sort state; add tab-aware modal routing
Status
Draft — behavior change needs upstream discussion before merge.
Summary
Removes
DgopService.setSortBy()fromCpuTemperatureandGpuTemperaturewidget click handlers. Widgets emit signals only; routing logic moves toPopoutService. This changes the default tab opened by thermal clicks — see "Behavior change" below.Behavior change
Upstream's
onCpuTempClicked/onGpuTempClickedopen the modal at whatever tab was last active (default: tab 0 Processes). This PR routes thermal clicks to specific tabs.Rationale: clicking a temperature widget should take you to the Performance/System tab, not to a generic process list. This feels more intentional and reduces navigation.
Design questions for upstream
setSortBy('cpu')in GpuTemperature a bug or intentional?PopoutServiceor closer to the widget layer?What changed
Widget layer
DgopService.setSortBy()from click handlers in both widgetssetSortBy('cpu')— copy-paste bug from CpuTemperatureRouting
PopoutService.toggleProcessListModal(1/3)Modal
toggle(tabIndex)— now delegates toshow()instead of duplicating logic; consistent state managementclampTab()— bounds [0, tabCount-1], derived fromtabCountproperty (was hardcoded3)show()restores sort state: callsDgopService.setSortBy('cpu')when opening tab 1 (Performance)nextTab()/previousTab()— usetabCountinstead of hardcoded4Async race condition fix (PopoutService)
Qt.callLater(() => modal.show(tabIndex))afterloader.active = trueQt.callLaterfires before the Loader finishes loading the component — race conditionpendingProcessTabproperty, consume it inConnections { target: loader; onStatusChanged }when status reachesLoader.ReadyOut of scope
DgopService.setSortBy()APItoggleProcessListModal()Validation checklist
make lint-qmlpasses