added an option to change text color in consoleDriver window#4433
Conversation
WalkthroughThe changes introduce a color customization feature for console text in the ConsoleDriverWindow. The XAML file adopts enum-based bindings for type safety and adds a ColorButton control to the action bar. The code-behind implements color palette cycling, applies selected colors to console text and related brushes, integrates color choices with theme switching, and modifies console output rendering to preserve text color formatting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/Ginger/Drivers/DriversWindows/ConsoleDriverWindow.xaml.cs (1)
598-601: 🧹 Nitpick | 🔵 Trivial
ColorButtonis excluded from theforeachloop that styles all toolbar buttons.
ColorButtonis handled separately with a null check (Lines 602-608, 654-660), but it duplicates the same styling logic. Including it in the array would be simpler and less error-prone.Proposed refactor
- foreach (var b in new[] { RecordButton, NewActionButton, ThemeToggleButton, GoButton, TopButton }) + foreach (var b in new[] { RecordButton, NewActionButton, ThemeToggleButton, GoButton, TopButton, ColorButton }) { b.Style = roundStyle; b.Background = btnBG; b.BorderBrush = btnBorder; b.Foreground = btnText; } - if (ColorButton != null) - { - ColorButton.Style = roundStyle; - ColorButton.Background = btnBG; - ColorButton.BorderBrush = btnBorder; - ColorButton.Foreground = btnText; - }Apply in both
ApplyDarkThemeandApplyLightTheme.Also applies to: 652-653
🤖 Fix all issues with AI agents
In `@Ginger/Ginger/Drivers/DriversWindows/ConsoleDriverWindow.xaml`:
- Line 102: Replace the plain Content of ColorButton with the same structured
XAML the other toolbar buttons use: create an inline StackPanel containing an
ImageMakerControl (or a small Ellipse/colored dot matching the runtime colored
swatch) and a TextBlock label so the initial visual matches what
ApplyConsoleTextColor/InitializeCommonUI will set at runtime; keep the existing
x:Name="ColorButton", Click="ColorButton_Click", Style, Margin and tooltips, and
ensure the inline content uses the same sizing/margins as sibling buttons so
first-paint appearance is consistent with the other controls.
In `@Ginger/Ginger/Drivers/DriversWindows/ConsoleDriverWindow.xaml.cs`:
- Line 34: Remove the unused using directive `using System.Windows.Markup;` from
the top of ConsoleDriverWindow.xaml.cs (it’s not referenced anywhere in the
file); update any usings ordering if needed and run a build/IDE cleanup to
ensure no other references depend on `System.Windows.Markup`.
- Around line 76-90: Replace the fragile parallel arrays _consoleTextPalette and
_consoleTextPaletteNames with a single structured collection (e.g., an array or
list of a small record/struct or tuple like { Color, Name }) and update uses to
read the color and name from that single collection using
_consoleTextPalette[_consoleTextPaletteIndex].Color and .Name; update any code
that references the old arrays (including _consoleTextPaletteIndex) to use the
new combined type so the palette and its label cannot fall out of sync.
- Around line 73-74: The init-time cast in InitTextBlocks that sets _colorText
is invalid because ColorButton.Content starts as a string and later is replaced
by a TextBlock, so _colorText stays null; fix by removing the cached _colorText
and instead obtain the TextBlock from ColorButton.Content when needed (e.g.,
inside ApplyDarkTheme and ApplyLightTheme before calling SetValue) or change the
XAML to make the button content a named TextBlock (so you can reference it
directly); update ApplyConsoleTextColor, ApplyDarkTheme, and ApplyLightTheme to
read the live TextBlock from ColorButton.Content (or use the new named element)
before updating Foreground, and remove the _colorText field and its
InitTextBlocks assignment if you choose the on-demand approach.
- Around line 92-108: ApplyConsoleTextColor currently replaces
ColorButton.Content with a fresh TextBlock and sets Foreground on both Runs,
which discards theme styling; instead, modify the existing ColorButton.Content
TextBlock (or create one only if absent) and update only the dot Run's
Foreground to the palette brush while leaving the label Run without an explicit
Foreground so it inherits the button's themed Foreground; locate
ApplyConsoleTextColor, ColorButton.Content, the created
System.Windows.Documents.Run("● ") and Run($"Text Color: ...") and change the
logic to reuse/update the existing TextBlock.Inlines (or when creating new runs,
set Foreground only on the dot and omit Foreground on the label) so theme
changes still apply.
Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit