feat(ui): NavigationView overhaul, Status dashboard, and a GitHub-dark palette - #70
Merged
Conversation
Records the framework, navigation, setting-model, elevation, testing and search decisions so they survive across sessions, plus a Decided against section for the status surfaces that were cut (managed count, last-scan timestamp, pause reason, pause persistence, suspended-set exposure). Drifted count is the only status surface.
The UI overhaul needs to count drift per section, which requires knowing which section a setting belongs to. Nothing in the code recorded that: section membership existed only as which Load*() method built the row and which TabItem hosted it. Adds SettingSectionMap.SectionFor(id) -> SettingSection, covering the 46 global ids plus the prefixed families (service:, task:, ai.app:, hdr:, refresh:, resolution:, drr:). Unmapped ids return an explicit Unknown -- never a thrown exception, never a silent default. Prefix parsing is not duplicated: it is extracted out of SettingDocsCatalog.Get into SettingDocsCatalog.ParseId, which both now call, so the two can never disagree about what "service:foo" means. Get's behavior is unchanged (its 172 existing tests pass untouched). Section membership follows the Load*() method and hosting TabItem, not the AppConfig grouping comments. Two places they disagree, documented inline: faststartup/visualfx sit under a "System toggles" comment with powerthrottling but are built onto the Global gaming tab, while powerthrottling is on CPU / Power; netthrottle is stored in AppConfig's ungrouped block but has been presented on the Network tab since v0.1.46. The table deliberately knows nothing about navigation groups -- grouping is a shell concern. Per-section counts (asserted, so a new catalog entry without a section decision fails the build): Gaming 13, Display 4, CpuPower 3, Telemetry 6, WindowsAi 13, Network 3, Debloat 8, Services 33, Bios 0, Unknown 0.
The UI needs a drifted count per section without re-running CheckDrift. MonitorService already computes exactly that set each scan and then threw it away. Exposes three members and nothing else: CurrentDrift (read-only, keyed by setting id), a DriftChanged event, and the pure MergeDrift the two are built on. Purely additive -- the diff removes no line, and scanning, applying, backoff and the circuit breaker are untouched. Two things the merge has to get right: Tier scoping. A tick only re-checks its own tier, so only that tier's entries are authoritative; the rest carry forward. Replacing wholesale would flush the ~40 stable settings on every 30-second display poll and flicker the count to near-zero. Same reasoning the circuit-breaker recovery sweep already uses. Auto-applied settings. A setting that drifted at scan start and was auto-applied and verified during the same tick is no longer drifting, so it is dropped from the snapshot. Otherwise the count would report drift the app had already corrected, for up to a full stable backstop (10 minutes). Uses the Verified flags ChangeApplier already returned -- nothing is re-read. DriftChanged fires only when the set of drifted ids changes, so a quiet machine raises no events. It is raised on the poll-timer thread, like AutoAppliedRebootRequired, so UI handlers must marshal. MergeDrift is pure and unit-tested headlessly, following the SelectNotifiable precedent in the same file, including a test that the snapshot groups cleanly through SettingSectionMap into per-section counts.
…install A beta build must be safe to run beside a stable install and must never be able to update itself. Adds a BETA compile constant, set with -p:Beta=true, and routes everything that has to differ through one new type, Services/AppIdentity.cs. Mechanism is an MSBuild property with a default, not a fourth Build Configuration: beta builds are Release builds plus one constant, so the Debug/Release matrix, the determinism settings and every publish flag stay as they are. Both paths must compile warning-free independently because TreatWarningsAsErrors is on and #if-excluded code is invisible to the other compile -- verified locally on each file, and CI builds both. Gated on BETA: - Config root moves to %APPDATA%\GamerGuardian-Beta. ConfigStore and ChangeLogger built that path independently, so moving one would have left the other in the stable root; both now call AppIdentity. As part of that, the session-header line that reported the config path by inferring it from the change log's own directory now reports the real path -- the inference assumed the two always share a folder. - Mutex name gets a .Beta suffix so both instances can run at once. - The update path is compiled out, not disabled at runtime: the startup check, CheckForUpdatesAsync itself, and the body of the Settings "Check now" handler. The button and its XAML are untouched, so there is no orphaned Click target and no unused handler -- the handler simply reports that updates are disabled. A beta binary contains no code that can reach the update feed. - Window title, title bar and tray tooltip carry " [BETA <sha>]", parsed from the InformationalVersion the beta workflow stamps as "<base>-beta.<sha>" (AssemblyVersion/FileVersion must stay a pure a.b.c.d, so the suffix can only live there). - HKCU Run value becomes GamerGuardian-Beta. A distinct name rather than extending the dev-build skip: launch-at-startup is a feature testers need to exercise, and a shared name would have the two builds overwrite each other's entry. - %TEMP% diagnostics become gamerguardian-beta_*, so two running instances never interleave into one log, and TempCleanup's patterns move with them. TempCleanup's installer sweep is compiled out under BETA so a beta build cannot delete a stable install's in-flight download. Non-beta behavior is unchanged. Every gated member resolves to the exact literal the code used before, enforced by AppIdentityTests rather than asserted: the test project compiles without BETA, so a beta-only value leaking into the default build fails the suite. The only XAML change is an x:Name on the existing TitleBar so the marker can be appended in code; markup is otherwise identical between flavors.
Builds the portable self-contained single-file EXE with the BETA constant set and uploads it as a workflow artifact. Nothing else. Trigger is workflow_dispatch only -- no push trigger, no tag trigger -- so it cannot fire as a side effect of committing, merging or tagging. The workflow token is contents: read at workflow scope with no job-level elevation, so it structurally cannot create a release, push a tag, or write to the repository even if a step tried to. release.yml needs contents/id-token/attestations write to publish; this has none of them. No installer is built and no release action is referenced. Publish flags are identical to release.yml plus -p:Beta=true. Version stamping follows dev-build.yml: a numeric base for AssemblyVersion and FileVersion, which must stay a pure a.b.c.d, and an explicit InformationalVersion of "<base>-beta.<sha>" that AppIdentity parses back for the in-app BETA marker. Stable tags only when picking the bump base. Two guards beyond the ask: the suite must pass before a build goes to testers (which also proves the non-BETA path still compiles warning-free), and the published binary is scanned for the update-feed URL so a run fails loudly if the BETA constant ever stops taking effect. Artifact name carries the run number and short sha.
…-field loading Two config-safety changes. First launch of a beta build now starts from the stable install's settings instead of defaults: if the beta root does not exist and the stable one has a config.json, it is copied across. Strictly one-way -- SeedConfigFrom reads the source and only ever writes under the target, so a beta build can never modify the stable config. It no-ops when the two directories are the same (a stable build), when the target already exists (not a first launch), or when there is nothing to copy, and a failure just leaves the caller on defaults. ConfigStore.Load catches everything and returns a fresh AppConfig on failure, which means a deserialization problem wipes every setting silently. Adds tests pinning that a config.json carrying unknown fields -- from an older or newer build -- keeps its known values rather than taking that branch, at both the top level and nested under global. The malformed-JSON reset is also pinned, so the lossy path stays a deliberate, documented behavior instead of an accident. To make Load and Save testable at all, ConfigStore gains a constructor taking an explicit directory; the default constructor is unchanged and still resolves through AppIdentity.
…le to fail The binary scan added in beta.yml was false assurance. Verified empirically: it passes against a NON-BETA published EXE too, so it could never have detected a leak. Two independent reasons. EnableCompressionInSingleFile means the managed assemblies inside publish/GamerGuardian.exe are compressed, so no literal from them appears in the file's bytes -- scanning the shipped EXE finds nothing in either flavor. And "compiled out" was overstated. #if BETA gated the two call sites but left UpdateService and UpdateAvailableWindow in the assembly, URL literal included, with no PublishTrimmed to drop them. The code was present and callable, merely uncalled. Measured on the uncompressed assembly, the update URL was in both flavors. Both are now fixed rather than the check being deleted. The csproj drops Services/UpdateService.cs, UI/UpdateAvailableWindow.xaml and its code-behind from the compile when Beta=true. Every reference to them already sat in a NON-BETA region, so no source change was needed. Measured after: the beta assembly is 22 KB smaller and contains none of the update URL, CheckLatestAsync, or UpdateAvailableWindow. A beta build now genuinely cannot reach the update feed, which is what Part Two asked for. beta.yml scans the managed assembly that gets bundled instead of the compressed EXE, using UTF-16 because .NET stores string literals in the #US heap as UTF-16 -- the earlier scan's UTF-8 pass would have missed the literal even uncompressed. build.yml gains a beta-compile job. It builds the BETA flavor on every PR, which matters because beta.yml is dispatch-only and TreatWarningsAsErrors plus #if-excluded code means the beta path can otherwise rot unnoticed between manual runs. It also builds the stable flavor and asserts the differential: URL present in stable, absent in beta. Asserting only "absent in beta" would pass against a scan that can never match anything, which is precisely the trap the original check fell into, so the job fails loudly if the stable side ever stops matching.
It was persisted in AppConfig, copied in AppConfigCloner, and bound to a "Group multiple drifts into one notification" checkbox, but nothing ever read it -- Notifier does not consult it. Users were offered a toggle that did nothing. Removed from all four places. Every existing config.json still carries the field, so the removal turns it into an unknown property on upgrade. ConfigStore.Load catches everything and returns a fresh AppConfig on failure, so if unknown fields were fatal this would silently wipe every user's settings. They are not -- System.Text.Json ignores them -- and there is now a test pinning exactly that case by name, plus one confirming the field is dropped on the next Save.
…igned literals The scan decoded the assembly as UTF-16 from offset 0. Literals live in the #US heap at arbitrary byte offsets, so an odd-aligned literal decodes to garbage and is never seen -- the check was a coin flip on every build. Found while verifying the BETA gating at binary level: the " [BETA]" marker literal reported as absent from a beta build, which was wrong. It is odd-aligned. The update URL happened to be even-aligned, which is the only reason the differential passed. A recompile shifting it odd would have made the guard silently pass on a real leak. Both scans now decode at offset 0 and offset 1 and match on either. Re-verified: update URL present in stable, absent in beta, marker present in beta only.
The contract for the NavigationView overhaul. Records, per surface: what it displays, the config keys it reads and writes, whether applying elevates, and what test coverage exists today. The finding that matters most: UI surfaces have zero automated coverage. No test references any window type. The compiler and the suite will not catch a lost surface during the rewrite, so this document is the only check and verification has to be manual, entry by entry. Flags the highest-risk items to lose -- both are read-only information screens that nothing would fail without: the CPU/Power tab's plan comparison chart and CCD dependency card, and the entire Recommended BIOS tab, which hosts no managed setting and so will never show a drift count.
…tract 10 views One window, WPF-UI 3.0.5 NavigationView, grouped by user intent rather than subsystem: Status pinned above Performance (Gaming, Display, CPU and power), Privacy (Telemetry, Windows AI, Network), Cleanup (Debloat, Services), Reference (BIOS), with General in the footer. Each tab became its own UserControl under UI/Views rather than moving the TabControl wholesale. SettingsWindow.xaml drops from 966 lines to 120; the 851 lines of tab content moved verbatim into the views, so no markup was rewritten in the same step that relocated it. The two shared resources (CopyableLearnMoreText, ToggleRowTemplate) moved to UI/SharedResources.xaml and are merged app-level -- they lived on SettingsWindow, which only worked while every tab was inside that window. Views are long-lived fields, not per-navigation constructions, so scroll and expander state survive moving between sections and the Load*() methods keep a stable target. Navigation is a content swap via NavigationView.ReplaceContent, so no page service or DI container is needed. Interactive views forward their handlers to SettingsWindow, which still owns the draft and every apply path -- the extraction split the XAML without also rewriting 2,300 lines of working behavior at the same time. New Status view is the payoff for the section map and the published drift set: aggregate drifted count, per-section counts via SettingSectionMap, and the pause toggle. Nothing else -- no managed count, last-scan timestamp, pause reason, or pause persistence, all recorded as decided against. OnClosed now detaches Status from the monitor before releasing the tree; those handlers sit on a long-lived service and would otherwise keep the whole window graph alive.
Adds installer/GamerGuardian-Beta.iss, modeled on the stable script but
distinct in every identity that would otherwise collide: AppId GUID, AppName,
DefaultDirName, Start Menu group, uninstall entry, and output filename. Per
user install like stable.
Two collisions the stable script's shape would have caused:
Both flavors ship an executable named GamerGuardian.exe, so the uninstall
step's Stop-Process by name would have killed a running STABLE install while
uninstalling the beta. It now filters on image path under {app}.
UninstallDelete and the Run-value cleanup target the beta's own config root
and Run value, so uninstalling the beta cannot take the stable install's
settings or startup entry with it.
The test project also gains a beta flavor. `dotnet test -p:Beta=true` used to
fail to compile, because the beta app compile removes UpdateService while
UpdateServiceTests still referenced it; that file is now excluded under BETA.
The identity tests were then asserting the stable literals against a beta
build and failing an app that was behaving correctly -- they are now
flavor-aware, so the beta run positively asserts the beta identity (separate
config root, .Beta mutex, own Run value, visible marker) rather than skipping
the question.
Verified on the built artifact: update URL absent from the beta assembly,
BETA marker present, beta run creates %APPDATA%\GamerGuardian-Beta containing
both config.json and changes.log, stable root untouched.
The side pane was completely dead -- no nav item responded to a click. NavigationView.SelectionChanged never fires for these items. They carry no TargetPageType, because navigation here is a content swap via ReplaceContent rather than a page service, and WPF-UI's internal navigate returns before raising the event. Nothing was listening to anything. NavigationViewItem derives from ButtonBase, so each item now handles Click directly, which is independent of the navigation machinery. The pane highlight is driven by hand for the same reason: NavigationView normally maintains IsActive as part of navigating, so with navigation bypassed nothing cleared the previously active item and the highlight would have stuck on Status permanently. Also widens the window from 900x720 to 1150x760 (min 920x560). The pane takes 210px that the old TabControl never did, which squeezed the per-setting name and description column badly enough to clip -- "Advertisin g ID" wrapping mid-word, "Current: Dis...", "Recommende..." truncated. The extra width restores roughly the content area the tabs had. Verified by running the app and clicking through: content swaps, the highlight follows, and the row template resolves from the app-level resource dictionary.
Windows PowerShell 5.1's Get-Content reads using the system ANSI codepage, not UTF-8. The extraction script that split the tabs into views therefore read each em-dash (UTF-8 E2 80 94) as three CP1252 characters and wrote them back as UTF-8, so "power scheme — High Performance" shipped as "power scheme â€" High Performance". Visible in the running app on the CPU / Power page. Three lines were affected, in CpuPowerView, NetworkView and SharedResources. Repaired in place; the em-dash count now matches the pre-extraction original exactly (3), and no other mojibake signature remains. Caught by looking at the running beta rather than by any build or test -- the corruption is valid UTF-8, just wrong characters, so nothing downstream had any reason to complain.
Borrows Sparkle's dashboard shape -- a grid of stat cards with tinted icon chips -- but not its card set. Each card here is context for something GamerGuardian actually manages: Processor model + which tuning recipe matched Graphics adapter + VRAM (new detection) Memory installed physical RAM (new detection) Windows edition + version/build, which gate several policies Displays count + primary panel's resolution and refresh Power plan active scheme vs the CPU-aware recommendation No storage card. Sparkle has one because it ships a junk cleaner; this app manages nothing about disks, so it would be decoration rather than context. The drift count stays the hero at the top -- it remains the only status surface, per the recorded decision. GPU and memory detection are both new. GPU comes from the display-class registry key rather than WMI, and memory from GlobalMemoryStatusEx, so the app keeps its pure P/Invoke-plus-registry shape and takes no new package dependency. This also closes the GPU-detection gap flagged in the Sparkle assessment, where HAGS is the obvious future consumer. Two bugs found by running it rather than by building it: The registry's ProductName still reads "Windows 10 ..." on Windows 11 -- Microsoft never updated the value -- so the card showed "Windows 10 Pro" on a build-26200 machine. CorrectEdition rewrites it above the 22000 boundary, which matters for an app that only supports Windows 11. An accent brush key was built as "<stem>BrushBackground" instead of "<stem>BackgroundBrush", which threw ResourceReferenceKeyNotFound and took the window down on open. Keys are now composed correctly and resolved through a fallback, so a missing theme brush degrades to a plain chip instead of crashing. 13 new tests covering the formatters, the edition fix, card shape, and the guarantee that the readers degrade to "Unknown" rather than throwing on unpredictable hardware.
…l the Memory card
Three complaints about the Status dashboard, one root cause between the first two.
Uniform size and level icons. The cards were ui:Card, whose template centres its
content presenter, so the one-row Memory card floated its header lower than its
two-row neighbours and the six icons did not line up. Swapped to a plain Border
that stretches into its UniformGrid cell, with a fixed-height (34px) header grid
so every icon and every first data row starts at the same offset. Subtitles
ellipsize rather than wrap so one long one cannot push a single header taller.
Too dark. ui:Card's dark-theme fill is 5% white, which measured 50/255 against a
39/255 page -- near-invisible. ControlFillColorSecondaryBrush measures 58 and
takes a CardStrokeColorDefaultBrush outline, so the cards actually read as cards.
Both brushes are WPF-UI theme brushes, so light theme still works.
Black text. Dropping ui:Card also dropped the Foreground its template supplied,
and TextBlock's own default is Black -- every value in the grid rendered black on
dark. Verified by sampling the capture: glyphs inside the cards measured 0-58
while text elsewhere reached 255. The Border now sets TextElement.Foreground
explicitly and descendants inherit it.
Memory card. It carried a single "Total" row and looked empty next to its
neighbours. It now reads the SMBIOS table via GetSystemFirmwareTable('RSMB') --
plain P/Invoke, no WMI, no new package -- and reports installed size with the
memory type plus the module layout and speed: "31.2 GB DDR5" / "2 x 16 GB @ 5600
MT/s". Configured speed wins over rated speed because that is what the machine
actually runs at, and the slowest module wins when they disagree. Mismatched
capacities are listed rather than collapsed, since a mismatched pair is worth
noticing.
The parse is a pure function over a byte buffer with 20 tests covering the
extended-size sentinel, the kilobyte unit bit, empty and unknown slots, short
SMBIOS 2.1 structures, string-set walking, end-of-table, and 200 rounds of random
garbage that must not throw.
721 stable / 703 beta tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The colour scheme is now GitHub dark rather than stock WinUI: canvas #0d1117, cards #151b23 on a #3d444d border, text #f0f6fc / #9198a1, and Primer's semantic green, amber and red for the status surfaces. Accent is Primer's #1f6feb, handed to WPF-UI's accent manager so the derived variants stay in the family. UI/Themes/GitHubDark.xaml holds the mapping. WPF-UI's theme dictionary is flat -- every key is a concrete brush, not a reference to a smaller set of primitives -- so recolouring means restating the keys the control templates actually read. The Primer tokens are declared once at the top and referenced by name so a palette tweak stays a one-line change. Two things had to be worked out by running the window rather than reading source: Merging the dictionary was not enough. WPF searches a ResourceDictionary's own entries before any of its merged ones, and WPF-UI writes part of the chrome into Application.Resources directly. A merged palette recoloured the content area and left the title bar, navigation pane and footer grey. ThemeService now copies the entries in at the same level and removes them again for the light theme, which restores whatever the theme dictionary underneath says. The window background is not usable either. FluentWindow's Background is overwritten at load so the backdrop can show through -- setting it to literal Red in XAML changed nothing on screen, which is how this was pinned down. The five windows now paint their root Grid instead, and drop Mica for WindowBackdropType None, which is also the setting the perf notes call for. Light theme is deliberately left stock: the request was for GitHub's dark scheme, and a half-translated light palette would look worse than what WPF-UI ships. Also switches the This PC cards to CardBackgroundFillColorDefaultBrush now that the palette makes it an opaque #151b23 with a real border, so they match every other card in the app. 721 stable / 703 beta tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"This PC" on the Status page was pure black -- glyphs measured 0-13 brightness against a background of 13. Same defect as the card values fixed in 28b6f27, one level up: a bare TextBlock has no Foreground of its own and WPF's default is Black, so any text that is not inside a control which sets one (ui:Card does, a Border does not) renders black. The earlier fix only covered the card contents, not the heading sitting directly in the view. Fixed at the root of all eleven views rather than per-element, so the whole class is closed instead of one instance: descendants inherit it, and anything with its own Foreground still wins. The five windows get the same treatment on their root grid -- ApplyResults, Notification and RebootPending appear at moments where unreadable text would be worst, and they had never been looked at under the new palette. Drops the now-redundant TextElement.Foreground from the This PC card border so there is one mechanism rather than two. 721 stable / 703 beta tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Renames the Unreleased heading per the process documented at the top of the file, and describes the overhaul in the terms a user experiences it: a home screen, a grouped sidebar instead of tabs, the This PC summary, and the GitHub-dark recolour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebuilds the settings window around a grouped
NavigationView, adds a Status home screen, and recolours the app with GitHub's Primer dark palette.What changed
Navigation. The
TabControlis gone. Eleven views are extracted into their own files underUI/Views/, grouped in the pane under Performance / Privacy / Cleanup / Reference, with General in the footer.docs/feature-inventory.mdwas written before the extraction and used to verify afterwards that nothing was lost — including the read-only information screens.Status page. Drifted count in aggregate and per section, a pause toggle, and a "This PC" grid: processor, graphics, memory, Windows, displays, power plan. Each card is context for settings the app manages, which is why there is no storage card.
Memory detail. New SMBIOS reader (
GetSystemFirmwareTable('RSMB')— plain P/Invoke, no WMI, no new package) reporting installed size with type plus module layout and speed. The parse is pure and has 20 tests covering the extended-size sentinel, the kilobyte unit bit, empty and unknown slots, short SMBIOS 2.1 structures, string-set walking, end-of-table, and 200 rounds of random garbage that must not throw.Palette.
UI/Themes/GitHubDark.xamlmaps Primer's dark tokens onto the keys WPF-UI's control templates read. Light theme is left stock.Beta channel.
-p:Beta=truedefines aBETAconstant that excludes the update path from the compile entirely,AppIdentityputs every per-flavor path behind it, andinstaller/GamerGuardian-Beta.issproduces an installer with a distinct AppId so a beta sits alongside a stable install.beta.ymlbuilds it;build.ymlasserts the differential so the guard can actually fail.Notes for review
Four defects in this branch were found only by running the app, not by building or testing it:
ReplaceContentbefore the template is applied. Initial navigation is deferred toLoaded.NavigationView.SelectionChangednever fires when items have noTargetPageType. Items are wired onClickinstead.ui:Cardrendered pure black, because a bareTextBlockdefaults to Black and nothing above a view supplies aForeground. Fixed at the root of all eleven views and the five windows.Testing
721 stable tests and 703 beta tests pass. Beta installer built and run on Windows 11 26200.