Skip to content

Commit 561df60

Browse files
The operator can maintain the twelve palette colors of each theme from Settings, with a measured contrast readout and a reset to default (#3577, arm B) (#3606)
* The operator can maintain the twelve palette colors of each theme from Settings, with a measured contrast readout and a reset to default (#3577, arm B) Arm (B) of #3577. Arm (A) (#3589) fixed the two contrast defects the report named; this is the request itself: user-maintainable colors with a reset to default, in Lite and the Darling viewer. - A per-user theme-overrides.json (Lite: beside settings.json; the viewer: beside viewer-settings.json, local to the machine), keyed by theme then by palette color, holding only what the operator changed. - A closed list of twelve of the eighteen palette colors, declared once in PerformanceMonitor.Ui (ThemeColorOverrides.Slots) with the reason the other six are withheld; pinned in Lite.Tests. - ThemeManager regenerates a theme that carries overrides from its embedded XAML text (ThemeXamlRewriter) instead of merging an override dictionary, because the theme's own brushes and every style setter resolve their {StaticResource} at load and a merged dictionary cannot reach them. The compiled BAML stays the path for a theme without overrides. A bad file, key, value or parse falls back to the stock theme with a logged warning; nothing here can take the app down. - A Colors section (shared ThemeColorsPanel) under each Settings window's theme combo: swatch, hex, HSV picker, and the WCAG ratio against the surface each color renders on, colour-coded 4.5 / 3. Warns, never refuses. Apply / Reset to default / Open file; a FileSystemWatcher re-applies outside edits. - WcagContrast brings arm (A)'s out-of-tree script into the Ui project; the tests pin it to the numbers #3589 published. - Docs: Lite README "Themes and colors", a paragraph in the Darling README's viewer section. No Dashboard port (frozen twin). * The watcher's re-read of theme-overrides.json is a log line on any failure, never a throw (#3577 review) Review on #3606: ReloadFromDiskIfChanged caught only IOException, and it runs from a DispatcherTimer.Tick on the UI thread with no caller above it - an UnauthorizedAccessException from an ACL change, an AV scan or a sync client's lock would have crashed the app over a file watcher, against the class header's own contract. - The read is behind ThemeManager.OverridesFileReader (File.ReadAllText in the app); IOException still retries up to MaxReloadRetries, and anything else - or retries exhausted - is LogWarning + the theme stays as it was. The method returns what it did (ReloadOutcome). - The Tick handler is the last line of defence: a broad catch around the whole reload, for whatever nobody anticipated in the rest of the path. - Pinned in Lite.Tests/ThemeColorOverrideTests: UnauthorizedAccess -> Failed, one warning naming it, loaded overrides untouched; IOException retries to the cap then warns; unchanged text is a no-op; no file configured is a no-op. * The regeneration test reads its brushes on the STA thread that built them (#3577 CI) First CI run: all six RegeneratingATheme_MovesEveryBrushTheXamlDerivesFromAnOverriddenColor cases failed reading SolidColorBrush.Color from the runner's thread - the parsed brushes are unfrozen DispatcherObjects owned by the STA thread. The product regeneration itself parsed cleanly with no warnings on every theme; the test now extracts plain Color structs inside the STA body and asserts on those. No product change.
1 parent 4087d27 commit 561df60

18 files changed

Lines changed: 2928 additions & 11 deletions
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/*
2+
* Copyright (c) 2026 Erik Darling, Darling Data LLC
3+
*
4+
* This file is part of the SQL Server Performance Monitor.
5+
*
6+
* Licensed under the MIT License. See LICENSE file in the project root for full license information.
7+
*/
8+
9+
using System;
10+
using System.IO;
11+
using System.Runtime.CompilerServices;
12+
using System.Text.RegularExpressions;
13+
using PerformanceMonitor.Ui;
14+
using Xunit;
15+
16+
namespace Darling.Tests;
17+
18+
/// <summary>
19+
/// #3577 (arm B) as it lands in the Darling viewer. The loader, the key list, the rewriter and the
20+
/// regeneration are shared <c>PerformanceMonitor.Ui</c> code and are pinned once, in
21+
/// <c>Lite.Tests/ThemeColorOverrideTests</c>, against the theme files of BOTH apps. What is the viewer's own
22+
/// is where its file lives and where it does not: <c>theme-overrides.json</c> sits beside
23+
/// <c>viewer-settings.json</c> under the viewer's per-user directory, and the preference is LOCAL to the
24+
/// machine — it never goes to the store or the control plane, because a color is a preference of the
25+
/// person at this screen and not a fact about the fleet. A viewer seat on another machine keeps its own.
26+
/// </summary>
27+
public sealed class ViewerThemeColorOverridesTests
28+
{
29+
private static readonly string ViewerDir = Path.Combine("Darling", "PerformanceMonitor.Darling.Viewer");
30+
31+
[Fact]
32+
public void TheOverridesFile_SitsBesideViewerSettings_UnderTheViewersOwnDirectory()
33+
{
34+
var app = File.ReadAllText(Path.Combine(RepoRoot(), ViewerDir, "App.xaml.cs"));
35+
36+
Assert.Contains("Path.GetDirectoryName(ViewerAppSettingsStore.DefaultFilePath())", app, StringComparison.Ordinal);
37+
Assert.Contains("ThemeColorOverrides.FileName", app, StringComparison.Ordinal);
38+
Assert.Equal("theme-overrides.json", ThemeColorOverrides.FileName);
39+
}
40+
41+
/// <summary>
42+
/// The preference is not a settings-model property, not a store column and not a control-plane knob.
43+
/// <c>ViewerAppSettings</c> keeps <c>ColorTheme</c> (which theme) and nothing about the colors inside
44+
/// it; the service and storage projects never see the word.
45+
/// </summary>
46+
[Fact]
47+
public void TheColorOverrides_AreNotPersistedInTheSettingsModel_TheStore_OrTheControlPlane()
48+
{
49+
var root = RepoRoot();
50+
51+
var settingsModel = File.ReadAllText(Path.Combine(root, ViewerDir, "ViewerAppSettings.cs"));
52+
Assert.DoesNotContain("ThemeColorOverride", settingsModel, StringComparison.Ordinal);
53+
Assert.DoesNotContain("theme-overrides", settingsModel, StringComparison.Ordinal);
54+
55+
foreach (var project in new[] { "PerformanceMonitor.Darling.Storage", "PerformanceMonitor.Darling.Service" })
56+
{
57+
var directory = Path.Combine(root, "Darling", project);
58+
foreach (var file in Directory.EnumerateFiles(directory, "*.cs", SearchOption.AllDirectories))
59+
{
60+
if (file.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}", StringComparison.Ordinal))
61+
{
62+
continue;
63+
}
64+
65+
var text = File.ReadAllText(file);
66+
Assert.False(text.Contains("ThemeColorOverride", StringComparison.Ordinal) || text.Contains("theme-overrides", StringComparison.Ordinal),
67+
$"{Path.GetRelativePath(root, file)} mentions the viewer's color overrides; they are viewer-local and must stay out of the store and the service.");
68+
}
69+
}
70+
}
71+
72+
/// <summary>
73+
/// The viewer's Settings window hosts the same shared panel Lite does, beneath its theme combo, and the
74+
/// panel is the ONLY colors surface — no viewer-only copy of the rows that could drift from Lite's.
75+
/// </summary>
76+
[Fact]
77+
public void TheViewerSettingsWindow_HostsTheSharedPanel_AndNoPrivateCopy()
78+
{
79+
var xaml = File.ReadAllText(Path.Combine(RepoRoot(), ViewerDir, "SettingsWindow.xaml"));
80+
81+
Assert.Single(Regex.Matches(xaml, "<ui:ThemeColorsPanel"));
82+
Assert.True(xaml.IndexOf("x:Name=\"ColorThemeCombo\"", StringComparison.Ordinal) < xaml.IndexOf("<ui:ThemeColorsPanel", StringComparison.Ordinal));
83+
Assert.DoesNotContain("AccentForegroundColor", xaml, StringComparison.Ordinal); // no hand-rolled rows
84+
}
85+
86+
/// <summary>The viewer's own three theme files are the ones the shared regeneration reads — embedded under the loader's names.</summary>
87+
[Fact]
88+
public void TheViewerProject_EmbedsItsThemeText()
89+
{
90+
var csproj = File.ReadAllText(Path.Combine(RepoRoot(), ViewerDir, "PerformanceMonitor.Darling.Viewer.csproj"));
91+
92+
Assert.Contains("<EmbeddedResource Include=\"Themes\\*.xaml\" LogicalName=\"Themes/%(Filename).xaml\" />", csproj, StringComparison.Ordinal);
93+
foreach (var theme in ThemeColorOverrides.ThemeNames)
94+
{
95+
Assert.True(File.Exists(Path.Combine(RepoRoot(), ViewerDir, "Themes", theme + "Theme.xaml")),
96+
$"{ThemeManager.ThemeTextResourceName(theme)} names a file the viewer does not ship.");
97+
}
98+
}
99+
100+
private static string RepoRoot([CallerFilePath] string thisFile = "")
101+
=> Path.GetFullPath(Path.Combine(Path.GetDirectoryName(thisFile)!, "..", ".."));
102+
}

‎Darling/PerformanceMonitor.Darling.Viewer/App.xaml.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
using System;
10+
using System.IO;
1011
using System.Reflection;
1112
using System.Threading.Tasks;
1213
using System.Windows;
@@ -67,6 +68,17 @@ diagnostic the one guaranteed to vanish. Nothing between here and its old positi
6768
failure. */
6869
ViewerLogger.Initialize();
6970

71+
/* #3577: the operator's per-theme color overrides live beside viewer-settings.json, in the viewer's
72+
own per-user directory - LOCAL to this machine, never in the store or the control plane, because
73+
a color is a preference of the person at this screen and not a fact about the fleet. The path
74+
and the log hooks go in BEFORE the first Apply so the very first paint carries them (the same
75+
no-flash reasoning as the theme itself); the watcher that re-applies an outside edit starts
76+
right after. */
77+
ThemeManager.OverridesFilePath = Path.Combine(
78+
Path.GetDirectoryName(ViewerAppSettingsStore.DefaultFilePath())!, ThemeColorOverrides.FileName);
79+
ThemeManager.LogWarning = message => ViewerLogger.Warn("Theme", message);
80+
ThemeManager.LogInfo = message => ViewerLogger.Info("Theme", message);
81+
7082
/* Apply the saved color theme through ThemeManager (App.xaml merges Dark as the design-time
7183
default) so ThemeManager owns the app-level merged dictionary at runtime, before StartupUri
7284
creates MainWindow. Reads the viewer-local settings directly (cheap JSON read) so the very
@@ -81,6 +93,8 @@ on any error. Light / CoolBreeze are honored via the Settings window's theme sel
8193
ThemeManager.Apply("Dark");
8294
}
8395

96+
ThemeManager.WatchOverridesFile();
97+
8498
/* #1050 (companion to the ported tray's WindowResumeGuard): WPF's GPU render thread can zombie its
8599
surface across sleep/wake or RDP, leaving a live-but-blank window — now reachable in the viewer
86100
because minimize-to-tray can hide it. Software rendering removes the GPU dependency entirely; charts

‎Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,12 @@
6969
<ItemGroup>
7070
<Resource Include="EDD.ico" />
7171
</ItemGroup>
72+
73+
<ItemGroup>
74+
<!-- #3577: the three theme dictionaries a SECOND time, as plain text, under Themes/{Name}Theme.xaml -
75+
the same line Lite's project carries, for the same reason. The compiled BAML stays the load path
76+
for a theme with no color overrides; a theme WITH overrides is regenerated from this text by the
77+
shared ThemeManager (see ThemeXamlRewriter's header for why it must be the text). -->
78+
<EmbeddedResource Include="Themes\*.xaml" LogicalName="Themes/%(Filename).xaml" />
79+
</ItemGroup>
7280
</Project>

‎Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<Window x:Class="PerformanceMonitor.Darling.Viewer.SettingsWindow"
22
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
33
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
4+
xmlns:ui="clr-namespace:PerformanceMonitor.Ui;assembly=PerformanceMonitor.Ui"
45
Title="Settings"
56
Height="750" Width="750"
67
MinHeight="500" MinWidth="600"
@@ -254,6 +255,13 @@
254255
<ComboBoxItem Content="Cool Breeze" Tag="CoolBreeze"/>
255256
</ComboBox>
256257
</StackPanel>
258+
<!-- #3577: the operator's own colors for the theme the combo shows - the same shared panel
259+
Lite hosts (PerformanceMonitor.Ui.ThemeColorsPanel), reading and writing the viewer's
260+
own theme-overrides.json beside viewer-settings.json. Viewer-LOCAL: this preference
261+
never goes to the store or the control plane. Collapsed by default, as in Lite. -->
262+
<Expander Header="Colors" IsExpanded="False" Margin="0,10,0,0">
263+
<ui:ThemeColorsPanel Margin="0,8,0,0"/>
264+
</Expander>
257265
</StackPanel>
258266
</Border>
259267

‎Darling/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,8 @@ The aggregate tabs — **Overview** and **Alerts** span every server; **Recommen
10171017

10181018
Only the visible tab loads (Lite's visible-only rule). The Alerts tab and the visible server tab's active inner tab refresh every 60 seconds; the Overview refreshes on its own faster 30-second timer (Lite's Overview cadence); and **Recommendations** refreshes on tab activation, its Refresh button, and its own server-selector change only, never on the timer — its findings change on the service's 30-minute analysis cadence, so a 60-second auto-refresh would be pointless churn (and would reset the incident expanders under the reader), matching Lite. The one finding that can appear between passes is **same-statement pileup** (evaluated on the collection cadence — see the `alerts` section), so a live pileup shows up here on the next activation or Refresh rather than the instant it fires; its notification does not wait for either.
10191019

1020+
The viewer has the same three color themes as Lite — Dark, Light and Cool Breeze, picked from **Settings → Color theme** — and the same **Colors** section beneath that drop-down (#3577): one row per adjustable palette color of the theme on screen, with a swatch, the hex, a picker and the measured contrast ratio against the surface it renders on (green ≥ 4.50:1, amber 3.00–4.49, red below — a warning, never a gate). **Apply** writes and applies live, **Reset to default** returns that one theme to its shipped palette, **Open file** hands you the JSON. The file is `%APPDATA%\PerformanceMonitorDarling\theme-overrides.json`, beside `viewer-settings.json`, and it is **local to this machine**: a color is a preference of the person at the screen, so it lives in neither the store nor the control plane, and a second viewer seat keeps its own. The keys, the JSON shape and the parse rules are the same as Lite's — see [Themes and colors in Lite's README](../Lite/README.md#themes-and-colors).
1021+
10201022
The viewer is read-only over collected data, but it does perform a small set of **user-initiated writes** — and those go straight to the PostgreSQL store, which is the coordination point (the service honors them on its next read; there is no viewer-to-service channel). From the Alerts tab, creating a mute rule from an alert (**Mute This Alert** / **Mute Similar**) or adding, editing, toggling, deleting, or purging one via **Manage Mute Rules** writes `config_mute_rules` (a rule scopes to a server by name, exactly as Lite's mute rules do); and **dismissing alerts** sets the `dismissed` flag on `config_alert_log` so they drop out of the Alert History view (a single atomic UPDATE — Darling has no parquet archive tier, so there is no dismissed-archive sidecar). The viewer never writes collector data.
10211023

10221024
### Restart Semantics

0 commit comments

Comments
 (0)