Skip to content

Update Taskbar Count Badges to 1.1.0 - #5250

Open
digart11 wants to merge 5 commits into
ramensoftware:mainfrom
digart11:taskbar-count-badges-1.1.0
Open

Update Taskbar Count Badges to 1.1.0#5250
digart11 wants to merge 5 commits into
ramensoftware:mainfrom
digart11:taskbar-count-badges-1.1.0

Conversation

@digart11

@digart11 digart11 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Adds top and bottom dot positions to Taskbar Count Badges.
Bottom mode replaces only the native Windows running indicator with the count dots, while leaving Windows task progress indicators untouched.

Changelog

  • Added top and bottom dot positions.
  • Top and bottom dots are displayed horizontally; left and right remain vertical.
  • Bottom dots replace the native Windows running indicator and show from one running window.
  • Added version history to the mod details documentation.

Mod authorship

If this pull request introduces a new mod, please complete the section below.

This mod was created by:

    • The submitter, without AI assistance
    • The submitter, with AI assistance
    • Claude
    • ChatGPT
    • Gemini
    • Another AI (please specify):
    • Other (please specify):

Please select the options that best apply. Your selection does not affect the acceptance criteria, but it helps reviewers understand the context of the code and provide relevant feedback.

@windhawk-reviewer windhawk-reviewer Bot added the waiting-for-author The author's turn: request an AI review, or respond to one that was posted. label Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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 /ai-review. Once you're happy with the result, comment /ready-for-reviewer to hand it over to a human reviewer.

See the pull request review process for the full details.

@digart11

digart11 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

Submission review

Note: 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 new top/bottom positions are a clean addition and the restore paths are wired into every exit (PruneDeadTrackedButtons, TrackTaskbarButton recycle, UpdateCountBadge failure branches, CleanupOnTaskbarThread) — that part looks careful. One item on how the native indicator is suppressed:

Suppress RunningIndicator at the composition level instead of snapshotting and restoring Opacity. HideRunningIndicatorForBottomDots saves runningIndicator.Opacity() as the "original" value and RestoreRunningIndicator writes that saved value back. Two problems:

  1. The saved value can be the mod's own 0.0, which makes the hide permanent. Wh_ModBeforeUninit has a documented path where the taskbar-thread cleanup doesn't run (Wh_Log(L"Couldn't execute taskbar-thread cleanup")) — the mod then unloads with every running button's indicator still at Opacity(0.0). On the next enable, tracked.runningIndicatorOpacity = runningIndicator.Opacity() captures 0.0 as the original, so from then on "restoring" sets 0.0. Switching to another dot position, switching to Number badges, or disabling the mod all leave the taskbar with no running indicators at all, unrecoverable until Explorer restarts. The comment at the bottom of Wh_ModBeforeUninit ("leaving ... any stale visual behind is safe") was true in 1.0.0 when the worst case was a leftover badge; with bottom dots the worst case is a taskbar missing its native indicators.
  2. A local Opacity value is outranked by the shell's RunningIndicatorStates transition storyboards. The reassert in ApplyCountToTrackedButton runs at UpdateVisualStates time — i.e. before GoToState starts the transition — so the animated value wins afterwards and the native indicator can show through under the dots on activation/hover. Relatedly, Opacity() returns the effective value, so capturing it mid-storyboard snapshots an arbitrary intermediate number as the "original".

Both go away with a composition-level hide, which is idempotent (no saved state to get poisoned), can't be overridden by XAML's property system, and leaves XAML layout geometry intact so the TransformToVisual anchoring math still works:

// hide
ElementCompositionPreview::GetElementVisual(runningIndicator).IsVisible(false);
// restore
ElementCompositionPreview::GetElementVisual(runningIndicator).IsVisible(true);

taskbar-blob-shape hides this exact element that way, and documents the precedence reason ("the RunningIndicatorStates transition storyboards hold ANIMATED values ... which outrank local values in XAML's precedence"); its restore is RestoreNativeVisuals. With that change, runningIndicatorOpacity can be dropped from TrackedButton entirely. If you'd rather stay on Opacity, at minimum restore with runningIndicator.ClearValue(UIElement::OpacityProperty()) instead of a captured value — that removes the local value rather than pinning a possibly-wrong one.

Optional improvements

Minor polish — none of this affects users, so it's your call.

  • BindDotPositionExpression re-resolves RunningIndicator with a full recursive subtree walk, even though HideRunningIndicatorForBottomDots resolved it moments earlier and tracked.runningIndicator already holds it. In bottom mode that doubles the search cost on every update; passing the resolved element (or the TrackedButton&) into the bottom branch avoids it.

  • A button without a RunningIndicator child permanently defeats the update cache. In ApplyCountToTrackedButton, runningIndicatorReady stays false whenever the hide hasn't succeeded, and UpdateCountBadge returning false rolls lastAppliedCount back — so every subsequent UpdateVisualStates redoes the whole path (several recursive FindChildByName sweeps plus RemoveOrphanDotBadges's scan of RootGrid) for a feature that isn't rendering anyway. Consider a per-button "no running indicator here" flag, cleared on settings-generation change and on element recycle, so it's attempted once. A Wh_Log on that path would also make the case diagnosable.

  • Identity comparison at HideRunningIndicatorForBottomDots: winrt::get_abi(previousIndicator) == winrt::get_abi(runningIndicator) compares IFrameworkElement*, while the rest of the mod compares through IUnknown (TrackTaskbarButton, RemoveOrphanDotBadges). COM identity is only guaranteed for IUnknown, so using try_as<winrt::Windows::Foundation::IUnknown>() here too would be both stricter and consistent with the surrounding code.

  • [[clang::no_destroy]] on g_trackedButtons looks unnecessary. TrackedButton holds only winrt::weak_refs, a void*, a double and PODs — destroying the list is an in-process refcount decrement plus a heap free, both of which are safe on the process-shutdown path, so the automatic destructor doesn't need suppressing. An unneeded suppression is mostly noise and invites cargo-culting; see Global objects and process shutdown for the cases where it genuinely is required. (If you keep it, the explicit reset() in CleanupOnTaskbarThreadProc is the right companion, so nothing else changes.)

  • Screenshots: showcase.png predates this change — worth refreshing it so the new top/bottom positions are visible in the mod description.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

  • Bottom mode discards the active/inactive distinction. Windows draws RunningIndicator wide and accent-colored for the foreground app's button and short/dim otherwise; replacing it with uniform dots means you can no longer tell at a glance which app is in front. You could reflect it — read the RunningIndicatorStates group's current state and check for ActiveRunningIndicator, the way taskbar-elastic-pill does, then use a different dot color/size — or at least call the trade-off out in the README.

  • Dots are drawn over the task progress strip. The README says progress indicators are untouched, which is true of the element, but ProgressIndicator occupies the same bottom strip as RunningIndicator and the badge is drawn there with Canvas.ZIndex = 100. Worth checking that an app showing taskbar progress (a file copy, a download) still reads correctly with dots on top of it.

  • Bottom mode multiplies the compositor work. With effectiveMinimumCount = 1, every running button now gets a Border in RootGrid plus two expression animations — a Translation expression referencing the whole layout chain (up to 20 visuals) and a TransformMatrix expression. In 1.0.0 the dots only existed for multi-window buttons, so this is a large increase in per-frame work on a busy taskbar. Worth checking taskbar animation smoothness with ~15+ running apps before/after.

  • Dot size has no upper clamp. std::max(2, Wh_GetIntSetting(L"VerticalDots.size")) only guards the lower bound. With the new Top position, dotY = iconY - kDotGap - dotSize goes negative for a large dot size, and since FindDotHostGrid deliberately requires an unclipped RootGrid, the row renders outside the button. A sanity clamp on the size (or clamping dotY to >= 0) would keep it contained.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 28, 2026
@digart11

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

Submission review

Note: 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 new bottom-dot mode is the interesting part of this update: it's the first thing in the mod that changes a shell-owned element (RunningIndicator) instead of only adding one, so the restore paths carry more weight than before. Two things there are worth fixing.

1. A hidden native running indicator can survive the mod being disabled, and nothing heals it.

Every restore path (RestoreRunningIndicator, CleanupOnTaskbarThread, PruneDeadTrackedButtons) works exclusively off entries this instance put into TrackedButtons(). If the taskbar-thread cleanup can't be marshaled — the path you explicitly handle at the end of Wh_ModBeforeUninit — or the mod is unloaded abnormally, the shell's own RunningIndicator composition visuals stay IsVisible(false) for every button that was showing dots. The user then has taskbar buttons with no running indicator at all, after the mod is already disabled, until Explorer restarts; and re-enabling the mod in any other mode won't fix it, because a fresh instance has no record of what the previous one hid.

That's a different class of leftover from a stale badge: badges already have a recovery path (RemoveOrphanDotBadges), and the shell state doesn't. Two changes:

  • Add an equivalent recovery for the indicator. The cheapest form is a one-shot reassert the first time this instance touches a button while not in bottom-dot mode (i.e. when tracked.lastAppliedCount is still the sentinel):

    if (!bottomDotMode && tracked.lastAppliedCount ==
                              std::numeric_limits<unsigned int>::max()) {
        // Undo a hide left behind by a previous instance whose cleanup
        // couldn't run.
        if (auto indicator = FindChildByName(taskListButton, L"RunningIndicator")) {
            try {
                Hosting::ElementCompositionPreview::GetElementVisual(indicator)
                    .IsVisible(true);
            } catch (...) {
            }
        }
    }

    A tree-wide sweep at init works too — see RestoreNativeIndicators in taskbar-elastic-pill.wh.cpp, which restores every TaskListButton's indicator by walking the tree rather than a tracked list. Worth keeping the reassert one-shot per button rather than continuous, so it doesn't fight other mods that legitimately suppress the indicator (taskbar-blob-shape uses the same IsVisible(false), taskbar-elastic-pill uses Opacity).

  • Update the comment in Wh_ModBeforeUninit ("leaving the no_destroy weak tracking state and any stale visual behind is safe: no code in this DLL remains callable after unload"). It's still true that nothing in the image stays callable, but as of 1.1.0 the leftover is no longer cosmetic-and-mod-owned — it's a suppressed shell element.

2. Bottom mode fails closed when RunningIndicator isn't found, making its own fallback unreachable.

UpdateCountBadge bails out before drawing anything if HideRunningIndicatorForBottomDots returns false, and that function returns false when the element simply isn't there:

if (bottomDotMode && count > 0 &&
    !HideRunningIndicatorForBottomDots(tracked, taskListButton))
{
    ...
    return false;   // no dots at all
}

Meanwhile BindDotPositionExpression has a documented fallback for exactly that situation ("Fallback for a Windows build where RunningIndicator doesn't currently expose usable geometry") — but with the bail-out above it only ever runs for the found but zero-sized case, never for the missing case. So on a build (or a button type) without that element, bottom dots silently show nothing, while left/right/top keep working.

It also means runningIndicatorReady in ApplyCountToTrackedButton stays false forever for such a button, so the cached-state early-out never fires and the full per-button work (two recursive FindChildByName sweeps, RemoveOrphanDotBadges, badge lookup) repeats on every UpdateVisualStates call.

Make the two agree: if the indicator can't be found there's nothing to hide, so let the dots draw with the below-the-icon fallback instead of returning false — e.g. have HideRunningIndicatorForBottomDots distinguish "no indicator present" (proceed, nothing hidden) from "hide failed" (bail out), and treat the former as ready.

Optional improvements

Minor polish — none of this affects users, so it's your call.

  • HideRunningIndicatorForBottomDots sets tracked.runningIndicatorHidden = true before the hide is actually applied, so if GetElementVisual(...).IsVisible(false) throws, the catch calls RestoreRunningIndicator, which forces IsVisible(true) on an element this mod never hid — overriding a state the shell (or another mod) may own. Set the flag inside the try, after the call succeeds, like taskbar-blob-shape does.

  • The sameIndicator check compares winrt::get_abi() on two FrameworkElement projections. COM identity is only guaranteed through IUnknown; elsewhere the mod already does this correctly (TrackTaskbarButton, RemoveOrphanDotBadges both go through IUnknown first). Same for the iconPanelBadge comparison in UpdateCountBadge.

  • BindDotPositionExpression re-runs FindChildByName(taskListButton, L"RunningIndicator") — a full recursive subtree walk — even though HideRunningIndicatorForBottomDots just resolved and cached the same element in tracked.runningIndicator a few lines earlier. Passing it through (or reading the weak ref) saves a walk per update in bottom mode.

  • RestoreRunningIndicator always restores to IsVisible(true) rather than the value that was there before hiding. That matches what other taskbar mods do, so it's fine as-is, but if you ever see the indicator reappear on a button where the shell wanted it gone, capturing visual.IsVisible() before the hide is the robust form.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

  • Bottom mode drops the active-window cue. The native indicator distinguishes the foreground app (wider/highlighted variant); uniform dots don't. If you want it back, the visual state is readable without extra hooks — GetVisualStateGroup(iconPanel, L"RunningIndicatorStates") and comparing the current state to ActiveRunningIndicator, as in taskbar-elastic-pill.wh.cpp and taskbar-blob-shape.wh.cpp — so the active button could get a different dot color/size.

  • Behavior.minimumCount is silently overridden to 1 in bottom mode. It's documented, but you could honor the setting instead and simply not hide the native indicator when the count is below the threshold — same "never leaves a button with no indicator" guarantee, one less special case, and the setting keeps working in all four positions.

  • Neither the dot size nor the resulting row length is bounded. Five dots at, say, size 10 give a 58 DIP row that will overhang the button on both sides in bottom/top mode, and top mode's dotY = iconY - kDotGap - dotSize can push the row above the taskbar edge. Clamping the dot size (or the total row length to the button width) would keep large sizes usable rather than clipped.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 28, 2026
@digart11

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

Submission review

Note: 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.


Two issues around the new bottom-dot mode, both in the RunningIndicator handling.

1. The "heal" pass un-hides RunningIndicator on buttons this mod never touched, and it fights other mods

ApplyCountToTrackedButton runs this for every newly tracked button whenever the mod is not in bottom-dot mode — which includes the default number-badge configuration:

if (!bottomDotMode &&
    item.lastAppliedCount == std::numeric_limits<unsigned int>::max())
{
    if (auto runningIndicator = FindChildByName(element, L"RunningIndicator"))
    {
        try
        {
            Hosting::ElementCompositionPreview::GetElementVisual(runningIndicator)
                .IsVisible(true);
        }
        catch (...) {}
    }
}

Because mod globals don't survive an unload, a fresh instance has no way to know who hid an indicator — so this blindly re-shows any indicator that anything hid via composition visibility. taskbar-blob-shape suppresses RunningIndicator using the exact same ElementCompositionPreview::GetElementVisual(...).IsVisible(false) mechanism and restores it in its own state handling. A user running both mods (with Count Badges on its default settings, which have nothing to do with the running indicator) gets the two mods flipping the same visual property against each other on every UpdateVisualStates. It also contradicts the README, which promises "Left, right, and top leave the native Windows running indicator untouched" — with this code, they don't.

Secondly, the comment says "Do this only once per button", but that isn't what the code does. lastAppliedCount is reset back to previousCount (i.e. stays max()) whenever UpdateCountBadge returns false, so on any button where the badge can't be applied — e.g. dots mode where FindDotHostGrid keeps returning null because RootGrid is clipped or not wider than IconPanel — the recursive FindChildByName walk and the IsVisible(true) write repeat on every single UpdateVisualStates call for that button, indefinitely.

Suggested fix: make the recovery targeted instead of blanket. Record in the mod's own persistent storage that indicators were actually hidden, and only heal when that flag is set:

// when a button's indicator is first hidden
Wh_SetIntValue(L"indicatorsHidden", 1);
...
// at the end of a successful CleanupOnTaskbarThread()
Wh_SetIntValue(L"indicatorsHidden", 0);

then gate the heal on Wh_GetIntValue(L"indicatorsHidden", 0), and latch a per-button bool so it genuinely runs at most once per button. Alternatively, just drop the heal — Wh_ModBeforeUninit already tries three different windows to reach the taskbar thread, so the "cleanup couldn't run" case is narrow, and leaving it unhandled is better than silently overriding another mod's deliberate change.

2. Bottom dots can get stuck in the fallback position

In BindDotPositionExpression, the Bottom case falls back to below-the-icon placement when the indicator has no usable geometry:

if (runningIndicator &&
    runningIndicator.ActualWidth() > 0 &&
    runningIndicator.ActualHeight() > 0)
{ /* anchor to the indicator */ }
else
{
    // Fallback for a Windows build where RunningIndicator doesn't
    // currently expose usable geometry.
    dotX = iconX + (iconBoundsWidth - stackWidth) / 2.0;
    dotY = iconY + iconBoundsHeight + kDotGap;
}

The fallback isn't only reached on an exotic Windows build — it's also reached transiently. TaskListButton_UpdateVisualStates_Hook calls ApplyCountToButton synchronously right after the original sets the visual state, so on the not running → running transition the icon can already be laid out (that's guarded) while RunningIndicator still has zero size because its RunningIndicatorStates storyboard/layout pass hasn't completed. When that happens UpdateCountBadge returns true, lastAppliedCount is committed, and the early-out in ApplyCountToTrackedButton then suppresses every subsequent update — so the dots stay at the below-the-icon offset until the count or the settings change again, which is a visible misalignment against the buttons that anchored correctly.

Suggested fix: don't cache a fallback placement as if it were final. Record it on the tracked button and force a re-application on the next update, the same way runningIndicatorReady already does:

struct TrackedButton { ...; bool bottomDotsUsedFallback = false; };

// in the early-out:
if (item.lastAppliedCount == count &&
    item.lastSettingsGeneration == settingsGeneration &&
    (!badgeExpected || item.badge.get()) &&
    runningIndicatorReady &&
    !item.bottomDotsUsedFallback)
{
    return;
}
Optional improvements

Minor polish — none of this affects users, so it's your call.

  • BindDotPositionExpression's Bottom case does its own FindChildByName(taskListButton, L"RunningIndicator"), even though HideRunningIndicatorForBottomDots has already resolved that element and stored it in tracked.runningIndicator a moment earlier. Passing the tracked weak ref through would save a third recursive subtree walk per update.

  • RunningIndicatorHideResult::NotPresent is never distinguished from Hidden by the only caller (which checks == Failed). Either collapse the enum to a bool, or use NotPresent for something.

  • The VerticalDots settings key is now a misnomer for a group that also holds top/bottom. Renaming it would reset existing users' dot settings, so keeping it is the right call — just worth a comment in the settings block so it doesn't look accidental.

  • The showcase screenshot still shows only the 1.0.0 styles. Since the whole point of this release is the two new dot positions, updating images/showcase.png to include a top and a bottom example would make the new modes obvious in the mod listing.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

  • Bottom mode drops the active-window affordance. Windows widens and accent-colors RunningIndicator for the focused app (ActiveRunningIndicator vs InactiveRunningIndicator), and replacing it with fixed-color dots loses that distinction entirely — with several apps open there's no longer any indication of which one has focus. You could read the current visual state and style the active button's dots differently, the way taskbar-elastic-pill does:

    auto group = GetVisualStateGroup(iconPanel, L"RunningIndicatorStates");
    auto state = group ? group.CurrentState() : nullptr;
    bool isActive = state && state.Name() == L"ActiveRunningIndicator";

    An "active dot color" setting (or just a size bump) would cover it.

  • In bottom mode the dots are vertically centred on the indicator (dotY = indicatorBounds.Y + (indicatorBounds.Height - dotSize) / 2), and the indicator is only ~3px tall. With a large configured dot size the row extends well past the indicator band, and since FindDotHostGrid deliberately picks an unclipped RootGrid, the dots can render outside the button. Clamping dotY to the button bounds for the bottom case would avoid that.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 28, 2026
@digart11

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 28, 2026
@windhawk-reviewer

Copy link
Copy Markdown

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 /ai-review again after 2026-08-29 15:16 UTC (in 22 hours) to get another one.

@windhawk-reviewer windhawk-reviewer Bot added the waiting-for-author The author's turn: request an AI review, or respond to one that was posted. label Aug 28, 2026
@windhawk-reviewer windhawk-reviewer Bot removed the waiting-for-ai-review An AI review was requested and is being prepared. label Aug 28, 2026
@digart11

Copy link
Copy Markdown
Contributor Author

/ai-review

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-ai-review An AI review was requested and is being prepared. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 29, 2026
@windhawk-reviewer

Copy link
Copy Markdown

Submission review

Note: 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.

The risky part of this change is that bottom mode now hides a shell-owned element (RunningIndicator) at the composition layer, so I checked the restore bookkeeping closely: hiding only ever happens through a tracked entry, entries are never dropped without restoring first (PruneDeadTrackedButtons, TrackTaskbarButton recycle path, CleanupOnTaskbarThread), and every failure path in UpdateCountBadge restores before returning. Switching position/style, count dropping to 0, container recycling and unload all end up with the native indicator visible again. The technique itself matches what taskbar-blob-shape already does. The rest is comments below.

Optional improvements

Minor polish — none of this affects users, so it's your call.

  • The indicator lookup runs up to three times per update. In bottom mode a single update calls FindChildByName(..., L"RunningIndicator") in ApplyCountToTrackedButton (the "is it ready" probe), again in HideRunningIndicatorForBottomDots, and a third time in BindDotPositionExpression's DotPosition::Bottom case — three recursive subtree walks for the same element. Resolving it once and passing it down (or reading tracked.runningIndicator after the hide) would remove two of them. Related: for a button that genuinely has no RunningIndicator, the probe walks the whole subtree on every UpdateVisualStates call, because runningIndicatorHidden never becomes true.

  • Hide/restore state is spread over ~8 call sites. RestoreRunningIndicator is now called from UpdateCountBadge (3×), ApplyCountToTrackedButton (2×), PruneDeadTrackedButtons, TrackTaskbarButton and CleanupOnTaskbarThread, and ApplyCountToTrackedButton also re-asserts IsVisible(false) inline instead of going through HideRunningIndicatorForBottomDots. It's correct as far as I can tell, but a single "reconcile the indicator for this button" helper called once per update would make the invariant much easier to verify later. In the same vein, RunningIndicatorHideResult::NotPresent and ::Hidden are treated identically by the only caller, so the enum could just be a bool.

  • [[clang::no_destroy]] on g_trackedButtons isn't needed (line ~392). TrackedButton holds only winrt::weak_refs, PODs and no thread-affine or out-of-process resources — a weak ref release is an in-process refcount decrement, so the automatic destructor is safe at process shutdown and the attribute (plus the std::optional wrapper) is suppression you don't need. See https://github.com/ramensoftware/windhawk/wiki/Global-objects-and-process-shutdown for which types actually require it. A plain std::list<TrackedButton> g_trackedButtons; would also remove a small hazard: TrackedButtons() dereferences the optional unconditionally, and CleanupOnTaskbarThreadProc disengages it while hooks are still installed (only g_unloading checks keep that from being reached).

  • Consider refreshing the README screenshots. showcase.png presumably still shows only the left/right dot variants; adding the new top and bottom placements (bottom especially, since it replaces the native indicator) would make the new options obvious. The image URLs both resolve, so this is just a content update in your own repo.

  • The VerticalDots settings key is now a misnomer for top/bottom. Keeping the key is the right call (renaming would reset everyone's settings) — maybe just a short comment next to ReadStringSetting(L"VerticalDots.position") so it doesn't look like a copy/paste slip later.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

  • Bottom mode drops the active/inactive cue. Windows uses the running indicator's length (and the ActiveRunningIndicator visual state) to show which app is in the foreground; replacing it with N uniform dots loses that, leaving only the button's background highlight. If you want to keep it, the state is cheap to read — see taskbar-elastic-pill.wh.cpp#L1450 and taskbar-blob-shape.wh.cpp#L845 — and could drive a different dot color or size for the active button.

  • The transient-fallback retry is unbounded. bottomDotsUsedFallback disables the lastAppliedCount/generation fast path, so as long as RunningIndicator reports zero size, every UpdateVisualStates call and every deferred refresh redoes the full path: RebuildDots clears and recreates the dot Borders and BindDotPositionExpression restarts both expression animations. Normally this converges after layout, but if some state leaves the indicator sized 0 while the count is > 0, that becomes permanent churn (and visible dot flicker) for that button. A small retry counter in TrackedButton — after N attempts, accept the below-icon position and clear the flag — would bound it.

  • Dot placement is computed once and cached by count + settings generation. dotStack.Margin is a static value in button coordinates; only the button-follow transform is live. So a geometry-only change — DPI change, dragging a window to a monitor with a different scale, another mod changing the taskbar/icon size — can leave the dots offset until the count or the settings change forces a re-apply. Including the button/icon bounds (or the indicator bounds in bottom mode) in the early-out condition would catch it without adding XAML handlers, which you're deliberately avoiding.

  • VerticalDots.size has no upper bound (std::max(2, ...) only). At large values the top row's dotY = iconY - kDotGap - dotSize goes negative and the horizontal row gets wider than the button, so dots can spill outside the button and over its neighbours. A std::clamp with a sane maximum (or clamping the final dotX/dotY into the button bounds) would keep it tidy.

  • If the taskbar-thread cleanup can't run on unload, a RunningIndicator hidden by bottom mode stays hidden until Explorer restarts — disabling the mod doesn't bring it back. Your comment in Wh_ModBeforeUninit already covers this and the three-way fallback chain makes it unlikely; noting it only so it's visible in the review.


Next steps:

  • /ai-review - after pushing fixes, to get a review of the updated code. You can repeat this as many times as you need, but each review is thorough and usually there's no need for more than 2-3 iterations.
  • /ready-for-reviewer - once you're satisfied with the state of the pull request, to hand it over to a human reviewer. If some findings above are left unaddressed, add a short note explaining why.

See the review process for details.

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-author The author's turn: request an AI review, or respond to one that was posted. and removed waiting-for-ai-review An AI review was requested and is being prepared. labels Aug 29, 2026
@digart11

Copy link
Copy Markdown
Contributor Author

/ready-for-reviewer

@windhawk-reviewer windhawk-reviewer Bot added waiting-for-reviewer Ready for a human reviewer, and in the queue for one. and removed waiting-for-author The author's turn: request an AI review, or respond to one that was posted. labels Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-reviewer Ready for a human reviewer, and in the queue for one.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant