Color themes

[kristiankunc]

fixes #570

Adds theming options for UKCP specific windows, hot-reloadable from OP -> general settings

Previews:

Default

NOVA

NERC

iTEC

[Copilot]

Pull request overview

Introduces selectable colour themes (Default, NODE, NERC, NOVA, iTEC) for UKCP windows and harmonises the previously green ASR/MSL/Timer and Departure Coordination windows to use the same themed brushes. A Theme struct and themed GdiplusBrushes replace the previous fixed brush set, a new combo box in the General Settings dialog lets users pick a palette, and several renderers/displays are updated to consume the brushes from the container.

Changes:

• New Graphics::Theme data + themed GdiplusBrushes with LoadTheme(); persisted via a new colourPalette user setting and applied at startup and from the General Settings dialog.
• Refactored renderers (MinStack, Regional Pressure, Countdown, Hold, Wake, Approach Sequencer, Departure Coordination) and components (TitleBar::DrawTheme, CollapsibleWindowTitleBar, StandardButtons overloads) to use themed brushes/pens; row/title heights tightened from 20 → 15.
• Tests added for Theme/GdiplusBrushes and existing tests updated to inject brushes.

Reviewed changes

Copilot reviewed 68 out of 68 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/plugin/graphics/Theme.{h,cpp}	New theme definitions (Default/NODE/NERC/NOVA/iTEC).
src/plugin/graphics/GdiplusBrushes.{h,cpp}	Replaces fixed brushes with themed brushes + LoadTheme.
src/plugin/euroscope/GeneralSettings*	Adds palette setting key, combo box wiring, applies theme on save.
src/plugin/bootstrap/InitialisePlugin.cpp	Loads saved palette and applies theme at bootstrap.
src/plugin/components/{TitleBar,CollapsibleWindowTitleBar,StandardButtons}.{h,cpp}	Adds themed draw paths and themed button factories.
src/plugin/hold/HoldDisplay*, HoldModule.*	Propagates brushes through hold display construction; switches to themed brushes.
src/plugin/wake/WakeCalculatorDisplay., WakeModule.	Switches wake calculator to themed brushes.
src/plugin/approach/ApproachSequencerDisplay., ApproachBootstrapProvider.	Switches sequencer to themed brushes.
src/plugin/departure/DepartureCoordinationList.*, DepartureModule.cpp	Themes departure coordination list; adds dynamic content height.
src/plugin/minstack/MinStackRenderer., regional/RegionalPressureRenderer., countdown/CountdownRenderer.cpp	Switches MSL/RPS/Timer to themed brushes; ROW_HEIGHT 20→15.
src/plugin/radarscreen/RadarScreenFactory.cpp, ScreenControls.cpp	Plumbs brushes through bootstrap; updates ScreenControls to new field names.
resource/UKControllerPlugin.rc, resource.h	Adds IDC_COLOUR_PALETTE combo box.
src/plugin/CMakeLists.txt, .clang-format-ignore	Build/format housekeeping for new files.
test/plugin/**/*	New tests for Theme/GdiplusBrushes/StandardButtons/TitleBar; existing tests updated to pass brushes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[19wintersp]

The LLMs giveth and the LLMs taketh away...

I would recommend:

• eliminating those drawing methods that take Colors as parameters (since they just seem to immediately construct a Brush of that colour), which should allow removing the Color fields of GdiplusBrushes leaving only the Brushes (and a reasonable number of fields thereof);
• ditching the inconsistent prop drilling of the brushes in favour of a ThemeManager singleton (cf. FontManager); and
• renaming the Theme class to Palette (this one's my fault, sorry), and GdiplusBrushes to just Brushes, stored as a member brushes of ThemeManager.

This should help clean up the data flow paths somewhat. It is probably safe to skip the initialisation check in the implementation of ThemeManager::Instance for performance; alternatively, a utility ThemeManager::Brushes method could both bypass an initialisation check and return the brushes structure directly. Separating the concepts of the theme, brushes, and palette will also improve extensibility for future interface options.

There are also some refactors I'll probably make in the future to GeneralSettingsDialog to reduce repetition, particularly in handling the comboboxes.

Specific gripes with code formatting/layout:

• Includes should be kept alphabetically-sorted where they are already (we have the clang-format setting for this disabled, perhaps that can be changed?).
• There seem to be a lot of this->brushes without any apparent reason not to just use a plain field access (brushes).
• There are a few cases where random includes (mostly system includes?) have been added to header files, again without apparent reason.
• The repetitive comments in the definition of the theme constants is a bit annoying, since the hex codes duplicate information (you can construct Color with an ARGB constant if you prefer to use hex), and the comments just exemplify the ugliness of C++ plain initialisers (C actually has named initialisers, and C++ doesn't support them ☹️). If we really want to label the colours at instantiation, perhaps each theme should be its own class inheriting Theme, with a *(void) constructor? This would let you label the Colors as they are initialised in the initialiser list.

I haven't looked at tests and won't provide much other code-specific feedback, since there's not much point in doing so before a refactor.

Sorry to request so many changes; hopefully your assistant can work with this pointed feedback. I could work on these changes myself if agreeable, but there would be a few weeks' lead time before I'd be free to do so.

[kristiankunc]

This should help clean up the data flow paths somewhat. It is probably safe to skip the initialisation check in the implementation of ThemeManager::Instance for performance; alternatively, a utility ThemeManager::Brushes method could both bypass an initialisation check and return the brushes structure directly. Separating the concepts of the theme, brushes, and palette will also improve extensibility for future interface options.

This makes sense - could make the the theming much more future-proof

There are also some refactors I'll probably make in the future to GeneralSettingsDialog to reduce repetition, particularly in handling the comboboxes.

Yeah this has also bugged me a little but I could not figure out a smart way to shrink it down, I'll see again.

Includes should be kept alphabetically-sorted where they are already (we have the clang-format setting for this disabled, perhaps that can be changed?).

Didn't know that. I'll re-enable the clang rule for it as most of the other files seem to obey it.

There are a few cases where random includes (mostly system includes?) have been added to header files, again without apparent reason.

Oh yeah, these probably got auto-imported while I accidentally pressed enter or copilot included them while I was typing trying to be smart. I'll remove those

The repetitive comments in the definition of the theme constants is a bit annoying, since the hex codes duplicate information (you can construct Color with an ARGB constant if you prefer to use hex), and the comments just exemplify the ugliness of C++ plain initialisers (C actually has named initialisers, and C++ doesn't support them ☹️). If we really want to label the colours at instantiation, perhaps each theme should be its own class inheriting Theme, with a *(void) constructor? This would let you label the Colors as they are initialised in the initialiser list.

I'll find a better way for this - the comments are not great and very much redundant. I've kept them in to get a preview in vscode mainly.

[sonarqubecloud]

Quality Gate failed

Failed conditions
4 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE