Skip to content

fix(thermal): decouple widget click from sort state; add tab-aware modal routing via openWidgetPopout#2690

Open
louzt wants to merge 3 commits into
AvengeMedia:masterfrom
louzt:fix/thermal-widget-routing-v2
Open

fix(thermal): decouple widget click from sort state; add tab-aware modal routing via openWidgetPopout#2690
louzt wants to merge 3 commits into
AvengeMedia:masterfrom
louzt:fix/thermal-widget-routing-v2

Conversation

@louzt

@louzt louzt commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Status

Ready for review — rebased against latest upstream/master (2026-07-07), conflict-free (MERGEABLE).

Summary

Removes DgopService.setSortBy() from CpuTemperature and GpuTemperature widget click handlers. Widgets emit signals only; routing logic moves to PopoutService. Adopts upstream's openWidgetPopout(spec) abstraction with explicit tabIndex for deterministic tab routing.

Behavior

Widget Upstream (before this PR) This PR
CPU temp click Opens tab 0 (Processes) Opens tab 1 (Performance), sorted by CPU
GPU temp click Opens tab 0 (Processes) Opens tab 3 (System)

What changed (3 commits)

Commit 1: a68ec133 — decouple widget click from sort state

  • Removes DgopService.setSortBy() from CpuTemperature and GpuTemperature click handlers
  • Fixes copy-paste bug: GpuTemperature was calling setSortBy('cpu') instead of 'gpu'
  • DankBarContent: replaces 20-line manual popout positioning with PopoutService.toggleProcessListModal(tabIndex)
  • ProcessListModal: adds tabIndex parameter to show()/toggle(), clampTab() validator, tabCount (4) and maxTabIndex readonly properties

Commit 2: 3019d7f7 — eliminate async race condition in modal tab routing

  • PopoutService: replaces Qt.callLater() with explicit Connections { target: processListModalLoader; onStatusChanged }show() called only after Loader.Ready, not just after setActive(true)
  • Tab index stored in pendingProcessTab and consumed once modal component is fully loaded
  • ProcessListModal: show() restores sort state by calling DgopService.setSortBy('cpu') when opening tab 1 (Performance)

Commit 3: 2e60b85f — toggle() delegates to show()

  • ProcessListModal.toggle(): now delegates to show() instead of duplicating visibility logic; handles clampTab, sort state, and visibility in one place

Architecture resolution

After initial Draft review, upstream merged openWidgetPopout(spec) abstraction. This PR was rebased to adopt that pattern:

// Upstream's pattern (adopted here):
topBarContent.openWidgetPopout({
    loader: processListPopoutLoader,
    widgetItem: cpuTempWidget,
    triggerSource: "cpu_temp",
    tabIndex: 1,   // CPU -> Performance tab
    mode: "click"
});

The tabIndex field is consumed by PopoutManager.requestPopout(popout, spec.tabIndex, spec.triggerSource) at DankBarContent.qml:573 — upstream left this gate open, this PR feeds it with the correct thermal routing.

Validation checklist

  • Click CPU temp -> tab 1 (Performance) opens, sorted by CPU
  • Click GPU temp -> tab 3 (System) opens
  • Click same widget again -> modal closes
  • Switch between CPU/GPU clicks -> correct tab each time
  • Modal opens without flicker (no Qt.callLater race condition)

Follow-up: Spanish localization

I joined the DankMaterialShell project on POEditor via the public join page. My POEditor account (louzt / davidmirelesll@outlook.com) shows the project under PUBLIC > SHARED.

According to the POEditor docs, contributor access is granted per-language by an admin. Once assigned to Spanish, I can pull existing strings, add translations, and push back via the API — or export and PR the .po file if that workflow is preferred.

Request: someone with admin access on the POEditor project add me (louzt / davidmirelesll@outlook.com) to the Spanish language so I can contribute translations.

@louzt louzt closed this Jun 24, 2026
@louzt louzt reopened this Jun 24, 2026
@louzt louzt marked this pull request as draft June 24, 2026 00:39
@Spoch-dev

This comment was marked as spam.

@louzt

louzt commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Closing

@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 PopoutService orchestration versus the modal layer in this project, and how would you handle the backend state/sorting synchronization to better align with upstream patterns?

@Purian23

Copy link
Copy Markdown
Collaborator

Closing

@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 PopoutService orchestration versus the modal layer in this project, and how would you handle the backend state/sorting synchronization to better align with upstream patterns?

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.

@louzt

louzt commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

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.

@louzt

louzt commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

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.

@louzt louzt marked this pull request as ready for review July 7, 2026 09:40
@louzt louzt force-pushed the fix/thermal-widget-routing-v2 branch from e39be76 to 2e60b85 Compare July 7, 2026 09:57
louzt added 3 commits July 7, 2026 04:01
…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.
@louzt louzt force-pushed the fix/thermal-widget-routing-v2 branch from 2e60b85 to 28e7ddb Compare July 7, 2026 10:02
@louzt louzt changed the title fix(thermal): decouple widget click from sort state; add tab-aware modal routing fix(thermal): decouple widget click from sort state; add tab-aware modal routing via openWidgetPopout Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants