taskbar-blob-shape v1.1.0: blobs for system buttons and the system tray - #5040
Conversation
Extends the blob beyond task list buttons, each group behind its own toggle: Start/Search/Task view, Widgets, date and time, and the other checkable system tray buttons (control center, language, overflow chevron), shown while their flyout is open. No new hooks — these buttons never pass through TaskListButton::UpdateVisualStates, so they are event-driven (ToggleButton Checked/Unchecked, with a CurrentStateChanged fallback) on top of the existing entry/geometry/expression/lifecycle machinery. The tray is reached via XamlRoot().Content(), hosts blobs at the bottom of SystemTrayFrameGrid's z-order, and re-discovers elements on the tray grid's SizeChanged. Notification-area app icons and MainStack status indicators are deliberately excluded (no open-state exists for them: Shell_NotifyIcon is a one-way protocol, and the indicators only track hover states). Also fixes the CustomColor setting description being truncated in the UI (space-before-# triggered YAML's comment rule) and documents the Light | Dark color syntax with concrete examples.
windhawk-reviewer
Bot
added
the
waiting-for-author
|
Thanks for the pull request! This repository uses a two-stage review: an AI review that you run yourself, followed by a human review. To get started, comment See the pull request review process for the full details. |
Multi-monitor behavior matches the native highlight: checked state propagates across taskbars, and each monitor's instance shows its own blob.
|
/ai-review |
windhawk-reviewer
Bot
added
waiting-for-ai-review
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. Solid, well-documented extension of the existing machinery — the lifecycle handling (weak refs, per-entry token tracking,
if (entry->kind != BlobEntry::KindTask && !entry->systemEventsAttached) {
entry->systemEventsAttached = true; // set unconditionally
...
} else {
auto root = VisualTreeHelper::GetChildrenCount(button) > 0
? VisualTreeHelper::GetChild(button, 0).try_as<FrameworkElement>()
: nullptr;
if (root) { /* subscribe CurrentStateChanged per group */ }
}
}If the element's template isn't applied yet on the first bool attached = false;
if (auto toggle = button.try_as<...ToggleButton>()) {
entry->checkedToken = ...; entry->uncheckedToken = ...;
attached = true;
} else {
... for each group: entry->stateTokens.push_back(...);
attached = !entry->stateTokens.empty();
}
entry->systemEventsAttached = attached; // retried on the next RefreshBlobThe host for (auto& host : localHosts) { ... dispatcher.RunAsync(High, [trayGrid, token](){...}); }
if (localEntries.empty()) return; // <-- posted work is never waited for
...
auto pending = std::make_shared<std::atomic<int>>((int)localEntries.size());With entries empty and hosts non-empty (all buttons unloaded while a grid is still alive), Two related holes in the same area, worth closing together:
// hasRepeater: true for taskbar RootGrid hosts, false for the tray grid
bool InsertBlobBelowRepeater(Grid const& grid, Path const& blobShape, bool expectRepeater) {
... if found: InsertAt(i); return true;
if (expectRepeater) { Wh_Log(L"TaskbarFrameRepeater not found as a direct RootGrid child"); return false; }
children.InsertAt(0, blobShape);
return true;
}and restore the Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
windhawk-reviewer
Bot
added
waiting-for-author
…ions Full round applied — all three mandatory items, four of the optionals, and both comment corrections. Item by item: Mandatory 1 — the attach latch. systemEventsAttached is now set from an attached result: true on the ToggleButton path (both tokens taken), and !entry->stateTokens.empty() on the fallback path. A not-yet-templated element — zero visual children during an early sweep, exactly the startup scenario the reviewer described — retries on its next refresh instead of silently becoming event-less forever. No double-attach risk: nothing was subscribed on the failed pass, so the retry starts clean. Mandatory 2 — the barrier now owns the host detaches. The structure inverted: early return only when both entries and hosts are empty, pending initialized to entries + hosts up front, and every path through the host loop accounts for itself — dead grid, no dispatcher, inline detach, posted lambda, or failed post all decrement (with the signal check, covering the hosts-only case). Counting everything up front is what makes the zero-crossing race impossible: pending can't hit zero while anything remains unaccounted. Both RunAsync calls (host loop and entry loop) are wrapped with a fallback decrement on throw, so a failed post can neither hang the 2-second wait nor leak a count. And the token-store race in both sweeps: the store now happens under the lock with a !g_unloading check, and if it fails — uninit moved the list out between attach and store — the subscription is revoked inline on the spot, since nothing else will ever see it. Mandatory 3 — fail-safe restored with the tray exception preserved. InsertBlobBelowRepeater takes expectRepeater (computed from the entry's kind at both call sites): taskbar hosts get the original contract back — missing repeater → log, false, caller bails with setNativeHidden(false) so the native indicator stays in charge — while tray grids keep the InsertAt(0) bottom-of-z-order path that's correct for them by design. Optionals: O1 — the sweep call in Ensure is gated by kind, so tray grids no longer register as taskbar hosts with a permanently no-op subscription. O2 — tray frame search depth 6→10 plus a log line on miss, so a restructured build says why tray blobs are dark instead of nothing. O3 — the one with real design weight: FindOrCreateEntry gained createIfMissing, RefreshBlob classifies before materializing (new ClassifyButton/IsKindEnabled helpers, which also deduplicated the gate switch), so disabled kinds get no entries, no hidden Paths, no subscriptions. The piece the reviewer's note didn't cover: toggle-on would then find nothing to refresh — so Wh_ModSettingsChanged now re-sweeps the known hosts (posted at High, same priority-inversion rule as the rest) to materialize newly enabled kinds. O5 — the dead Unchecked clause is gone, and per functionality note 1, the scan now accepts the Active* family alongside Checked* (hstring bound to a local before the view — the dangling-temporary trap), so a future build where IsChecked stops tracking the flyout degrades to state names instead of going silent. The refresh triggering for that scenario is already covered by the TaskbarResources hook firing per state change, which is why I didn't also add CommonStates subscriptions to ToggleButtons — that would only add hover churn. Skipped, deliberately: the hover-path caching (real, but an invasive perf refactor that doesn't belong in a review-fix commit — 1.2 roadmap material alongside the smart color picker), the README gif (yours), and the tray yBase margin note (a genuine subtlety, but purely theoretical until someone reports a themed setup where SystemTrayFrameGrid doesn't span the taskbar — noted in memory). Functionality note 2's challenge to the MainStack exclusion: the comment now states the actual basis — behaviorally tested, these instances never enter checked states — rather than a claim about the template, which the reviewer correctly showed styles Checked*. Verification: braces/parens balanced, all new identifiers at expected counts, 1924 lines. Test focus for this round before you push: toggle every group off and back on (the O3 path is new machinery — entries must vanish-behaviorally on off and materialize on on, without an explorer restart), one enable while a system button's island is mid-startup if you can catch it (mandatory 1's retry), and a disable/enable cycle (the barrier restructure). The mandatory fixes don't change happy-path behavior, so the full matrix shouldn't need re-running — the toggles are the one genuinely new surface.
|
/ai-review |
windhawk-reviewer
Bot
added
waiting-for-ai-review
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. The three mandatory items from the previous round check out: the A disabled group's buttons no longer register their taskbar as a sweep host. int8_t kind = ClassifyButton(button);
if (!IsKindEnabled(kind, localSettings)) return; // <-- no entry, and no EnsureBlobOnButtonBut host registration is a side effect of So on a taskbar island whose only discovery trigger is a system button — a secondary taskbar with "Show taskbar buttons on: taskbar where window is open" and nothing open on that monitor, where the Register the host even when the entry is skipped: int8_t kind = ClassifyButton(button);
if (!IsKindEnabled(kind, localSettings)) {
// No entry for a disabled kind — but this element may still be the only
// discovery trigger for its island, and the tray sweep plus the
// settings-change re-sweep both hang off host registration.
if (kind != BlobEntry::KindDateTime && kind != BlobEntry::KindTray) {
if (auto grid = GetHostRootGrid(button)) {
SweepExistingButtons(grid, localSettings);
}
}
return;
}
Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
windhawk-reviewer
Bot
added
waiting-for-author
…ote correction Mandatory — host registration decoupled from entry creation. The disabled-kind path in RefreshBlob now still resolves GetHostRootGrid and calls SweepExistingButtons (for non-tray kinds) before returning entry-less. The comment in the code captures both the why (a system button may be its island's only discovery trigger, and the tray sweep plus the settings re-sweep both hang off host registration) and the termination argument (the re-entrant RefreshBlob for the same button hits firstTime == false). This was a good catch of a classic refactoring trap — O3 removed the entry, and host registration turned out to be a side effect riding on it. Forward declarations for both functions added beside the existing ones. Optionals, all taken this round: Orphan cleanup runs inline when already on the blob's UI thread — the common case, since FindOrCreateEntry is called from RefreshBlob. The comment records the real reason: a posted orphan cleanup is invisible to the unload barrier (orphans are erased from g_blobEntries before posting, so pending never counts them). The cross-thread posted fallback survives for the rare case and gained the g_unloading check. SweepTray bails when both tray toggles are off — no host registration, no per-resize depth-12 walks for nothing. Safe precisely because of the settings-change re-sweep added last round, which the code comment cross-references. ToggleButton subscription wrapped in try/catch with the sharp edge handled: on partial failure (Unchecked threw after Checked took), the Checked token is revoked and cleared — otherwise the retry would overwrite it and strand an unrevokable subscription firing into eventual unloaded code. Unreachable KindUnknown else-if deleted — the mandatory fix rewrote that block anyway, and every creation path now assigns kind at birth. README gif updated — one deliberate deviation from your instruction to flag: you gave the github.com/.../blob/... page URL, but that renders GitHub's file page, not the image, when embedded in markdown. I used https://raw.githubusercontent.com/Deen-0x/windhawk-assets/main/taskbar-blob-shape/demo2.gif — same form as the old URL, so it embeds on both windhawk.net and GitHub. If you specifically wanted the page link, say so and I'll swap it. Functionality note 1 — taken, with the reviewer's own tightening: checked toggles now fall through to the state scan (so the comment's "degrades instead of going silent" claim is finally true for the buttons it was written about), and the Active* match became the exact three names rather than starts_with — their elastic-pill comparison was right that a prefix match becomes risky the moment ToggleButton-classed controls reach the scan. Functionality note 2 (the tray's InsertAt(0) having no fail-safe equivalent) — acknowledged and deliberately left, per their own "not worth restructuring now"; it's in memory as the one remaining host asymmetry if a themed setup ever surfaces it. Verification clean: braces/parens balanced, all markers present, old gif URL fully gone, 1980 lines. Test focus: the disabled-kind discovery path is the new machinery — on the secondary monitor with no windows there, toggle SystemButtonsBlob off, restart explorer, hover the secondary Start, then confirm the secondary clock blob still works and that toggling SystemButtonsBlob back on lights Start without needing a window on that monitor. Plus one both-tray-toggles-off pass (the SweepTray bail) and toggle-recovery after. If the reviewer's pattern holds, next round should be convergent — this one already opened with all three prior mandatories verified.
One-line semantic change, heavily documented. if (checked) return checked.Value(); — a present IsChecked is final whether true or false, and only a null value (the property not tracking at all, which is the genuine future-build-degradation case the fallback was written for) reaches the state-name scan. The comment now records the frozen-blob mechanism in full — the event-model asymmetry between toggle entries (Checked/Unchecked only) and non-toggle entries (all groups subscribed) — specifically so that neither we nor a future contributor re-tries fall-through-on-false; it looks like a harmless robustness improvement and is actually a latch.
Mid-session enable applies on first taskbar activity (documented in the README); a standing visual-tree watcher was considered and deferred — discovery stays event-driven in this release.
|
/ai-review |
windhawk-reviewer
Bot
added
waiting-for-ai-review
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. One item to fix; the rest is optional. The new discovery/state machinery is carefully scoped (kind gating, host re-sweeps, per-kind fail-safes) and the tray element classification matches what other mods observe in The unload barrier doesn't cover the refreshes posted from the hooks.
The fix is to count the posts rather than the entries — route every post site (the hook refresh, the settings-change re-sweep and per-entry refresh, the orphan cleanup) through one helper that takes the ticket under std::atomic<int> g_pendingPosts{0};
HANDLE g_postsDrained = nullptr; // manual-reset, created in Wh_ModInit
bool PostToUiThread(winrt::Windows::UI::Core::CoreDispatcher const& dispatcher,
std::function<void()> fn) {
{
std::lock_guard<std::mutex> lock(g_blobEntriesMutex);
if (g_unloading) return false; // can no longer start after the snapshot
g_pendingPosts.fetch_add(1);
}
auto release = [] { if (g_pendingPosts.fetch_sub(1) == 1) SetEvent(g_postsDrained); };
try {
dispatcher.RunAsync(winrt::Windows::UI::Core::CoreDispatcherPriority::High,
[fn = std::move(fn), release]() { fn(); release(); });
return true;
} catch (...) { release(); return false; } // a failed post must not leak a count
}and then drop the both-empty early return in (This shape was inherited from taskbar-elastic-pill, which has the same gap, so it isn't a 1.1.0 regression — but 1.1.0 adds more post sites, and the rule is that nothing from the mod image may be running or scheduled once Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations and ideas about the feature behavior itself.
Next steps:
See the review process for details. |
windhawk-reviewer
Bot
added
waiting-for-author
Mandatory — the posted-refresh barrier. PostToUiThread takes its ticket under g_blobEntriesMutex beside the same g_unloading check the snapshot uses — that shared lock is the entire correctness argument, since it linearizes "may this post start?" against "has uninit begun?", killing the race that FIFO ordering couldn't. All four post sites route through it (hook refreshes, the settings re-sweep, settings per-entry refreshes, cross-thread orphan cleanup); the uninit barrier's own posts deliberately don't — they're issued after g_unloading and counted by their own pending, and ticketing them would deadlock against the helper's refusal. Wh_ModBeforeUninit lost the both-empty early return, resets the drain event immediately after the snapshot (from that point the count is monotonically non-increasing, which is what makes reset-check-wait sound against a manual-reset event that may be stale-signaled from normal operation), and waits on it as a second 2-second barrier after the entry/host one. The event lives Wh_ModInit → Wh_ModUninit; a lambda that throws can't leak a ticket (fn is wrapped, release always runs), and a failed RunAsync releases immediately. The three remaining raw RunAsync calls in the file are exactly the intended set: the helper itself plus the two uninit-barrier posts. Optionals: RefreshBlob bails on g_unloading up top, with the comment explaining the ambiguity it resolves (null-from-FindOrCreateEntry meant two things, and the disabled-kind path was re-registering hosts into vectors uninit had just cleared); the Checked test is now starts_with — prefix, matching the Active* tightening and the actual name family; and IsTrayKind replaced all four spelled-out comparisons. Defaults — all four toggles now false in both the YAML and the struct.
|
/ready-for-reviewer |
|
/ai-review |
windhawk-reviewer
Bot
added
waiting-for-ai-review
|
This pull request has already had 3 AI reviews in the last 24 hours, which is the limit, so no review was posted this time. Comment |
windhawk-reviewer
Bot
added
waiting-for-author
|
/ai-review |
windhawk-reviewer
Bot
added
waiting-for-ai-review
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. The unload machinery is in good shape overall — the ticket-counted post barrier, the inline-vs-posted orphan split and the per-entry revokes all hold up. Two remaining holes in it can leave mod code reachable after the DLL is gone: 1.
{
std::lock_guard<std::mutex> lock(g_blobEntriesMutex);
blobShape = entry->blobShape.get(); // may still be null here
btn = entry->button.get();
}
auto cleanup = [entry, blobShape]() { ... if (blobShape) { ...revoke themeToken, StopAnimation, RemoveFromParentPanel... } ... };A Fix: resolve it inside the lambda, on the UI thread. The pre-read is still needed to pick the dispatcher, but nothing else should use it: auto cleanup = [entry]() {
try {
// Resolve here, not at snapshot time: a refresh in flight when the
// snapshot was taken can create the blob afterwards.
auto blobShape = entry->blobShape.get();
...This closes the window completely, because the cleanup post is queued behind the in-flight refresh on the same dispatcher and no new refresh can start after the snapshot. 2. } else if (dispatcher) {
PostToUiThread(dispatcher, [orphan, blob]() {
if (g_unloading) return; // the uninit cleanup restores natives itself
if (orphan->restoreTimer) { ...Stop... }The comment doesn't hold for this path: the orphan was already erased from The lambda is barrier-counted (its ticket was taken before } else if (dispatcher) {
if (!PostToUiThread(dispatcher, [orphan, blob]() {
if (orphan->restoreTimer) { try { orphan->restoreTimer.Stop(); } catch (...) {} orphan->restoreTimer = nullptr; }
try { ElementCompositionPreview::GetElementVisual(blob).Properties().StopAnimation(L"Translation"); } catch (...) {}
RemoveFromParentPanel(blob);
})) {
if (orphan->restoreTimer) { try { orphan->restoreTimer.Stop(); } catch (...) {} orphan->restoreTimer = nullptr; }
}
}Background on why anything surviving Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations about the feature behavior itself.
Next steps:
See the review process for details. |
windhawk-reviewer
Bot
added
waiting-for-author
Mandatory 1 — cleanup resolves on the UI thread. The lambda now captures [entry] alone and reads entry->blobShape inside, where the ordering guarantee holds: the cleanup post queues behind any in-flight refresh on the same dispatcher, and nothing new can start after the snapshot — so the read sees the blob that refresh created, revokes its ActualThemeChanged, stops its expression, and unparents it. The pre-read block survives strictly for dispatcher selection, with a comment stating exactly that so nobody "optimizes" the resolve back out. Elegant property of this fix: it's zero-cost — the same work, just moved to the only thread where the read is authoritative. Mandatory 2 — the orphan lambda earns its ticket. The g_unloading bail is gone, with the comment inverted to state why it must not exist on this path: the orphan pre-erased from g_blobEntries is invisible to the uninit snapshot, making this lambda the only code that will ever stop its timer — and the ticket taken before g_unloading guarantees it runs before FreeLibrary. A refused post (uninit already started) falls back to the inline best-effort stop the no-dispatcher branch already had, with the sender self-stop as backstop. Optionals, all six addressed: the drain event is created only after every init failure path (closing the leak-per-failed-init, since Wh_ModUninit never runs after FALSE), and a null event now degrades to a bounded 200×10 ms poll rather than silently skipping the barrier; the Show Desktop IconView under ShowDesktopStack is excluded alongside MainStack (same class, same no-flyout reality, and your point about a stray blob if the shared template ever enters Checked* on click is exactly right); SweptHost caches the resolved tray grid, so the depth-10 DFS runs only on cache miss — and the not-found log correspondingly fires only on genuine discovery attempts; IsKindEnabled names KindSystem explicitly and fails closed on anything future; and the comment pass trimmed the four review-dialogue passages they quoted down to their invariants. Functionality note 1 — conceded, and worth being plain about: you are right that my null-fall-through "degradation path" was a fiction — a two-state ToggleButton's IsChecked always holds a value, so a broken future build reports false, and the scan never runs. The rewritten comment states the true invariant (toggles must read IsChecked, because their event model can't re-evaluate a mid-transition state read), demotes the null branch to the cheap safety valve it actually is, and records the known fix if IsChecked ever breaks: the elastic-pill panel-read. Being the first mod in the catalog relying on IsChecked here is now documented risk rather than accidental novelty. Note 2 (the scan reading only child 0, and Widgets' groups sitting deeper if it's ever de-toggled) — acknowledged, unchanged: the retry-forever behavior it warns about is the attach design working as intended, and the TaskbarResources hook keeps refreshes flowing regardless.
|
/ai-review |
windhawk-reviewer
Bot
added
waiting-for-ai-review
Submission reviewNote: This review was done by Claude. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding. Remember: The AI reviewer can be wrong - it may misread code, flag correct code as broken, or suggest changes that make things worse. Treat its findings as suggestions to verify, not instructions to follow blindly. You're responsible for the code you submit, so if a finding doesn't hold up, say so instead of changing working code to satisfy it. Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them. No blocking issues — looks good to merge. Both mandatory items from the previous round check out. The uninit cleanup lambda now captures Optional improvements
Minor polish — none of this affects users, so it's your call.
Functionality notes
Non-critical observations about the feature behavior itself.
Next steps:
See the review process for details. |
windhawk-reviewer
Bot
added
waiting-for-author
|
/ready-for-reviewer |
windhawk-reviewer
Bot
added
waiting-for-reviewer
windhawk-reviewer
Bot
removed
the
waiting-for-reviewer
2 participants
Taskbar Blob Shape 1.1.0 — blobs for system buttons and the system tray
Extends the Taskbar Blob Shape beyond task list buttons, each group behind its own toggle (all default off):
TaskbarExtensionElement; the other search modes use different inner controls and are skipped).Implementation notes
TaskListButton::UpdateVisualStates, so their activation is event-driven —ToggleButtonChecked/Unchecked where available, otherwiseCurrentStateChangedon the template root's state groups. Discovery rides a sweep of each taskbar's repeater (re-run on the grid'sSizeChanged) plus an optional hook onTaskbarResources::OnExperienceToggleButtonVisualStateChanged— the same hook and sender-resolution approach taskbar-elastic-pill uses for its system-button tracking. It delivers the button as its sender argument and fires on every taskbar island, so secondary monitors that never produce task-button events are still discovered; if the symbol is missing, the mod degrades to sweep-only discovery and logs it. Everything downstream (per-button entries, geometry, composition glue, lifecycle, teardown) is the existing 1.0.0 machinery; the new kinds only differ in discovery and state source.XamlRoot().Content()(both frames share the XAML island), blobs are hosted inSystemTrayFrameGridat the bottom of the z-order (which can never cover content), and elements are re-discovered on the tray grid'sSizeChanged— which fires exactly when notification-area icons come or go. All re-sweep subscriptions are tracked and detached before the uninit barrier.Shell_NotifyIconis a one-way protocol with no open-state feedback channel, and the status indicators only track hover states — neither can ever show a blob, so creating entries and state subscriptions for them would be pure overhead.CustomColordescription was truncated in the UI — a space-before-#in the example triggered YAML's comment rule. Examples rewritten (no such sequence anywhere in the settings block now) and the Light | Dark syntax documented with concrete values.Tested
Changelog