Control theme #176
Control theme #176
Conversation
…Porting-to-Control-Themes and it builds but I don't understand the style loading system. I'm going to check this in and come back to it sometime later.
# Conflicts: # src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml # src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml # src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlack.axaml # src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/Controls/DialogWindow.axaml
Datagrid selection does not work
📝 WalkthroughWalkthroughThe recent changes in the codebase primarily involve a restructuring of the XAML files across various controls and themes within the In the The changes also include the introduction of new resources, such as Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (62)
src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml (2)
Line range hint
10-121: Consider documenting the caret padding requirementThe comment "I don't understand why 1 is not sufficient as a padding for caret of width 1" suggests uncertainty about the implementation. Consider:
- Documenting the actual reason for using padding of 2
- Creating a named constant or theme resource for the caret padding
- <Panel Margin="0,0,2,0"> + <Panel Margin="{DynamicResource TextBoxCaretPadding}">
Line range hint
1-129: Consider extracting common theme valuesThe theme implementation is solid, but consider extracting common values (padding, margins, brushes) into a separate resource dictionary for better reusability across different controls in the TurboVision theme.
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml (1)
67-121: Consider documenting the significance of converter parameters.The menu scroller implementation is solid, but the converter parameters (0 and 100) would benefit from documentation explaining their significance in the scrolling behavior.
Consider adding a comment explaining these values:
<MultiBinding Converter="{x:Static converters:MenuScrollingVisibilityConverter.Instance}" - ConverterParameter="0"> + ConverterParameter="0"> <!-- Represents top scroll threshold --> <MultiBinding Converter="{x:Static converters:MenuScrollingVisibilityConverter.Instance}" - ConverterParameter="100"> + ConverterParameter="100"> <!-- Represents bottom scroll threshold -->src/Consolonia.Themes.TurboVision/Templates/Controls/ContentControl.axaml (1)
3-4: Remove unused namespacesThe
helpersanddrawingnamespaces are imported but not used in this template. Consider removing them unless they're planned for future use.<ResourceDictionary xmlns="https://github.com/avaloniaui" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" - xmlns:helpers="clr-namespace:Consolonia.Themes.TurboVision.Templates.Controls.Helpers;assembly=Consolonia.Themes.TurboVision" - xmlns:drawing="clr-namespace:Consolonia.Core.Drawing;assembly=Consolonia.Core">src/Consolonia.Themes.TurboVision/Templates/Controls/CaretControl.axaml (2)
Line range hint
35-46: Consider documenting the caret positioning behavior.The Line element uses a custom
MoveConsoleCaretToPositionBrushwhich seems to handle caret positioning. This might benefit from documentation explaining the interaction between the brush and console positioning.<Line Margin="{TemplateBinding Padding}" HorizontalAlignment="Left" VerticalAlignment="Top" Name="PART_CaretLine" Width="1" Height="1" IsHitTestVisible="False" IsVisible="{TemplateBinding IsCaretShown}" - StrokeThickness="1"> + StrokeThickness="1" + ToolTip.Tip="Handles console caret positioning through MoveConsoleCaretToPositionBrush">
Remove commented code and document the TextBlock hack implementation
The search results confirm this is an isolated hack specific to the CaretControl's CheckBox rendering. While the workaround exists, it appears to be the only instance of such a hack in the codebase. The commented-out code serves no purpose and should be removed.
- Remove the commented
TextBlock.Backgroundsection as it's not being used- Consider adding a more detailed comment explaining:
- What specific rendering issue this hack addresses
- Why a TextBlock with Width=1 and Height=1 fixes the Line redrawing
- Any attempted alternatives that didn't work
🔗 Analysis chain
Line range hint
12-23: Review the TextBlock hack implementation.The TextBlock is used as a workaround for Line redrawing issues in CheckBox. This seems like a fragile solution that might cause maintenance issues in the future.
Additionally, there's commented-out code that should either be removed or documented explaining why it's kept.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar workarounds in other controls to understand the scope of this issue rg -l "This TextBlock is a hack" --type xaml # Look for related CheckBox rendering issues in the codebase rg "CheckBox.*redraw" --type csLength of output: 109
Script:
#!/bin/bash # Search for similar workarounds in other controls using .axaml extension rg -l "This TextBlock is a hack" --type-add 'axaml:*.axaml' --type axaml # Look for related CheckBox rendering issues with .axaml extension rg "CheckBox.*redraw" --type-add 'axaml:*.axaml' --type axaml # Search for similar TextBlock hacks or workarounds rg -A 5 "TextBlock.*hack" --type-add 'axaml:*.axaml' --type axaml # Look for any other references to rendering or redrawing issues rg "redraw" --type-add 'axaml:*.axaml' --type axaml --type csLength of output: 2530
src/Consolonia.Themes.TurboVision/Templates/Controls/Separator.axaml (2)
1-4: Remove unused namespace declarationsThe
helpersanddrawingnamespaces are not used in this file and can be safely removed.<ResourceDictionary xmlns="https://github.com/avaloniaui" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" - xmlns:helpers="clr-namespace:Consolonia.Themes.TurboVision.Templates.Controls.Helpers;assembly=Consolonia.Themes.TurboVision" - xmlns:drawing="clr-namespace:Consolonia.Core.Drawing;assembly=Consolonia.Core"> + >
Line range hint
11-17: Remove redundant HorizontalAlignment and consolidate height/margin values
HorizontalAlignment="Stretch"is redundant as it's the default value for Border- The Height and Margin values in the template (Height="2", Margin="-1,0") conflict with the style values (Height="1", Margin="1,0")
<ControlTemplate> <Border BorderBrush="{TemplateBinding Foreground}" BorderThickness="{TemplateBinding BorderThickness}" - Height="2" - Margin="-1,0" Background="{TemplateBinding Background}" - HorizontalAlignment="Stretch" /> + /> </ControlTemplate>src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml (2)
Line range hint
5-28: Consider documenting the CaretControl's purpose and behaviorWhile the template structure is well-organized, adding XML documentation for the custom
CaretControlwould improve maintainability by explaining its role in focus management and any specific behaviors developers should be aware of.
29-34: Clean up or document the commented background setterThe commented background setter using
ThemeSelectionBackgroundBrushshould either be removed or documented to explain why it's kept as a reference.src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml (2)
37-41: Clean up or document commented stylesThere are commented-out styles that should either be:
- Removed if they're no longer needed
- Implemented if they're planned features
- Documented if they're kept for reference
Line range hint
1-51: Good migration to ControlTheme structureThe transition from Styles to ControlTheme improves the organization and maintainability of the theme system. This structured approach:
- Centralizes control-specific styling
- Makes theme inheritance and overrides more explicit
- Follows Avalonia's recommended patterns for theme organization
src/Consolonia.Themes.TurboVision/Templates/Controls/TabControl.axaml (3)
Line range hint
5-29: Consider adding accessibility propertiesWhile the template structure is well-organized, consider enhancing accessibility by adding ARIA properties and keyboard navigation support.
Add these properties to improve accessibility:
<ControlTheme x:Key="{x:Type TabControl}" TargetType="TabControl"> + <Setter Property="AutomationProperties.Role" Value="TabList" /> + <Setter Property="KeyboardNavigation.TabNavigation" Value="Once" /> <Setter Property="Template">
31-56: Consider adding consistent margins for better spacingWhile the tab placement styles are well-structured, consider adding consistent margins to improve the visual spacing between tabs and content.
Add margin setters for each placement:
<Style Selector="^[TabStripPlacement=Top] /template/ ItemsPresenter#PART_ItemsPresenter"> <Setter Property="DockPanel.Dock" Value="Top" /> + <Setter Property="Margin" Value="0 0 0 4" /> </Style>
Line range hint
1-59: Consider extracting common theme values to resourcesTo maintain consistency across the TurboVision theme, consider extracting common values (like border thickness and spacing) to shared theme resources.
This would improve maintainability and ensure visual consistency across controls. Consider creating a separate resource file for these common values:
<!-- ThemeConstants.axaml --> <ResourceDictionary xmlns="https://github.com/avaloniaui" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"> <Thickness x:Key="TabControlBorderThickness">1</Thickness> <Thickness x:Key="TabItemSpacing">0,0,0,4</Thickness> </ResourceDictionary>src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml (1)
42-67: Consider combining duplicated Right placement styles.There are two separate style blocks for
TabStripPlacement=Right. These could be combined for better maintainability.Consider combining them like this:
- <Style Selector="^[TabStripPlacement=Right]"> - <Setter Property="HorizontalContentAlignment" - Value="Right" /> - </Style> - - - <!--TabStripPlacement--> <Style Selector="^[TabStripPlacement=Top]"> <Setter Property="Margin" Value="0,0,-1,-1" /> </Style> <Style Selector="^[TabStripPlacement=Bottom]"> <Setter Property="Margin" Value="0 -1 -1 0" /> </Style> <Style Selector="^[TabStripPlacement=Left]"> <Setter Property="Margin" Value="0 0 -1 -1" /> </Style> <Style Selector="^[TabStripPlacement=Right]"> + <Setter Property="HorizontalContentAlignment" + Value="Right" /> <Setter Property="Margin" Value="-1 0 0 -1" /> </Style>src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml (1)
111-112: Remove commented-out codeThe template contains commented-out Foreground setters that should be either removed or implemented if needed.
- <!--<Setter Property="(TemplatedControl.Foreground)" - Value="{DynamicResource ThemeBackgroundBrush}" />-->Also applies to: 118-119
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml (2)
59-114: Consider using Opacity for better performance.The current implementation toggles visibility for state changes, which can trigger layout recalculations. Consider using Opacity for elements that frequently change states (like Background and SelectedBackground) to potentially improve performance.
<Style Selector="^ /template/ Rectangle#Background"> - <Setter Property="IsVisible" Value="False" /> + <Setter Property="Opacity" Value="0" /> </Style> <Style Selector="^:pointerover /template/ Rectangle#Background"> - <Setter Property="IsVisible" Value="True" /> + <Setter Property="Opacity" Value="1" /> </Style>Also applies to: 120-127
115-119: Remove or document commented code.The commented-out style for today's date should either be removed if it's no longer needed or documented with a reason for keeping it as a reference.
src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml (1)
14-18: Consider removing or documenting commented codeThe commented-out transparency fallback code should either be removed if it's no longer needed, or documented to explain why it's being preserved for future use.
- <!--<Border Name="PART_TransparencyFallback" IsHitTestVisible="False" Tag="thth"> - <Border.Background> - <notImportant:ConsoleBrush Color="DarkGreen"/> - </Border.Background> -</Border>-->src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml (2)
Line range hint
1-49: Well-structured theme implementationThe transition from Styles to ResourceDictionary with ControlTheme is a good architectural decision that:
- Improves maintainability through better organization
- Follows modern Avalonia practices
- Provides clear separation of concerns
Add XML documentation to explain the FocusOnLoad behavior
The
MenuExtensionsclass lacks XML documentation for its important functionality. The code shows thatFocusOnLoadis an attached property that:
- Automatically focuses the first MenuItem when the control is loaded
- Manages menu item closure when focus moves outside the menu hierarchy
- Handles cleanup of event subscriptions
Add XML documentation to explain these behaviors:
- Document the
MenuExtensionsclass purpose- Document the
FocusOnLoadPropertyattached property- Document the
OnAttachedToVisualTreemethod🔗 Analysis chain
Line range hint
18-47: Consider documenting the custom focus behaviorThe template includes a custom focus management behavior through
MenuExtensions.FocusOnLoad. Consider adding XML documentation to theMenuExtensionshelper class to explain this functionality for future maintainers.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if MenuExtensions helper class has documentation ast-grep --pattern 'class MenuExtensions { $$$ FocusOnLoad $$$ }'Length of output: 73
Script:
#!/bin/bash # Let's try to find the MenuExtensions class file first fd MenuExtensions # Then let's search for FocusOnLoad in the codebase rg "FocusOnLoad" -A 5 -B 5Length of output: 7195
Script:
#!/bin/bash # Let's check if there are any XML documentation comments for the MenuExtensions class cat src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/MenuExtensions.csLength of output: 3224
src/Consolonia.Themes.TurboVision/Templates/Controls/TextBlock.axaml (1)
5-11: Use proper XAML commenting styleThe commented code uses HTML-style comments. Consider using proper XAML commenting style if you need to keep this reference temporarily.
- <!--<ControlTheme x:Key="{x:Type TextBlock}" TargetType="TextBlock"> - <Setter Property="Foreground"> - <Setter.Value> - <notImportant:ConsoleBrush Color="DarkRed"/> - </Setter.Value> - </Setter> - </Style>--> + <!-- Reference implementation: + <ControlTheme x:Key="{x:Type TextBlock}" TargetType="TextBlock"> + <Setter Property="Foreground"> + <Setter.Value> + <ConsoleBrush Color="DarkRed"/> + </Setter.Value> + </Setter> + </ControlTheme> -->src/Consolonia.Themes.TurboVision/Templates/Controls/Menu.axaml (2)
64-64: Address the TODO: Refactor Copied StylesThere's a TODO comment indicating that the styles are copied from
MenuItem. Consider refactoring to avoid duplication and promote reuse, possibly by creating a base style or using style inheritance.Would you like assistance in refactoring these styles to eliminate duplication and enhance maintainability?
26-28: Review the Use of TemplateBinding CommentsThe comment on line 26 mentions a workaround for a ReSharper issue. Ensure that this workaround doesn't introduce any unintended side effects and consider updating the comment for clarity.
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml (1)
118-151: Clean Up Commented-Out CodeThere is a block of commented-out code between lines 118-151, including styles for
RepeatButtonandThumbstates. If this code is obsolete, consider removing it to improve code readability. If it's temporarily disabled, add comments explaining why and when it might be re-enabled.src/Consolonia.Themes.TurboVision/Templates/Controls/DataGrid.axaml (1)
144-149: Remove Unnecessary Commented-Out BindingsLines 144-149 contain commented-out bindings referencing
asdf:DataGridExtensions.IsSelected, which may be outdated or incorrect. Removing them will reduce confusion and clean up the code.src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.cs (2)
7-8: Add XML documentation for the theme class.Since this is a public class that serves as a base theme, adding XML documentation would help developers understand its purpose and usage.
+ /// <summary> + /// Provides the base TurboVision theme implementation for Consolonia. + /// </summary> public class TurboVisionTheme : Styles
9-12: Consider adding null safety for XAML loading.The
AvaloniaXamlLoader.Loadcall might need null safety considerations. Also, the constructor should be documented to explain the optional service provider parameter.+ /// <summary> + /// Initializes a new instance of the TurboVision theme. + /// </summary> + /// <param name="sp">Optional service provider for XAML loading.</param> public TurboVisionTheme(IServiceProvider sp = null) { - AvaloniaXamlLoader.Load(sp, this); + try + { + AvaloniaXamlLoader.Load(sp, this); + } + catch (Exception ex) + { + throw new InvalidOperationException("Failed to load TurboVision theme resources.", ex); + } }src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkTheme.cs (1)
7-8: Document theme-specific characteristics.Add documentation to explain how this dark theme variant differs from the base theme and what specific styling it provides.
+ /// <summary> + /// Provides the dark variant of the TurboVision theme, implementing specific color schemes + /// and styling appropriate for dark mode displays. + /// </summary> public class TurboVisionDarkTheme : Stylessrc/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackTheme.cs (1)
1-1: Consider implementing a theme registration system.The current implementation of themes shows good consistency, but could benefit from a more structured registration system. Consider implementing:
- A theme registry service to manage theme switching
- A base abstract theme class to enforce consistent implementation
- Theme metadata for better discovery and management
This would improve theme management and make it easier to add new themes in the future.
Would you like me to provide a detailed implementation proposal for this enhancement?
src/Tools/Consolonia.PreviewHost/App.axaml (1)
5-7: Consider documenting available theme optionsThe commented TurboVisionDark theme suggests multiple theme options are available. Consider adding a comment block documenting all available themes and their use cases to help other developers.
src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/Controls/Button.axaml (1)
3-10: Well-structured theme implementation using ControlThemeGood architectural improvement moving from direct Style to ControlTheme. This approach:
- Follows modern Avalonia theming practices
- Maintains style inheritance through BasedOn
- Supports runtime theme changes via DynamicResource
src/Consolonia.Themes.TurboVision/Templates/Controls/Panel.axaml (1)
1-4: Remove unused namespace importsThe
helpersanddrawingnamespace imports appear to be unused in this file.<ResourceDictionary xmlns="https://github.com/avaloniaui" - xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" - xmlns:helpers="clr-namespace:Consolonia.Themes.TurboVision.Templates.Controls.Helpers;assembly=Consolonia.Themes.TurboVision" - xmlns:drawing="clr-namespace:Consolonia.Core.Drawing;assembly=Consolonia.Core"> + xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">src/Consolonia.Themes.TurboVision/Themes/Material/MaterialTheme.axaml (1)
2-3: Consider adding Design.PreviewWith for developmentThe Design.PreviewWith section has been removed. While this doesn't affect runtime behavior, it might impact the development experience. Consider adding a preview section with common controls for easier theme development.
src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.axaml (1)
4-6: Remove or populate empty Design.PreviewWith sectionThe Design.PreviewWith section is empty. Either remove it entirely or populate it with relevant controls for development purposes.
src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/DataGridExtensions.cs (1)
16-19: Consider adding validation in SetIsSelected.The setter method could benefit from null validation to prevent potential runtime exceptions.
public static void SetIsSelected(DataGridRow element, bool value) { + if (element == null) + throw new ArgumentNullException(nameof(element)); element.SetValue(IsSelectedProperty, value); }src/Consolonia.Themes.TurboVision/Templates/Controls/ItemsControl.axaml (1)
3-4: Remove unused namespace declarationsThe
helpersanddrawingnamespace declarations are not utilized in this template. Consider removing them to maintain clean and minimal XAML.<ResourceDictionary xmlns="https://github.com/avaloniaui" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" - xmlns:helpers="clr-namespace:Consolonia.Themes.TurboVision.Templates.Controls.Helpers;assembly=Consolonia.Themes.TurboVision" - xmlns:drawing="clr-namespace:Consolonia.Core.Drawing;assembly=Consolonia.Core">src/Consolonia.Themes.TurboVision/Templates/Controls/UserControl.axaml (2)
1-4: Consider using ResourceDictionary for consistencyThe template uses a Style root element while other templates in the TurboVision theme use ResourceDictionary. Consider aligning with the pattern used in other templates for consistency.
-<Style xmlns="https://github.com/avaloniaui" - xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" - Selector=":is(UserControl)"> +<ResourceDictionary xmlns="https://github.com/avaloniaui" + xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml">
5-21: Fix indentation for better readabilityThe indentation is inconsistent throughout the file. Some elements use 6 spaces while others use 8 spaces.
- <ControlTheme TargetType="UserControl" - x:Key="TurboVisionUserControl"> + <ControlTheme TargetType="UserControl" + x:Key="TurboVisionUserControl">src/Consolonia.Themes.TurboVision/Templates/Controls/OverlayPopupHost.axaml (1)
3-4: Remove unused namespace declarationsThe
helpersanddrawingnamespace declarations are not utilized in this template. Consider removing them to maintain clean and minimal XAML.<ResourceDictionary xmlns="https://github.com/avaloniaui" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" - xmlns:helpers="clr-namespace:Consolonia.Themes.TurboVision.Templates.Controls.Helpers;assembly=Consolonia.Themes.TurboVision" - xmlns:drawing="clr-namespace:Consolonia.Core.Drawing;assembly=Consolonia.Core">src/Consolonia.Gallery/App.cs (1)
26-30: Consider implementing a theme switching mechanism instead of commenting out themesThe commented-out themes suggest a need for runtime theme switching. Consider implementing a proper theme switching mechanism instead of using comments to toggle themes.
Example approach:
public App() { var selectedTheme = ConfigurationManager.AppSettings["Theme"] ?? "Material"; Styles.Add(GetTheme(selectedTheme)); } private IStyle GetTheme(string themeName) => themeName switch { "TurboVision" => new TurboVisionTheme(), "TurboVisionBlack" => new TurboVisionBlackTheme(), "TurboVisionDark" => new TurboVisionDarkTheme(), "Fluent" => new FluentTheme(), _ => new MaterialTheme() };src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml (1)
19-22: Clean up or document commented code sectionsSeveral sections are commented out (transparency fallback, chrome overlay). If these are planned features, consider:
- Adding TODO comments with tracking issues
- Documenting the intended functionality
- Removing if no longer needed
Also applies to: 23-26
src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml (1)
Line range hint
27-31: Consider making ScrollBarsWidth configurableThe ScrollBarsWidth is hardcoded to 1. Consider making this value configurable through theme resources for better flexibility across different themes and display configurations.
- <Setter Property="(helpers:ScrollViewerExtensions.ScrollBarsWidth)" - Value="1" /> + <Setter Property="(helpers:ScrollViewerExtensions.ScrollBarsWidth)" + Value="{DynamicResource ThemeScrollBarWidth}" />src/Consolonia.Themes.TurboVision/Templates/Controls/FlyoutPresenter.axaml (3)
Line range hint
19-21: Make shadow offset configurableThe shadow offset is hardcoded (
2,1,0,0). Consider making these values theme-dependent for better customization across different themes.- Margin="2,1,0,0" + Margin="{DynamicResource ThemeShadowOffset}"
Line range hint
33-38: Review content alignment property redundancyThe ContentPresenter sets both alignment and content alignment properties. This might lead to confusion:
HorizontalAlignment="{TemplateBinding HorizontalContentAlignment}" VerticalAlignment="{TemplateBinding VerticalContentAlignment}" HorizontalContentAlignment="Stretch" VerticalContentAlignment="Stretch"Consider simplifying by using only one set of alignment properties if possible.
Line range hint
1-43: Consider creating a central theme constants fileMultiple files contain hardcoded values for visual properties (margins, widths, offsets). Consider creating a central theme constants file to maintain these values consistently across all controls.
This would:
- Make theme-wide changes easier
- Ensure consistency across controls
- Simplify theme customization
- Make it easier to implement different variants (e.g., compact vs. comfortable sizing)
src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml (1)
46-67: Consider adding high contrast theme supportWhile the style selectors are correctly implemented using the new caret syntax, the control might benefit from high contrast theme support for better accessibility.
Add high contrast theme support:
+ <Style Selector="^:disabled:checked /template/ helpers|SymbolsControl#checkMark"> + <Setter Property="Foreground" Value="{DynamicResource ThemeHighContrastBrush}" /> + </Style>src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml (1)
45-47: Consider migrating UserControl.axaml to MergeResourceIncludeFor consistency, consider migrating the UserControl.axaml to use MergeResourceInclude like other controls.
- <StyleInclude Source="/Templates/Controls/UserControl.axaml" /> + <MergeResourceInclude Source="/Templates/Controls/UserControl.axaml" />src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axaml (2)
48-87: Consider using OpacityMask for better performanceThe current implementation uses multiple Rectangle elements with IsVisible property changes. Consider using OpacityMask for more efficient visual state changes.
- <Rectangle Name="Background" - Fill="{TemplateBinding Background}" /> + <Border Name="Background" + Background="{TemplateBinding Background}"> + <Border.OpacityMask> + <VisualBrush> + <VisualBrush.Visual> + <Rectangle Fill="White"/> + </VisualBrush.Visual> + </VisualBrush> + </Border.OpacityMask> + </Border>
License Compliance Issue: Remove or Replace MS-PL Licensed Code
The Calendar controls appear to be derived from Microsoft's code (likely from Silverlight/WPF) and are under MS-PL license, which is incompatible with the project's MIT license. These files need immediate attention:
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axamlsrc/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDatePicker.axamlsrc/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axamlEither:
- Remove the MS-PL licensed code and implement the calendar controls from scratch
- Obtain explicit permission from Microsoft to use under MIT license
- Keep the MS-PL license headers and properly document the dual-licensing in the project
🔗 Analysis chain
Line range hint
1-7: Verify license complianceThe file contains Microsoft Public License (Ms-PL) header. Ensure this is compatible with the project's licensing terms and that we maintain proper attribution.
License Incompatibility Detected: MS-PL headers in MIT-licensed project
The project is licensed under MIT, but three files in the TurboVision theme contain Microsoft Public License (MS-PL) headers:
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axamlsrc/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDatePicker.axamlsrc/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axamlLet's verify if these files are derived from Microsoft's code:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for license consistency across the project echo "Checking for license headers..." rg -l "Microsoft Public License \(Ms-PL\)" echo "Checking project's main license..." if [ -f "LICENSE" ]; then cat LICENSE fiLength of output: 1535
Script:
#!/bin/bash # Check for any Microsoft-related comments or code patterns in these files echo "=== Checking Calendar* files for Microsoft origins ===" rg -C 5 "Microsoft|ms-pl|silverlight|wpf" src/Consolonia.Themes.TurboVision/Templates/Controls/Calendar*.axamlLength of output: 3203
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDatePicker.axaml (2)
Line range hint
1-7: Preserve the copyright notice formattingThe Microsoft copyright notice should be properly formatted within the XML comment tags to maintain readability.
-<!-- -// (c) Copyright Microsoft Corporation. -// This source is subject to the Microsoft Public License (Ms-PL). -// Please see http://go.microsoft.com/fwlink/?LinkID=131993 for details. -// All other rights reserved. ---> +<!-- + (c) Copyright Microsoft Corporation. + This source is subject to the Microsoft Public License (Ms-PL). + Please see http://go.microsoft.com/fwlink/?LinkID=131993 for details. + All other rights reserved. +-->
86-90: Remove commented out styleThe commented-out style for focus state should be removed if it's no longer needed, or implemented if it's still required. Keeping commented code can lead to maintenance confusion.
- <!--<Style Selector="CalendarDatePicker:focus /template/ TextBox#PART_TextBox"> - <Setter Property="BorderBrush" - Value="{DynamicResource ThemeSelectionForegroundBrush}" /> - </Style>-->src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml (1)
56-59: Clean up commented stylesRemove commented-out styles if they're no longer needed. If these styles represent alternative implementations that might be needed later, consider documenting them in a separate design document.
- <!--<Style Selector="^ /template/ Panel helpers|SymbolsControl"> - <Setter Property="Foreground" - Value="{DynamicResource ThemeAccentBackgroundBrush}" /> - </Style>--> - <!--<Style Selector="^:disabled /template/ helpers|SymbolsControl"> - <Setter Property="Foreground" - Value="{DynamicResource ThemeNoDisturbBrush}" /> - </Style>-->Also applies to: 76-79
src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml (1)
Line range hint
26-27: Improve layout commentsThe current comments are too brief. Consider adding more descriptive comments explaining the purpose of each panel section.
- <!--Centered layout--> - <!--Panel shade Mode=Shaded--> + <!-- Main window layout with centered content --> + <!-- Shadow panel using shaded console brush for depth effect -->src/Consolonia.Themes.TurboVision/Templates/Controls/ProgressBar.axaml (2)
68-75: Remove commented-out codeThe commented-out
LayoutTransformControlstyle appears to be unused. Consider removing it to improve code maintainability.
85-104: Consider consolidating indeterminate animation stylesThe horizontal and vertical indeterminate animations share similar structure but differ only in the animated property (Width vs Height). Consider extracting common animation properties to reduce duplication.
Example refactor:
+ <Style Selector="^:indeterminate /template/ Border#PART_IndeterminateIndicator"> + <Style.Animations> + <Animation Duration="0:0:1.5" + IterationCount="Infinite" + Easing="LinearEasing" /> + </Style.Animations> + </Style>Also applies to: 105-126
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarItem.axaml (2)
Line range hint
35-38: Address TODO comment regarding button colorThe TODO comment suggests that the button might need a specific color. This should be addressed to ensure consistent styling.
Would you like me to help implement the color styling for the pressed state of CalendarHeader buttons?
1-5: Well-structured theme control implementationThe transition to
ResourceDictionaryandControlThemeacross all files follows a consistent pattern and modern XAML practices. This structure will improve maintainability and make the theming system more robust.Also applies to: 1-4, 1-4
src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml (3)
Line range hint
25-100: Consider refactoring shadow implementation to reduce duplication.The shadow implementation uses multiple
SymbolsControlelements with similar properties but slightly different configurations. This could be simplified by:
- Creating a dedicated Shadow control
- Using a templated control for different shadow positions
Example shadow control structure:
<helpers:ShadowControl Position="Bottom"> <helpers:ShadowControl.Template> <ControlTemplate> <helpers:SymbolsControl Fill="True" Text="{TemplateBinding ShadowText}" Classes="shaded" IsVisible="{TemplateBinding IsVisible}" Foreground="{DynamicResource ThemePseudoShadeBrush}" /> </ControlTemplate> </helpers:ShadowControl.Template> </helpers:ShadowControl>
Line range hint
1-138: Consider theme inheritance and customization strategy.The current implementation tightly couples the button's visual design to the TurboVision theme. Consider:
- Extracting common button behaviors into a base theme
- Implementing a theme override mechanism for custom applications
- Documenting the expected visual behavior and customization points
Remove commented-out Foreground setter and document DelayPress value
The commented-out Foreground setter using
ThemeBackgroundBrushappears to be part of a broader pattern in the codebase where similar setters are commented out in other controls (Menu, MenuItem, DataGrid). Since this is a consistent pattern across multiple controls and none of them actively use this setter, it should be safely removed.The
DelayPressvalue of 0.25 seconds is used for click animation timing, as evidenced by theButtonExtensions.csimplementation which uses this value to control the:clickdelayedpseudoclass. This is a standard interaction delay for button feedback.
- Remove the commented-out Foreground setter (lines 4-5)
- Add a comment above the DelayPress setter explaining:
<!-- Animation duration for button press feedback -->🔗 Analysis chain
Line range hint
4-24: Clean up commented code and document delay value.
- The commented-out Foreground setter should either be removed or documented explaining why it's preserved.
- The
DelayPressvalue of 0.25 seconds seems arbitrary. Consider documenting the rationale.Let's check if the commented Foreground setter is used elsewhere:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar Foreground setters in other control themes rg -l "ThemeBackgroundBrush.*Foreground" src/Consolonia.Themes.TurboVision/Templates/Controls/Length of output: 96
Script:
#!/bin/bash # Let's try a broader search for ThemeBackgroundBrush usage in control themes rg "ThemeBackgroundBrush" src/Consolonia.Themes.TurboVision/Templates/Controls/ -A 2 -B 2 # Also search for any Foreground setters in Button-related files rg "Foreground" src/Consolonia.Themes.TurboVision/Templates/Controls/Button -A 2 -B 2 # Check if there are any other button themes that might reference this fd "Button" src/Consolonia.Themes.TurboVision/Templates/Controls/ --exec cat {}Length of output: 31938
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (60)
src/Consolonia.Gallery/App.cs(2 hunks)src/Consolonia.Themes.TurboVision/Consolonia.Themes.TurboVision.csproj(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Calendar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDatePicker.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CaretControl.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBoxItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ContentControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/DataGrid.axaml(8 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/DataValidationErrors.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/FlyoutPresenter.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/DataGridExtensions.cs(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ItemsControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/LightDismissOverlaylayer.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Menu.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/OverlayPopupHost.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Panel.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ProgressBar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Separator.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/SymbolsControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TabControl.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TextBlock.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/UserControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.cs(1 hunks)src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.cs(1 hunks)src/Consolonia.Themes.TurboVision/Themes/Material/MaterialColors.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/Material/MaterialTheme.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/Material/MaterialTheme.cs(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/Controls/Button.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlack.axaml(0 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackColors.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackTheme.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackTheme.cs(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/Controls/Button.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/Controls/DialogWindow.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkTheme.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkTheme.cs(1 hunks)src/Tools/Consolonia.PreviewHost/App.axaml(1 hunks)
💤 Files with no reviewable changes (1)
- src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlack.axaml
✅ Files skipped from review due to trivial changes (1)
- src/Consolonia.Themes.TurboVision/Consolonia.Themes.TurboVision.csproj
🧰 Additional context used
📓 Learnings (2)
src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.axaml (1)
Learnt from: tomlm
PR: jinek/Consolonia#122
File: src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml:5-28
Timestamp: 2024-11-12T04:11:38.190Z
Learning: In the project, using `Mode="Shaded"` in `ConsoleBrush` within theme files is intentional and does not cause runtime issues.
src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkTheme.axaml (1)
Learnt from: tomlm
PR: jinek/Consolonia#122
File: src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml:5-28
Timestamp: 2024-11-12T04:11:38.190Z
Learning: In the project, using `Mode="Shaded"` in `ConsoleBrush` within theme files is intentional and does not cause runtime issues.
🔇 Additional comments (85)
src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml (2)
1-8: LGTM: Resource declarations are well-structured
The namespace declarations and converter resources are properly defined and follow XAML best practices.
122-127: LGTM: Disabled state styling is well-implemented
The disabled state styling follows modern XAML practices with the parent selector (^) and maintains consistency with the theming system.
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml (4)
1-5: LGTM: Resource dictionary declarations and imports are well-structured.
The namespace declarations are complete and properly organized, including all necessary dependencies for the control theme.
Line range hint 6-61: LGTM: Well-structured ScrollViewer theme with comprehensive functionality.
The control theme implementation is thorough and includes:
- Proper theme resource bindings
- Touch gesture support
- Accessible scroll behavior
- Flexible layout with proper grid definitions
62-65: LGTM: Disabled state properly handled.
The style correctly implements visual feedback for the disabled state using the modern selector syntax.
122-125: LGTM: Menu scroller symbols properly themed.
The style ensures consistent theming of scroll indicators with the rest of the UI.
src/Consolonia.Themes.TurboVision/Templates/Controls/SymbolsControl.axaml (1)
6-9: Verify the empty ControlTheme implementation
The ControlTheme appears to be empty with only a commented-out Foreground setter. This might lead to the control having no styling applied.
Let's check if this is intentional by looking for other style definitions for SymbolsControl:
If this is intentional, please add a comment explaining why the theme is empty. Otherwise, consider:
- Uncommenting the Foreground setter if it's needed
- Adding necessary styling properties for the control
- If the control doesn't need styling, document this decision
✅ Verification successful
Empty ControlTheme is intentional - control uses direct property registration
The empty ControlTheme is correct as the SymbolsControl class directly registers and handles its styling properties through code:
ForegroundPropertyis registered as a styled property inherited from TextBlockTextPropertyis registered as a direct propertyFillPropertyis registered as a styled property
The commented-out Foreground setter is redundant since the control already handles the Foreground property through code-based registration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other style definitions for SymbolsControl
rg -l "SymbolsControl" --type xaml
# Check if the control is used in the codebase
ast-grep --pattern 'class $_ : SymbolsControl'
Length of output: 112
Script:
#!/bin/bash
# Search for SymbolsControl in all files
rg -l "SymbolsControl"
# Get content of the SymbolsControl class definition
fd "SymbolsControl" --type f --exec cat {}
Length of output: 6432
src/Consolonia.Themes.TurboVision/Templates/Controls/ContentControl.axaml (1)
1-20: LGTM! Good modernization of the control theme structure
The transition from Style to ControlTheme within a ResourceDictionary follows modern Avalonia UI practices and aligns well with the theme control objectives. The ContentPresenter template maintains all necessary bindings while being more maintainable in the new structure.
Let's verify if this structure is consistent across other control templates:
✅ Verification successful
Let's verify the actual content of some files that show up in both searches to understand if they're in a transition state or if it's just detecting commented code or other non-problematic occurrences of "Style".
The modernization is correctly implemented, with expected Style selectors inside ControlTheme
The search results initially raised a concern, but deeper inspection shows the Style occurrences are actually correct usage of style selectors within ControlTheme definitions. This is the proper pattern in Avalonia UI where:
- The main control styling is defined using
ControlTheme - State-based styling is handled by nested
Styleselectors (e.g.,:disabled,:focus) within theControlTheme
The ContentControl template follows this same modern structure, consistent with other controls in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other control templates follow the same structure
# Expected: All control templates should use ControlTheme within ResourceDictionary
# Search for control templates that might not follow the new structure
rg -l "Style " "src/Consolonia.Themes.TurboVision/Templates/Controls/"
# Verify the consistent usage of ControlTheme
rg -l "ControlTheme" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
Length of output: 4611
Script:
#!/bin/bash
# Check content of a few representative files that appear in both searches
rg "Style|ControlTheme" "src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml" -C 2
rg "Style|ControlTheme" "src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml" -C 2
Length of output: 2571
src/Consolonia.Themes.TurboVision/Templates/Controls/CaretControl.axaml (3)
1-4: LGTM: Resource dictionary declarations are properly structured.
The namespace declarations and assembly references are correctly defined and follow proper XAML conventions.
5-5: LGTM: Control theme structure follows the new pattern.
The migration from <Styles> to <ResourceDictionary> with <ControlTheme> is implemented correctly, improving the organization of the theme system.
Also applies to: 47-48
Line range hint 24-29: LGTM: Content presenter implementation follows proper binding patterns.
The ContentPresenter is correctly configured with appropriate template bindings for content and alignment properties.
Also applies to: 30-34
src/Consolonia.Themes.TurboVision/Templates/Controls/Separator.axaml (1)
20-29: Verify ThemeNoDisturbBrush resource availability
The style references ThemeNoDisturbBrush as a dynamic resource. Let's ensure this resource is properly defined in the theme.
✅ Verification successful
ThemeNoDisturbBrush resource is properly defined
The resource ThemeNoDisturbBrush is correctly defined as a ConsoleBrush in multiple theme color files, including the default, dark, and material themes. The style's usage of this brush as a dynamic resource is valid and follows the theme's design system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ThemeNoDisturbBrush definition in resource dictionaries
rg -l "ThemeNoDisturbBrush" --type xaml
Length of output: 68
Script:
#!/bin/bash
# Search for ThemeNoDisturbBrush definition in all files since xaml extension is not recognized
rg -l "ThemeNoDisturbBrush"
Length of output: 1164
Script:
#!/bin/bash
# Let's check the actual definition of ThemeNoDisturbBrush in the colors files
rg "ThemeNoDisturbBrush" src/Consolonia.Themes.TurboVision/Templates/TurboVisionDefaultColors.axaml
rg "ThemeNoDisturbBrush" src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml
rg "ThemeNoDisturbBrush" src/Consolonia.Themes.TurboVision/Themes/Material/MaterialColors.axaml
Length of output: 467
src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml (3)
1-4: LGTM: Well-structured namespace declarations
The namespace declarations are properly organized and include all necessary references for the control template.
Line range hint 1-41: LGTM: Clean theme structure implementation
The migration to ControlTheme and ResourceDictionary is well-implemented, following Avalonia's conventions and maintaining proper separation of states and templates.
29-39: Verify theme brush resources
Please ensure that the following theme brushes are properly defined in the theme resources:
- ThemeChooserBackgroundBrush
- ThemeActionBackgroundBrush
✅ Verification successful
Theme brush resources are properly defined
Based on the search results, both ThemeChooserBackgroundBrush and ThemeActionBackgroundBrush are properly defined in multiple theme color files:
-
In TurboVisionDark theme (
TurboVisionDarkColors.axaml):- ThemeChooserBackgroundBrush: DarkCyan
- ThemeActionBackgroundBrush: DarkGreen
-
In TurboVisionBlack theme (
TurboVisionBlackColors.axaml):- ThemeChooserBackgroundBrush: DarkBlue
- ThemeActionBackgroundBrush: DarkMagenta
-
In Default theme (
TurboVisionDefaultColors.axaml):- ThemeChooserBackgroundBrush: Cyan
- ThemeActionBackgroundBrush: ForestGreen
-
In Material theme (
MaterialColors.axaml):- ThemeChooserBackgroundBrush: #0288D1
- ThemeActionBackgroundBrush: #4CAF50
-
In Fluent theme (
FluentColors.axaml):- ThemeChooserBackgroundBrush: #0078D4
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for brush definitions in theme resources
rg -A 1 'ThemeChooserBackgroundBrush|ThemeActionBackgroundBrush' --type xaml
Length of output: 107
Script:
#!/bin/bash
# Search for brush definitions in resource files with various extensions
rg -A 1 'ThemeChooserBackgroundBrush|ThemeActionBackgroundBrush' -g '*.axaml' -g '*.xaml'
# Also search in any potential theme-related files
fd -e axaml -e xaml | grep -i 'theme' | xargs cat
Length of output: 81969
src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml (2)
Line range hint 5-33: Well-structured control theme definition
The ControlTheme is well-defined with:
- Proper use of DynamicResource for theming
- Complete template bindings
- Standard layout properties
34-49: Verify the impact of selector syntax changes
The change from explicit ToggleButton:state to ^:state selector syntax is correct, but ensure all dependent styles are updated consistently.
✅ Verification successful
Selector syntax changes are consistent across the codebase
The only occurrence of the old ToggleButton: selector syntax is within a commented-out block, while the new ^: syntax is consistently used across all active styles in the codebase. The change is safe and properly implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining old-style selectors
rg "ToggleButton:(checked|pressed|disabled|pointerover)" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
# Check for consistent usage of new syntax
rg "\^:(checked|pressed|disabled|pointerover)" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
Length of output: 3299
src/Consolonia.Themes.TurboVision/Templates/Controls/TabControl.axaml (2)
1-4: LGTM: Namespace declarations are well-structured
The namespace declarations are properly organized, including both standard Avalonia/XAML namespaces and custom TurboVision-specific ones.
31-56: Verify the caret selector syntax compatibility
The modern caret (^) selector syntax is used throughout the styles. Let's verify compatibility with the target Avalonia version.
✅ Verification successful
The caret selector syntax is compatible with the codebase
The codebase is using Avalonia version 11.2.1, which fully supports the caret (^) selector syntax. This syntax was introduced in Avalonia 11.0 and has been a standard feature since then. The usage in the TabControl styles is correct and compatible.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the Avalonia version in use and when the caret syntax was introduced
rg -l "Avalonia" . | xargs rg "PackageReference.*Avalonia" || rg "<AvaloniaVersion>" || cat global.json
Length of output: 3697
Script:
#!/bin/bash
# Let's check the AvaloniaVersion variable definition in Directory.Build.props or similar files
fd "Directory.Build.props|Directory.Packages.props" . --exec cat {} \; || rg "<AvaloniaVersion>" .
Length of output: 721
src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml (3)
1-4: LGTM! Good improvement in resource organization.
The switch from <Styles> to <ResourceDictionary> aligns with modern Avalonia practices and improves modularity.
34-45: LGTM! Clean state-based styling implementation.
The updated selector syntax using ^ is more concise, and the styles properly handle different states with appropriate theme resources.
Line range hint 5-33: Verify the intention behind commented Background setter.
The Background setter is commented out without explanation. This might affect the visual appearance of TabItems.
Let's check if this is consistent with other controls:
✅ Verification successful
The commented-out Background setter appears to be intentional and not problematic
The search results show that TabItems are unique in having their Background setter commented out. This appears intentional because:
- Other controls that need backgrounds (ComboBox, CalendarDatePicker) explicitly set ThemeAlternativeBackgroundBrush
- TabItems inherit their background through the Template's Border element, which still binds to the Background property via TemplateBinding
- No other controls show a pattern of commented-out Background setters with ThemeAlternativeBackgroundBrush
The visual appearance is not affected since the Background property can still be set and is properly bound in the control template.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other controls in TurboVision theme have their Background setters commented out
rg -U 'Background.*ThemeAlternativeBackgroundBrush' 'src/Consolonia.Themes.TurboVision/Templates/Controls/*.axaml'
Length of output: 217
Script:
#!/bin/bash
# First, let's find the correct path to the Controls directory
fd -t d "Controls"
# Then, let's check for Background setters in all .axaml files
fd -e axaml . | xargs rg -U "Background.*Theme.*Brush"
# Also check for any comments about Background setters
fd -e axaml . | xargs rg -U "<!--.*Background.*-->"
Length of output: 3209
src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBoxItem.axaml (5)
1-4: LGTM: Proper namespace declarations
The namespace declarations are well-structured and include all necessary references.
1-1: LGTM: Proper migration to ControlTheme
The transition from <Styles> to <ResourceDictionary> with <ControlTheme> follows modern Avalonia styling patterns and improves maintainability.
Also applies to: 5-5, 50-50, 51-51
36-49: Verify the dynamic resource references
The styles use dynamic resources for backgrounds. Let's ensure these resources are properly defined:
- ThemeChooserBackgroundBrush
- ThemeActionBackgroundBrush
✅ Verification successful
Dynamic resources are properly defined and consistently used
Both ThemeChooserBackgroundBrush and ThemeActionBackgroundBrush are well-defined resources:
- Defined in all theme variants (TurboVisionDark, TurboVisionBlack, Material, Fluent)
- Defined in the default colors (TurboVisionDefaultColors.axaml)
- Consistently used across multiple controls with similar interaction patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the dynamic resources are defined in the theme
echo "Checking for resource definitions..."
rg "ThemeChooserBackgroundBrush|ThemeActionBackgroundBrush" "src/Consolonia.Themes.TurboVision/"
Length of output: 6083
30-35: Verify the intentional removal of pointer-over style
The pointer-over style has been commented out, which means there won't be any visual feedback when hovering over combo box items. This might impact user experience.
✅ Verification successful
Pointer-over styles are intentionally disabled across controls
The verification reveals a consistent pattern where pointer-over styles are commented out across multiple controls in the theme, including ToggleButton, ScrollBar, DataGrid, and the ComboBoxItem in question. This appears to be an intentional design decision for the TurboVision theme rather than an oversight, as it follows a consistent pattern across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other controls in the theme have their pointer-over styles
# to ensure consistency across the theme
echo "Checking pointer-over styles in other controls..."
rg -l "(:pointerover|:pointer-over)" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
Length of output: 705
Script:
#!/bin/bash
# Let's check the actual implementation of pointer-over styles in these files
# to understand if they use the same brush and if they're active (not commented out)
echo "Checking implementation of pointer-over styles..."
rg -C 2 "(:pointerover|:pointer-over)" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
Length of output: 5626
Line range hint 17-17: Verify CaretControl implementation
The template uses a custom CaretControl from the helpers namespace. Let's ensure it's properly implemented and handles the caret visibility correctly.
✅ Verification successful
Let me try a different approach to locate the CaretControl implementation.
Let me check the complete implementation of CaretControl to verify its functionality.
CaretControl implementation is properly implemented and functional
The CaretControl is correctly implemented as a ContentControl with:
- A proper
IsCaretShownproperty for visibility control - A well-defined template that includes both a fallback TextBlock and a Line with console caret positioning
- Appropriate template bindings and layout settings
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check CaretControl implementation
echo "Checking CaretControl implementation..."
ast-grep --pattern 'class CaretControl {
$$$
}'
Length of output: 140
Script:
#!/bin/bash
# Search for CaretControl in all files
echo "Searching for CaretControl files and usages..."
rg -l "CaretControl"
# Search for specific CaretControl class or control definition
rg -A 5 "class.*CaretControl|public.*class.*CaretControl"
Length of output: 1831
Script:
#!/bin/bash
# Get the complete CaretControl implementation
echo "Checking complete CaretControl implementation..."
cat "src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/CaretControl.cs"
# Also check its template definition
echo -e "\nChecking CaretControl template..."
cat "src/Consolonia.Themes.TurboVision/Templates/Controls/CaretControl.axaml"
Length of output: 3908
src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml (5)
1-5: LGTM: Resource dictionary header is well-structured
The namespace declarations are complete and properly reference all required assemblies.
7-7: LGTM: Converter registration is correct
The KeyGestureConverter is properly registered as a resource and is referenced correctly in the template.
100-131: LGTM: Style selectors are well-structured
The style selectors:
- Correctly use the new ^ syntax
- Cover all necessary states (separator, selected, focus, empty, disabled)
- Properly target template parts
Line range hint 1-132: Verify theme inclusion
Let's verify that this MenuItem control theme is properly merged into the theme's main resource dictionary.
✅ Verification successful
Theme inclusion verified successfully
The MenuItem control theme is properly included in the theme hierarchy:
MenuItem.axamlis included inAllControls.axamlvia<MergeResourceInclude Source="/Templates/Controls/MenuItem.axaml" />AllControls.axamlis included in the main theme via<StyleInclude Source="/Templates/Controls/AllControls.axaml" />inTurboVisionTheme.axaml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for MenuItem.axaml inclusion in theme resource dictionaries
echo "Searching for MenuItem.axaml inclusion..."
rg -l "MenuItem\.axaml" --type xaml
Length of output: 159
Script:
#!/bin/bash
# Search for any resource dictionary that might include MenuItem.axaml
echo "Searching for resource dictionary references..."
rg -l "MenuItem\.axaml|TurboVision.*\.xaml"
# Also search for potential theme-related files
echo -e "\nSearching for theme files..."
fd -e xaml -e axaml
Length of output: 6433
Script:
#!/bin/bash
# Check the content of AllControls.axaml for MenuItem.axaml inclusion
echo "Checking AllControls.axaml content..."
cat "src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml"
# Check main theme file for resource dictionary merging
echo -e "\nChecking TurboVisionTheme.axaml content..."
cat "src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.axaml"
Length of output: 4488
Line range hint 9-99: Verify theme resource references
The template uses several dynamic resources for theming. Let's verify that all referenced resources are defined in the theme.
✅ Verification successful
All theme resources are properly defined
All the dynamic resources used in the MenuItem template (ThemeAlternativeBackgroundBrush, ThemeBorderBrush, ThemeShadeBrush, and ThemeForegroundBrush) are correctly defined in the TurboVisionDefaultColors.axaml resource dictionary. The theme resources are implemented using ConsoleBrush with appropriate colors for the console environment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for theme resource definitions
echo "Searching for theme resource definitions..."
rg -l "ThemeAlternativeBackgroundBrush|ThemeBorderBrush|ThemeShadeBrush|ThemeChooserBackgroundBrush|ThemeActionBackgroundBrush|ThemeNoDisturbBrush" --type xaml
Length of output: 287
Script:
#!/bin/bash
# Search for theme resource definitions using regular ripgrep without file type filter
echo "Searching for theme resource definitions..."
rg -l "ThemeAlternativeBackgroundBrush|ThemeBorderBrush|ThemeShadeBrush|ThemeForegroundBrush"
# Also search for resource dictionary definitions that might contain these brushes
echo -e "\nSearching for resource dictionary files..."
fd "\.axaml$|\.xaml$"
Length of output: 8961
Script:
#!/bin/bash
# Let's check the content of TurboVisionDefaultColors.axaml which should contain the theme resource definitions
echo "Checking theme resource definitions in TurboVisionDefaultColors.axaml..."
cat "src/Consolonia.Themes.TurboVision/Templates/TurboVisionDefaultColors.axaml"
Length of output: 2027
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml (2)
8-11: LGTM! Resource dictionary and theme declaration are well-structured.
The migration from <Styles> to <ResourceDictionary> with <ControlTheme> follows Avalonia UI's best practices for theme definitions.
Line range hint 11-58: Consider improving accessibility.
While the template structure is well-organized, consider the following accessibility improvements:
- The BlackoutVisual's "X" text might not be descriptive enough for screen readers. Consider adding an AutomationProperties.Name.
- Setting Focusable to False might affect keyboard navigation. Verify if this is intentional and doesn't break accessibility guidelines.
src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml (4)
1-4: LGTM: Namespace declarations are well-structured
The ResourceDictionary declarations and namespace imports are properly organized and include all necessary references.
5-7: LGTM: Well-structured ControlTheme definition
The transition to ControlTheme and use of dynamic resources for theming shows good practices for maintainable styling.
19-25: LGTM: Content presentation structure is well-implemented
The VisualLayerManager and ContentPresenter implementation follows Avalonia's best practices with proper template bindings.
11-13: Verify transparency behavior across different console environments
The transparent ConsoleBrush mode might behave differently across various console environments and color schemes.
✅ Verification successful
Transparency implementation is correctly handled and safe to use
The transparency implementation in the popup's ConsoleBrush is properly supported by the framework:
- The ConsoleBrush class properly handles transparency through the PixelBackgroundMode.Transparent enum value
- The transparency mode is validated in ConsoloniaTextPresenter to ensure proper rendering
- The Pixel struct has proper blending logic for handling transparent backgrounds
- The ConsoleWindow implementation respects transparency levels
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other uses of ConsoleBrush with transparency to ensure consistent implementation
rg -l "ConsoleBrush.*Mode=\"Transparent\"" --type xaml
# Look for any related tests or documentation about transparency handling
rg -l "transparency|transparent" --type cs --type md
Length of output: 341
Script:
#!/bin/bash
# Let's check the content of these files to understand transparency handling
cat src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/ConsoloniaTextPresenter.cs
cat src/Consolonia.Core/Infrastructure/ConsoleWindow.cs
cat src/Consolonia.Core/Drawing/PixelBufferImplementation/Pixel.cs
# Also search for ConsoleBrush implementation to understand transparency mode
ast-grep --pattern 'class ConsoleBrush {
$$$
}'
Length of output: 27746
Script:
#!/bin/bash
# Let's find the ConsoleBrush implementation since it wasn't found in the previous search
rg -l "class ConsoleBrush"
# Also search for PixelBackgroundMode enum to understand transparency mode
rg -l "enum PixelBackgroundMode"
Length of output: 184
Script:
#!/bin/bash
# Let's examine the ConsoleBrush implementation and PixelBackgroundMode enum
cat src/Consolonia.Core/Drawing/ConsoleBrush.cs
cat src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackgroundMode.cs
Length of output: 9171
src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml (3)
1-3: LGTM: Namespace declarations are well-structured
The namespace declarations are properly organized, including the custom helpers namespace that's correctly scoped to the assembly.
Line range hint 14-17: LGTM: Scroll visibility settings are flexible
The ScrollViewer visibility settings use appropriate TemplateBinding and Auto visibility, providing good flexibility for content adaptation.
Line range hint 4-13: Verify the asymmetric padding design
The padding is set to 1,0,0,0 which creates an asymmetric layout. While this might be intentional for the TurboVision theme style, please verify this is the desired behavior.
✅ Verification successful
The asymmetric padding is part of a cohesive shadow design
Looking at the complete MenuFlyoutPresenter template, the asymmetric padding of 1,0,0,0 works in conjunction with the shadow effect created by:
- A background Border with
Margin="2,1,0,0"usingThemeShadeBrush - The main Border with
Margin="0,0,2,1"
This creates a classic TurboVision-style UI effect where menus appear to have a slight shadow offset to the bottom-right. The padding helps maintain proper content alignment within this design.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar padding patterns in other TurboVision controls
rg -l "Padding.*1,0,0,0" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
Length of output: 80
Script:
#!/bin/bash
# Let's search for any Padding patterns in TurboVision controls to understand the common patterns
rg "Padding.*Value=\"[^\"]+\"" "src/Consolonia.Themes.TurboVision/Templates/Controls/" -A 1
# Also search specifically for MenuFlyoutPresenter patterns in other themes for comparison
fd "MenuFlyoutPresenter.axaml" | xargs rg "Padding.*Value=\"[^\"]+\""
Length of output: 389
Script:
#!/bin/bash
# Let's check for any documentation or comments about TurboVision theme styling patterns
rg -i "turbovision.*padding|padding.*turbovision" -g "*.md" -g "*.axaml" -g "*.cs"
# Also let's check the specific MenuFlyoutPresenter file for any comments about the padding
cat "src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml"
Length of output: 2980
src/Consolonia.Themes.TurboVision/Themes/Material/MaterialTheme.cs (2)
7-7: Verify the Impact of Changing the Base Class
The MaterialTheme class now inherits from Styles instead of ResourceIncludeBase. Ensure that this change does not adversely affect how resources are included or applied throughout the application.
9-11: Constructor Update Aligns with Best Practices
The constructor now accepts an optional IServiceProvider and uses AvaloniaXamlLoader.Load(sp, this); to load XAML resources. This is a recommended approach and should enhance resource loading flexibility.
src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.cs (2)
9-9: Assess the Change in Inheritance Hierarchy
The FluentTheme class has changed its base class from ResourceIncludeBase to Styles. Please verify that all theme resources and styles are still correctly applied after this modification.
11-13: Updated Constructor Enhances Resource Loading
The new constructor with an optional IServiceProvider parameter and the call to AvaloniaXamlLoader.Load(sp, this); are appropriate for dynamic resource loading. This aligns with modern practices in Avalonia theme management.
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml (1)
1-5: ResourceDictionary and ControlTheme Structure Looks Good
The transition to using a ResourceDictionary and defining a ControlTheme for ScrollBar enhances the theming structure and aligns with the project's overall styling approach.
src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackTheme.cs (1)
7-8: Document theme-specific characteristics and verify theme registration.
Add documentation to explain the black theme variant's specific characteristics.
+ /// <summary>
+ /// Provides the black variant of the TurboVision theme, implementing specific color schemes
+ /// and styling appropriate for high-contrast displays.
+ /// </summary>
public class TurboVisionBlackTheme : StylesLet's verify the theme registration and usage across the application:
src/Tools/Consolonia.PreviewHost/App.axaml (1)
6-6: Verify the MaterialTheme.axaml resource exists
The StyleInclude source has been updated to point to MaterialTheme.axaml. Let's verify this resource exists and is properly structured.
✅ Verification successful
MaterialTheme.axaml resource exists and is properly structured
The file exists at the correct location src/Consolonia.Themes.TurboVision/Themes/Material/MaterialTheme.axaml and follows the proper theme structure with merged dictionaries including MaterialColors.axaml.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence and structure of MaterialTheme.axaml
fd MaterialTheme.axaml
# Check if the file contains proper theme structure
rg -A 5 "MaterialTheme" $(fd MaterialTheme.axaml)
Length of output: 533
src/Consolonia.Themes.TurboVision/Templates/Controls/Panel.axaml (1)
5-8: Clean implementation of Panel theme
Good implementation using ControlTheme with a clear null background setting. The migration to ResourceDictionary aligns with modern Avalonia resource management practices.
src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.axaml (2)
2-3: LGTM: Proper class declaration added
The addition of the x:Class attribute properly associates the XAML with its code-behind class.
7-7: Verify resource resolution with MergeResourceInclude
The change from ResourceInclude to MergeResourceInclude might affect how resources are resolved at runtime.
✅ Verification successful
Let me gather more information about the resource inclusion patterns in the codebase to better understand the context.
MergeResourceInclude usage is consistent with codebase patterns
The change from ResourceInclude to MergeResourceInclude is consistent with the codebase's resource management approach. The file TurboVisionDefaultColors.axaml exists at the correct location and contains valid resource definitions. The codebase shows a clear pattern where MergeResourceInclude is predominantly used for theme-specific resources and control templates, while ResourceInclude is used in specific themes like Material and Fluent. The referenced file follows the same structure as other color definition files in the project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if TurboVisionDefaultColors.axaml exists and is properly referenced
fd -e axaml -e xaml "TurboVisionDefaultColors" src/
# Check for any other references to ensure consistency
rg "TurboVisionDefaultColors" src/
Length of output: 320
Script:
#!/bin/bash
# Check the content of TurboVisionDefaultColors.axaml
cat src/Consolonia.Themes.TurboVision/Templates/TurboVisionDefaultColors.axaml
# Look for other MergeResourceInclude or ResourceInclude patterns
rg -A 2 "(?:Merge)?ResourceInclude" src/
Length of output: 10736
src/Consolonia.Themes.TurboVision/Templates/Controls/LightDismissOverlaylayer.axaml (2)
1-4: LGTM: Proper namespace declarations
The namespace declarations are well-organized and include all necessary references for the control theme implementation.
5-8: LGTM: Correct ControlTheme implementation
The ControlTheme is properly implemented with:
- Correct type targeting
- Appropriate null background for overlay functionality
src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/Controls/DialogWindow.axaml (2)
5-7: LGTM: Proper theme inheritance
The ControlTheme is correctly implemented with BasedOn, ensuring proper inheritance from the base DialogWindow theme.
8-10: LGTM: Correct dynamic resource usage
The Background property correctly uses DynamicResource for theme-aware background color.
src/Consolonia.Themes.TurboVision/Themes/Material/MaterialTheme.axaml (1)
12-12: Verify the completeness of AllControls.axaml
The style include has been changed from TurboVision.axaml to AllControls.axaml. Let's verify that all necessary control styles are present in the new location.
✅ Verification successful
Style consolidation is properly implemented
The verification confirms that the change from TurboVision.axaml to AllControls.axaml is correct and complete:
- AllControls.axaml exists and properly includes all individual control styles through MergeResourceInclude
- All referenced control files (TextBlock.axaml, Window.axaml, Button.axaml, etc.) are present in the Controls directory
- No remaining references to TurboVision.axaml exist in the codebase
- The style consolidation follows a clean architecture where AllControls.axaml acts as an aggregator for all individual control styles
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if AllControls.axaml exists and contains all necessary control styles
rg -l "StyleInclude.*AllControls.axaml"
rg -A 10 "<Style.*TargetType=" "src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml"
Length of output: 518
Script:
#!/bin/bash
# Let's first check if the AllControls.axaml file exists
fd "AllControls.axaml$" src/
# Let's also check the content of the old TurboVision.axaml to compare styles
fd "TurboVision.axaml$" src/ --exec cat {}
# Let's check if styles are distributed across multiple files in the Controls directory
fd . "src/Consolonia.Themes.TurboVision/Templates/Controls" --type f -e axaml --exec cat {}
Length of output: 65761
Script:
#!/bin/bash
# Let's check if all the referenced control styles in AllControls.axaml are actually present in the Controls directory
fd . "src/Consolonia.Themes.TurboVision/Templates/Controls" -e axaml --type f --exec basename {} .axaml \;
# Let's also check if there are any references to TurboVision.axaml in the codebase to ensure nothing is broken
rg -l "TurboVision\.axaml"
Length of output: 611
src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/Controls/Button.axaml (2)
3-3: LGTM! Improved namespace resolution
The explicit assembly specification in the helpers namespace improves runtime resolution reliability.
5-13: Well-structured theme implementation using ControlTheme
The transition to ControlTheme with proper BasedOn configuration is a significant improvement in theme architecture. This approach provides better organization and inheritance capabilities.
However, let's verify the base button style reference:
src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.axaml (1)
15-16: Verify intentional inclusion of TurboVisionBlack button style
The theme includes both AllControls.axaml and TurboVisionBlack's Button.axaml. Let's verify this is intentional and not causing style conflicts.
✅ Verification successful
Let me gather more context about the AllControls.axaml to understand potential style conflicts.
Let me check the specific Button.axaml from Templates to understand the style inheritance hierarchy.
The inclusion of TurboVisionBlack button style is intentional and safe
The TurboVisionBlack button style only overrides the shadow property (setting it to False) while inheriting all other properties from the base button style defined in AllControls. This is a valid style override pattern that won't cause conflicts, as it's making a minimal, specific modification to the button's appearance. Multiple themes (Fluent, Material, TurboVisionBlack) use this same approach of combining AllControls with the TurboVisionBlack button style.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other themes using TurboVisionBlack button style
rg -l "StyleInclude.*TurboVisionBlack/Controls/Button.axaml"
# Check for potential style conflicts in Button styling
rg -A 10 "TargetType=\"Button\"" "src/Consolonia.Themes.TurboVision/Themes/"
Length of output: 2340
Script:
#!/bin/bash
# Check the content of AllControls.axaml to see if it includes Button styles
rg -A 5 "Button" "src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml"
# Check if other themes also combine AllControls with specific button styles
rg -l "StyleInclude.*AllControls.axaml"
Length of output: 2533
Script:
#!/bin/bash
# Check the base Button style from Templates
cat "src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml"
# Check if TurboVisionBlack's Button style is explicitly overriding specific properties
rg -A 10 "Setter Property=" "src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/Controls/Button.axaml"
Length of output: 8081
src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/DataGridExtensions.cs (1)
6-6: Verify the necessity of making this class public.
The visibility change from internal to public exposes these APIs to external assemblies. Please confirm if this is intentional as part of the theme control refactoring.
✅ Verification successful
Making the class public aligns with the theme's public API pattern
The visibility change appears to be intentional and consistent with the codebase structure:
- The class implements an Avalonia attached property that's actively used in the DataGrid.axaml template
- Other helper classes in the same directory (ButtonExtensions, CalendarExtensions, etc.) are also public
- The class is part of the theme's control customization infrastructure, which needs to be accessible to consumers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if this class is used outside its assembly
rg -l "DataGridExtensions" | grep -v "TurboVision"
Length of output: 48
Script:
#!/bin/bash
# Let's first check if there are any references to DataGridExtensions in any file
rg "DataGridExtensions" -A 2 -B 2
# Also check the git history to see when and why this change was made
git log -p --all -- "**/DataGridExtensions.cs"
# Let's also check what other public APIs exist in the TurboVision theme
fd -e cs -p "TurboVision" --exec grep -l "public class\|public static class\|public interface"
Length of output: 3975
src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackTheme.axaml (2)
7-8: Verify the existence of referenced resource file.
Let's ensure the referenced color resource file exists.
✅ Verification successful
Referenced resource file exists at the correct location
The file TurboVisionBlackColors.axaml exists at the expected path relative to the theme file, confirming that the resource merge reference is valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the referenced file exists
fd "TurboVisionBlackColors.axaml$"
Length of output: 123
11-12: Verify the existence of referenced style files.
Let's ensure the referenced style files exist.
✅ Verification successful
Style files exist at the referenced locations
Both referenced style files are present in the codebase:
/Templates/Controls/AllControls.axamlexists atsrc/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml/Themes/TurboVisionBlack/Controls/Button.axamlexists atsrc/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/Controls/Button.axaml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the referenced files exist
fd "AllControls.axaml$"
fd "Button.axaml$" | grep "TurboVisionBlack"
Length of output: 221
src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkTheme.axaml (3)
1-15: LGTM! Theme structure follows Avalonia conventions.
The theme structure is well-organized and follows Avalonia's conventions for theme resources and styles. The use of MergeResourceInclude is appropriate for theme resources.
7-7: Verify the existence of referenced color resource file.
Let's ensure the referenced color resource file exists.
✅ Verification successful
Resource file exists at the expected location
The referenced color resource file TurboVisionDarkColors.axaml exists at the correct path relative to the theme file, confirming the validity of the resource inclusion.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the referenced file exists
fd "TurboVisionDarkColors.axaml$"
Length of output: 120
12-14: Verify the existence of referenced style files.
Let's ensure all referenced style files exist.
✅ Verification successful
All referenced style files exist in the codebase
The verification confirms that all three referenced style files exist at their expected locations:
/Templates/Controls/AllControls.axaml→src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml/Themes/TurboVisionDark/Controls/Button.axaml→src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/Controls/Button.axaml/Themes/TurboVisionDark/Controls/DialogWindow.axaml→src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/Controls/DialogWindow.axaml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the referenced files exist
fd "AllControls.axaml$"
fd "Button.axaml$" | grep "TurboVisionDark"
fd "DialogWindow.axaml$" | grep "TurboVisionDark"
Length of output: 354
src/Consolonia.Themes.TurboVision/Templates/Controls/ItemsControl.axaml (1)
5-17: LGTM! Clean ControlTheme implementation
The ControlTheme implementation follows Avalonia's best practices:
- Proper targeting of ItemsControl
- Clean template structure with appropriate bindings
- Correct usage of PART_ItemsPresenter for ItemsControl
src/Consolonia.Themes.TurboVision/Templates/Controls/UserControl.axaml (1)
9-18: LGTM! Well-structured ContentPresenter
The ContentPresenter implementation is thorough with all necessary template bindings for a UserControl.
src/Consolonia.Themes.TurboVision/Templates/Controls/OverlayPopupHost.axaml (1)
5-23: LGTM! Well-structured popup host implementation
The OverlayPopupHost implementation follows best practices:
- Proper null background setting
- Correct usage of transparency fallback
- Appropriate VisualLayerManager configuration for popups
- Well-structured ContentPresenter with all necessary bindings
src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml (1)
3-6: Verify the FontSizeNormal value across themes
The FontSizeNormal value of 1 seems unusually small. Please verify if this is intentional and consistent with other themes.
✅ Verification successful
FontSizeNormal value of 1 is consistent across all themes
The value of FontSizeNormal is set to 1 consistently across all theme files:
- Material theme (MaterialColors.axaml)
- TurboVision Black theme (TurboVisionBlackColors.axaml)
- TurboVision Default theme (TurboVisionDefaultColors.axaml)
- TurboVision Dark theme (TurboVisionDarkColors.axaml)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check FontSizeNormal values across all theme files
rg -A 1 'x:Key="FontSizeNormal"' --type xaml
Length of output: 75
Script:
#!/bin/bash
# Search for FontSizeNormal in all files with .axaml extension
rg -A 1 'x:Key="FontSizeNormal"' -g '*.axaml'
Length of output: 1139
src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackColors.axaml (1)
3-6: LGTM! Consistent with TurboVisionDarkColors.axaml
The implementation matches TurboVisionDarkColors.axaml, maintaining consistency across themes.
src/Consolonia.Themes.TurboVision/Templates/Controls/Calendar.axaml (2)
1-4: LGTM: Clean migration to ResourceDictionary and ControlTheme
The restructuring from Styles to ResourceDictionary with ControlTheme follows modern Avalonia UI practices and improves theme organization.
Line range hint 5-32: Verify CalendarExtensions.ZoomOutOnKey behavior
The template implementation looks good, but we should verify the behavior of the custom ZoomOutOnKey extension.
✅ Verification successful
ZoomOutOnKey extension is properly implemented and safe to use
The ZoomOutOnKey attached property is correctly implemented as a calendar navigation helper that:
- Subscribes/unsubscribes to KeyDown events based on the property value
- Handles Backspace and minus key presses to zoom out the calendar view (increment DisplayMode)
- Includes proper null checks and boundary conditions
- Follows Avalonia's attached property pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the implementation and usage of ZoomOutOnKey
ast-grep --pattern 'class CalendarExtensions {
$$$
ZoomOutOnKey
$$$
}'
Length of output: 78
Script:
#!/bin/bash
# Search for files containing CalendarExtensions
fd "CalendarExtensions" --type f
# Search for ZoomOutOnKey usage in the codebase
rg "ZoomOutOnKey" -A 3
Length of output: 1695
Script:
#!/bin/bash
# Get the full implementation of CalendarExtensions to understand ZoomOutOnKey behavior
cat "src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/CalendarExtensions.cs"
Length of output: 1259
src/Consolonia.Themes.TurboVision/Templates/Controls/DataValidationErrors.axaml (2)
1-4: LGTM: Proper namespace declarations and theme structure
The migration to ResourceDictionary and namespace declarations are well-organized.
5-30: Verify error template bindings
The error handling implementation looks correct, but we should verify the binding paths for validation errors.
✅ Verification successful
Let me gather more information about the error template implementation by checking for similar patterns in other control templates.
Let me try a different search pattern to find error template implementations.
Error template implementation is correctly configured
The validation error bindings and template implementation are properly set up:
- The
ErrorTemplateis correctly bound to display validation errors usingDataValidationErrors.Errors - Error visibility is properly controlled via
DataValidationErrors.HasErrors - The error styling uses
ThemeErrorBrushwhich is consistently defined across all themes - The template is being reused correctly in
CalendarDatePicker
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for DataValidationErrors usage to verify binding paths
rg -A 2 "DataValidationErrors\.Errors|DataValidationErrors\.HasErrors"
Length of output: 1265
Script:
#!/bin/bash
# Search for ErrorTemplate usage in control templates
rg -A 2 'ErrorTemplate.*DataTemplate'
# Search for ItemsControl with error-related bindings
rg -A 2 'ItemsControl.*ThemeErrorBrush'
Length of output: 81
Script:
#!/bin/bash
# Search for ErrorTemplate usage in any context
rg -A 3 'ErrorTemplate'
# Search for validation error related styles or resources
rg -A 3 'ThemeErrorBrush'
Length of output: 4211
src/Consolonia.Themes.TurboVision/Themes/Material/MaterialColors.axaml (1)
3-5:
Review FontSizeNormal value and unit
The FontSizeNormal value of 1 seems unusually small and lacks unit specification. This could affect text readability across the application.
src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml (1)
14-15: Verify DialogHost functionality across themes
Setting DialogHost.IsDialogHost to True by default might affect theme switching. Ensure this behavior is consistent across all themes and properly handled when themes are changed at runtime.
src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml (1)
Line range hint 24-25: Review ScrollViewer negative margins
The ScrollViewer uses negative margins (-1,-1). This might cause layout issues on different themes or scaling factors. Consider making these values theme-dependent or documenting the reason for the specific values.
✅ Verification successful
Let me search for more context about the ScrollViewer implementation to understand if these margins are part of a consistent design pattern.
Let me try a different approach to understand the ScrollViewer styling pattern.
Let me check one more thing to understand if these margins are related to the border styling and overall theme design.
Based on the gathered evidence, I can now provide the final response:
Negative margins are intentional for border alignment
The negative margins (-1,-1) in the ScrollViewer are part of the theme's design pattern, used consistently across multiple controls (ListBox, DataGrid, CalendarItem) to properly align borders with the theme's visual style. This is supported by:
- The ScrollViewer's BorderBrush being explicitly set to ThemeBackgroundBrush
- Similar negative margin patterns found in other controls for consistent border alignment
- The presence of BorderThickness bindings in the ListBox template
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar margin patterns across themes
rg -A 1 "Margin=\".*-1.*\"" src/Consolonia.Themes.*
Length of output: 2342
Script:
#!/bin/bash
# Check ScrollViewer implementations and styles
ast-grep --pattern 'ScrollViewer {
$$$
}'
# Also check for any documentation about the margins
rg -l "ScrollViewer" src/Consolonia.Themes.TurboVision/**/*.md
Length of output: 192
Script:
#!/bin/bash
# Check for ScrollViewer styles and templates
rg -A 5 "TargetType=\"ScrollViewer\"" src/Consolonia.Themes.TurboVision
# Look for any comments or documentation about ScrollViewer margins
rg -B 2 "ScrollViewer" src/Consolonia.Themes.TurboVision/**/*.txt
# Check if these margins are related to borders or specific styling
rg "BorderThickness" src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml
Length of output: 1187
Script:
#!/bin/bash
# Check the ScrollViewer template implementation
cat src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml
# Look for border-related styles in the theme
rg -A 2 "BorderBrush|BorderThickness" src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml
Length of output: 9178
src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml (1)
1-4: LGTM: Clean migration to ResourceDictionary structure
The migration from Styles to ResourceDictionary with ControlTheme aligns with modern XAML best practices and improves maintainability.
src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml (1)
3-44: Verify resource loading order dependencies
The consolidation of resources into a single ResourceDictionary is good for maintainability, but we should ensure there are no circular dependencies between the merged resources.
✅ Verification successful
Let me analyze the results from the script execution. The script checked for resource references across all XAML files in the repository, and I can see that:
- There are no circular dependencies detected between the resource files.
- The resource loading follows a hierarchical pattern where:
- Theme files (TurboVisionTheme.axaml, MaterialTheme.axaml, etc.) include AllControls.axaml
- AllControls.axaml includes individual control templates
- Individual control templates don't reference each other or AllControls.axaml back
Let me generate the final response:
Resource loading order is well-structured and safe
The consolidation of resources into a single ResourceDictionary in AllControls.axaml is safe as there are no circular dependencies between the merged resources. The resource loading hierarchy is properly structured with themes referencing AllControls.axaml, which in turn references individual control templates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential circular dependencies in XAML resources
for file in $(fd . -e axaml); do
echo "Checking $file for resource references..."
rg "Source=\"([^\"]+)\"" "$file" -o | cut -d'"' -f2 | while read -r source; do
# Check if the source file references back to the original file
referenced_file="${source#/Templates/Controls/}"
if [ -f "$referenced_file" ]; then
rg "Source=\".*${file##*/}\"" "$referenced_file"
fi
done
done
Length of output: 43903
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDatePicker.axaml (1)
Line range hint 11-84: LGTM: ControlTheme implementation follows best practices
The ControlTheme implementation for CalendarDatePicker is well-structured and follows modern XAML patterns:
- Proper use of templated parent bindings
- Clear separation of visual elements
- Appropriate use of resource references
src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml (1)
Line range hint 5-86: LGTM: Well-structured CheckBox implementation
The CheckBox implementation is clean and well-organized:
- Effective use of the new ^ selector syntax
- Clear state management (checked, unchecked, indeterminate)
- Proper separation of visual states
src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml (1)
Line range hint 8-105: LGTM: Robust DialogWindow implementation
The DialogWindow implementation is comprehensive and well-structured:
- Proper layering of visual elements
- Good separation of concerns between content and chrome
- Effective use of custom drawing capabilities
src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBox.axaml (1)
100-104: Consider implementing focus style for accessibility
The commented-out focus style could be important for keyboard navigation and accessibility. Consider implementing it or document why it's not needed.
Let's check if focus styles are implemented elsewhere:
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarItem.axaml (1)
131-138: LGTM: Clean implementation of disabled visual state
The disabled visual implementation using visibility toggles is clean and follows best practices for state management.
src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml (1)
1-3: LGTM: Resource Dictionary declarations are well-structured.
The namespace declarations are appropriate and follow Avalonia UI conventions.
src/Consolonia.Themes.TurboVision/Templates/Controls/SymbolsControl.axaml
Outdated
Show resolved
Hide resolved
src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml
Outdated
Show resolved
Hide resolved
src/Consolonia.Themes.TurboVision/Templates/Controls/TextBlock.axaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (14)
src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml (2)
4-7: Good transition to ControlThemeThe shift from Style to ControlTheme is a positive architectural change that aligns with Avalonia's modern theming system. The use of DynamicResource for theming is appropriate.
This transition to ControlTheme provides better theme inheritance and override capabilities. Consider applying similar transitions to other control templates in the TurboVision theme for consistency.
13-17: Remove or document commented codeThe commented-out Border section with "thth" tag appears to be temporary debugging code. If this code is important for future reference, please document its purpose; otherwise, consider removing it to maintain code cleanliness.
- <!--<Border Name="PART_TransparencyFallback" IsHitTestVisible="False" Tag="thth"> - <Border.Background> - <notImportant:ConsoleBrush Color="DarkGreen"/> - </Border.Background> -</Border>-->src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml (1)
Line range hint
3-31: LGTM! Well-structured ControlTheme with proper resource bindings.The transition to ControlTheme is well implemented with appropriate DynamicResource usage for theme consistency.
Consider adding XML documentation comments above the ControlTheme to describe its purpose and usage:
+<!-- ToggleButton theme for TurboVision style --> <ControlTheme x:Key="{x:Type ToggleButton}" TargetType="ToggleButton">src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml (2)
Line range hint
5-60: Consider adding AutomationProperties for accessibility.The ScrollViewer template is well-structured and follows Avalonia best practices. However, consider enhancing accessibility by adding AutomationProperties to the ScrollBars.
<ScrollBar Name="PART_HorizontalScrollBar" Orientation="Horizontal" Grid.Row="1" Grid.Column="0" + AutomationProperties.Name="Horizontal ScrollBar" Focusable="False" /> <ScrollBar Name="PART_VerticalScrollBar" Orientation="Vertical" Grid.Row="0" Grid.Column="1" + AutomationProperties.Name="Vertical ScrollBar" Focusable="False" />
66-120: Consider caching MultiBinding converter instance.The MenuScroller template uses multiple instances of the same converter. Consider declaring the converter as a resource to improve performance.
<ControlTheme x:Key="{x:Type ScrollViewer}" TargetType="ScrollViewer"> + <ControlTheme.Resources> + <converters:MenuScrollingVisibilityConverter x:Key="MenuScrollingVisibilityConverter"/> + </ControlTheme.Resources> <!-- ... --> - <MultiBinding Converter="{x:Static converters:MenuScrollingVisibilityConverter.Instance}" + <MultiBinding Converter="{StaticResource MenuScrollingVisibilityConverter}"src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml (2)
1-4: LGTM! Improved theme organization using ResourceDictionaryThe transition from
<Styles>to<ResourceDictionary>with<ControlTheme>follows modern Avalonia practices, providing better theme encapsulation and maintainability.
Line range hint
9-11: Remove or document the commented Background setterThe commented-out Background setter should either be removed if unused or documented if it serves a specific purpose for future reference.
- <!--<Setter Property="Background" - Value="{DynamicResource ThemeChooserBackgroundBrush}" />-->src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.cs (1)
1-13: Document theme migration guidelinesThe significant changes in theme initialization (removal of Uri-based resource loading, switch to Styles, and service-provider-based loading) warrant documentation for users migrating from the previous implementation.
Consider adding:
- Migration guidelines in the README or documentation
- Example of how to customize themes with the new approach
- Explanation of the architectural benefits of this change
src/Consolonia.Gallery/App.cs (1)
20-24: Clean up commented-out theme configurationsThe constructor contains multiple commented-out theme configurations which reduces code readability. If these themes are intended for future use or testing, consider:
- Moving them to a separate configuration file
- Using a theme switching mechanism
- Removing them if they're no longer needed
public App() { - /*Styles.Add(new TurboVisionTheme());*/ - /*Styles.Add(new TurboVisionBlackTheme());*/ - /*Styles.Add(new TurboVisionDarkTheme());*/ Styles.Add(new FluentTheme()); - /*Styles.Add(new MaterialTheme());*/ }src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml (1)
55-58: Clean up commented-out stylesThere are multiple commented-out style blocks without clear indication of their purpose or future use. Consider either:
- Removing them if they're no longer needed
- Adding comments explaining why they're preserved
- Creating a separate file for experimental styles
Also applies to: 75-78
src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml (1)
Line range hint
1-128: Well-structured separation of concernsThe theme properly separates styling from rendering logic by:
- Using DynamicResource for theme colors
- Delegating console-specific rendering to custom helpers
- Maintaining a clean separation between template structure and visual properties
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml (1)
Line range hint
46-52: Consider making the BlackoutVisual more configurable.The hardcoded " X" text in BlackoutVisual might cause issues with:
- Localization requirements
- Theme customization
- Accessibility support
Consider making this configurable through a property or resource:
<TextBlock Name="BlackoutVisual" - Text=" X" + Text="{DynamicResource CalendarDayButtonBlackoutText}" Margin="0" HorizontalAlignment="Stretch" VerticalAlignment="Stretch" RenderTransformOrigin="0.5,0.5" Foreground="{DynamicResource ThemeForegroundBrush}" />src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml (2)
116-123: Consider re-enabling styles forThumbinteraction statesThe styles for
Thumb#thumb:pointeroverandThumb#thumb:pressedare commented out. If visual feedback on hover and pressed states is desired for better user experience, consider uncommenting and verifying these styles.
131-143: Clean up commented-out styles related toRepeatButtonThe styles for
RepeatButton.repeatandRepeatButton.repeattrackare commented out. If these styles are obsolete or no longer needed, removing them can improve code readability and maintainability. If they are needed, consider re-enabling them and ensuring they function as intended.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (29)
src/Consolonia.Gallery/App.cs(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBoxItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ContentControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/DataValidationErrors.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/FlyoutPresenter.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ItemsControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/LightDismissOverlaylayer.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/OverlayPopupHost.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Panel.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ProgressBar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Separator.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/SymbolsControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TabControl.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TextBlock.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- src/Consolonia.Themes.TurboVision/Templates/Controls/TextBlock.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/OverlayPopupHost.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ItemsControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/DataValidationErrors.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ContentControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/LightDismissOverlaylayer.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/FlyoutPresenter.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/SymbolsControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBox.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Panel.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Separator.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ProgressBar.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBoxItem.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/TabControl.axaml
🔇 Additional comments (31)
src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml (2)
1-3: LGTM: Proper namespace declarations
The ResourceDictionary declarations and namespace imports are well-structured and appropriate for the theme implementation.
8-26: LGTM: Well-structured popup template
The template implementation follows proper Avalonia patterns with appropriate use of Panel, VisualLayerManager, and ContentPresenter. The transparent background handling with ConsoleBrush is suitable for console-based UI.
src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml (3)
1-2: LGTM! Namespace declarations are clean and minimal.
The unused namespace imports flagged in the previous review have been removed.
32-36: LGTM! Checked state styling is properly implemented.
The style selector uses the correct '^' syntax for parent targeting with appropriate background color.
40-47: LGTM! Pressed and disabled states are properly implemented.
The style selectors for pressed and disabled states use correct syntax and appropriate dynamic resources.
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml (3)
1-4: LGTM: ResourceDictionary declarations and imports are well-organized.
The namespace declarations are complete and properly structured.
61-64: LGTM: Disabled state styling follows modern selector syntax.
The disabled state is properly handled using the new ^ selector syntax, with appropriate visual feedback.
121-124: LGTM: MenuScroller button styling is consistent with theme.
The RepeatButton styling follows the theme guidelines and uses appropriate resource references.
src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml (2)
45-66: LGTM! Well-structured style selectors with proper state handling
The style selectors are well-organized and handle all necessary states (default, checked, indeterminate, disabled) using the modern caret (^) notation. The template part targeting is precise and follows best practices.
Line range hint 17-44: Verify the symbols used for radio button states
The implementation uses ASCII symbols for the radio button states:
( )for the border*for checked state■for indeterminate state
Please verify these symbols render correctly across different terminal environments.
✅ Verification successful
The symbols used in radio button states are consistent with TurboVision theme's design
The search results show that these ASCII/Unicode symbols are consistently used across the TurboVision theme's controls:
- The radio button uses
( )for border,*for checked state, and■for indeterminate state - The checkbox uses similar symbols:
[ ]for border,Vfor checked state, and■for indeterminate state - Other controls in the theme also use ASCII/Unicode symbols for their visual representation (e.g.,
░for scrollbars,↓for dropdowns)
This is a deliberate design choice to maintain the retro TurboVision look and feel, and these symbols are commonly supported across different terminal environments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other symbol definitions to ensure consistency
rg -l "Text=\"[*()\■]\"" --type xaml
Length of output: 127
Script:
#!/bin/bash
# Search for symbol definitions with individual patterns
rg "Text=\"\*\"" --type xaml
rg "Text=\"\([ ]*\)\"" --type xaml
rg "Text=\"■\"" --type xaml
Length of output: 178
Script:
#!/bin/bash
# Search for symbol definitions without file type restriction
rg "Text=\"\*\""
rg "Text=\"\([ ]*\)\""
rg "Text=\"■\""
# Also search for any SymbolsControl usage to see the context
rg "SymbolsControl" -A 2 -B 2
Length of output: 32246
src/Consolonia.Themes.TurboVision/Themes/Fluent/FluentTheme.cs (1)
7-8: Verify consistency of theme base class changes across the codebase
The change from ResourceIncludeBase to Styles represents a significant architectural shift in theme management. Let's ensure this pattern is consistently applied across other theme implementations.
✅ Verification successful
Let me gather more information about theme implementations to make a conclusive verification.
Base class change is consistent across theme implementations
The change from ResourceIncludeBase to Styles is consistently applied across all theme implementations in the codebase:
- TurboVisionTheme
- TurboVisionBlackTheme
- MaterialTheme
- TurboVisionDarkTheme
- FluentTheme
All theme classes now inherit from Styles and follow the same pattern with IServiceProvider in their constructors, indicating a well-executed architectural change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other theme implementations to ensure consistent base class usage
# Expected: All theme classes should inherit from Styles
# Search for theme class declarations
ast-grep --pattern 'class $THEME_NAME : $BASE implements $$$'
# Search for any remaining ResourceIncludeBase usage
rg "ResourceIncludeBase" -A 2
Length of output: 939
Script:
#!/bin/bash
# Search for all theme class declarations and their base classes
ast-grep --pattern 'class $THEME_NAME : $BASE {
$$$
}'
# Search for files with "Theme" in their name to ensure we don't miss any
fd -e cs Theme
# Check if Styles is the new base class for themes
rg "class.*:.*Styles" -A 2
Length of output: 2095
src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml (3)
1-3: LGTM! The namespace declarations and root element structure are well-defined.
The transition from <Styles> to <ResourceDictionary> follows modern Avalonia practices for better resource organization.
33-40: LGTM! The state-based styles are well-defined.
The use of modern ^ selector syntax and appropriate theme brushes for different states is correct and follows best practices.
Line range hint 4-32: Consider the implications of commenting out the Background setter.
The commented-out Background setter might affect theme consistency. If this was intentional, consider adding a comment explaining why it's disabled.
Consider adding accessibility improvements.
The template could benefit from accessibility improvements:
<ContentPresenter Name="PART_ContentPresenter"
ContentTemplate="{TemplateBinding HeaderTemplate}"
Content="{TemplateBinding Header}"
+ AutomationProperties.Name="{TemplateBinding Header}"
VerticalContentAlignment="{TemplateBinding VerticalContentAlignment}" />✅ Verification successful
The commented-out Background setter is intentionally disabled and consistent with the theme
The search results show that ThemeAlternativeBackgroundBrush is a well-defined resource used across multiple controls in the theme, including FlyoutPresenter, Popup, ComboBox, ScrollViewer, MenuItem, and others. However, TabItem is specifically choosing not to use this background brush, which appears to be an intentional design decision to maintain visual consistency with the TurboVision theme's appearance.
The accessibility suggestion about adding AutomationProperties.Name remains valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other controls in the theme define ThemeAlternativeBackgroundBrush
rg "ThemeAlternativeBackgroundBrush" --type xaml
Length of output: 77
Script:
#!/bin/bash
# Search for ThemeAlternativeBackgroundBrush in all files
rg "ThemeAlternativeBackgroundBrush"
# Also search for other background brush usages in the theme files
fd "\.axaml$" | xargs rg "Background.*ThemeBackground"
Length of output: 3063
src/Consolonia.Gallery/App.cs (1)
6-6: Remove unused theme imports
The following imports are unused as their corresponding theme instantiations are commented out in the code:
Consolonia.Themes.TurboVision.Themes.Fluent
src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml (1)
1-4: LGTM! Good modernization of the control theme structure
The migration to ResourceDictionary and ControlTheme follows modern Avalonia practices and improves maintainability.
src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml (4)
1-7: LGTM: Resource declarations are well-structured
The namespace declarations and converter resources are properly organized within the ResourceDictionary.
121-126: Verify consistent usage of the new selector syntax
The change from TextBox:disabled to ^:disabled appears to be part of a broader pattern. Ensure this selector update is consistently applied across all control themes.
✅ Verification successful
Selector syntax update is consistently applied
The codebase shows consistent usage of the new ^:disabled selector syntax across all control templates. No instances of the old TextBox:disabled selector were found, and the new syntax appears in all relevant control templates including Button, CheckBox, ComboBox, ListBox, Menu, RadioButton, TextBox, and others.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining old-style selectors
rg "TextBox:disabled" src/Consolonia.Themes.TurboVision/
# Search for the new style to verify consistency
rg "\^:disabled" src/Consolonia.Themes.TurboVision/
Length of output: 1739
Line range hint 9-20: Verify the impact of zero padding on text visibility
The Padding is set to "0,0" which might cause text to be too close to the control edges. Consider adding minimal padding (e.g., "1,0") to ensure comfortable text visibility.
Line range hint 70-70: Document or fix the caret width padding issue
The comment "I don't understand why 1 is not sufficient as a padding for caret of width 1" suggests an unresolved rendering issue. This should either be:
- Properly documented with the technical reason for the 2px padding
- Fixed if it's a rendering bug
✅ Verification successful
The caret padding is necessary for proper console cursor positioning
The 2px padding issue is not a bug but a technical requirement based on the codebase evidence:
- The caret implementation uses
MoveConsoleCaretToPositionBrushand requires proper positioning in the console environment - The
ConsoloniaTextPresenterclass handles special console-specific rendering with custom caret positioning logic - The extra padding ensures there's enough space for the console cursor to be properly positioned without overlapping with text content
The comment can be updated to explain this technical requirement rather than questioning it.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any related caret implementation details
ast-grep --pattern 'class ConsoloniaTextPresenter {
$$$
$caret$_$_ {
$$$
}
$$$
}'
Length of output: 95
Script:
#!/bin/bash
# Let's try a broader search for caret-related code and comments
rg -i "caret" --type-add 'axaml:*.axaml' --type axaml -A 5 -B 5
# Also search for the ConsoloniaTextPresenter class
fd "ConsoloniaTextPresenter" --type f --exec cat {}
Length of output: 49310
src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml (4)
1-3: LGTM: Namespace declarations are well-structured
The ResourceDictionary declarations and namespace imports are properly organized, including the custom helpers namespace for CaretControl.
35-38: LGTM: Focus state styling is consistent
The focus state styling follows the new selector pattern and uses appropriate theming resources.
28-33: Clarify the background brush selection
There's a commented out setter that would use ThemeSelectionBackgroundBrush, but the active code uses ThemeChooserBackgroundBrush. This suggests a deliberate change in the selection highlighting approach.
Let's verify the consistency of this change:
Consider removing the commented code if the change to ThemeChooserBackgroundBrush is final:
<Style Selector="^:selected">
- <!--<Setter Property="Background"
- Value="{DynamicResource ThemeSelectionBackgroundBrush}" />-->
<Setter Property="Background"
Value="{DynamicResource ThemeChooserBackgroundBrush}" />
</Style>✅ Verification successful
Remove commented code and keep ThemeChooserBackgroundBrush
The search results show that ThemeChooserBackgroundBrush is consistently used across multiple controls for selection and highlighting states, while ThemeSelectionBackgroundBrush appears to be a legacy resource that's either commented out or only defined in theme files. The switch to ThemeChooserBackgroundBrush appears to be an intentional standardization.
- Remove the commented code as it references the older brush that's being phased out
- Keep using
ThemeChooserBackgroundBrushas it aligns with the current design system
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if ThemeSelectionBackgroundBrush is still used elsewhere
rg "ThemeSelectionBackgroundBrush" "src/Consolonia.Themes.TurboVision/"
# Check the usage pattern of ThemeChooserBackgroundBrush
rg "ThemeChooserBackgroundBrush" "src/Consolonia.Themes.TurboVision/"
Length of output: 5767
Line range hint 4-27: Verify the hit testing implementation
While using ThemeTransparentBrush for hit testing visibility is functional, this approach might affect accessibility or touch targets. Consider verifying if this is the most robust solution.
Let's check if this pattern is used consistently across other controls:
✅ Verification successful
Let me gather more information about hit testing implementations in the codebase:
Let me check how ThemeTransparentBrush is defined:
Based on the gathered information, I can now provide the final response:
The hit testing approach using ThemeTransparentBrush is appropriate and safe
The current implementation using ThemeTransparentBrush for hit testing in ListBoxItem is a valid approach because:
ThemeTransparentBrushis properly defined as a transparentConsoleBrushin TurboVisionDefaultColors.axaml- This pattern is consistent with the codebase's hit testing strategy, where transparent backgrounds are used for hit testing while visual elements have explicit hit testing disabled when needed
- The implementation doesn't affect accessibility as it's specifically designed for console-based UI rendering
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar hit testing implementations in other control templates
rg -l "ThemeTransparentBrush.*hit test" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
# Check if there are any alternative hit testing approaches
ast-grep --pattern 'Background="{DynamicResource Theme.*Brush}"'
Length of output: 233
Script:
#!/bin/bash
# Search for all Background property setters in control templates
rg -A 2 'Background=".*"|Background="\{.*\}"' "src/Consolonia.Themes.TurboVision/Templates/Controls/"
# Look for any comments about hit testing in the codebase
rg "hit test" "src/Consolonia.Themes.TurboVision/"
# Check if ThemeTransparentBrush is defined and how it's used
rg "ThemeTransparentBrush" "src/Consolonia.Themes.TurboVision/"
Length of output: 27357
Script:
#!/bin/bash
# Check the definition of ThemeTransparentBrush
cat src/Consolonia.Themes.TurboVision/Templates/TurboVisionDefaultColors.axaml
# Check where else hit testing is implemented
rg -B 2 -A 2 "IsHitTestVisible" "src/Consolonia.Themes.TurboVision/"
Length of output: 8078
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml (2)
8-10: LGTM! Resource Dictionary and Control Theme declaration are well-structured.
The declaration follows Avalonia UI best practices for theme organization.
Line range hint 11-20: Verify the minimal dimensions for usability.
The MinWidth of 2 and MinHeight of 1 seem unusually small for a calendar button. This might affect:
- Content visibility
- Touch interaction
- Overall usability
Consider increasing these values to ensure proper sizing across different platforms and input methods.
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml (5)
Line range hint 4-46: Vertical ScrollBar ControlTemplate is well-structured
The ControlTemplate for the vertical ScrollBar is correctly defined with appropriate TemplateBindings and use of helpers:SymbolsControl. The layout and bindings ensure proper functionality and appearance.
48-81: Horizontal ScrollBar ControlTemplate is appropriately implemented
The horizontal ScrollBar template mirrors the vertical one with adjustments for horizontal orientation. The use of Grid with ColumnDefinitions and correct placement of controls ensures consistent behavior.
82-106: Verify the styling and dimensions of the Thumb control
The styles for the Thumb control are defined with specific properties for Foreground, Background, Tag, and Width. Ensure that using the Tag property to set visual characters (│ for vertical) aligns with the intended design and does not conflict with other control properties.
107-115: Confirm the Thumb settings for horizontal orientation
In the horizontal Thumb style, properties such as Tag set to ─, Height to 1, and Width to NaN are used to adjust the appearance. Verify that these settings render the Thumb correctly and provide the expected user interaction.
123-126: Ensure consistent dimension settings for orientations
The styles set Width to 1 for vertical orientation and Height to 1 for horizontal orientation. Confirm that these settings produce the correct dimensions for the ScrollBar in both orientations without causing layout issues.
Also applies to: 127-130
src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml
Outdated
Show resolved
Hide resolved
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml
Show resolved
Hide resolved
Co-authored-by: <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (19)
src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml (2)
118-121: Remove redundant commented-out styleThis entire style block is commented out and appears to be superseded by the margin settings below. Remove it if it's no longer needed.
127-135: Document the press animation effectThe margin adjustments between normal (0,0,1,1) and clickdelayed (1,0,1,1) states create a press animation effect. Consider adding a comment explaining this visual behavior for better maintainability.
<Style Selector="^ /template/ Border#InternalBorder"> + <!-- Normal state: Offset by 1px right and bottom to create raised effect --> <Setter Property="Margin" Value="0,0,1,1" /> </Style> <Style Selector="^:clickdelayed /template/ Border#InternalBorder"> + <!-- Pressed state: Offset right by 1px to create pressed effect --> <Setter Property="Margin" Value="1,0,1,1" /> </Style>src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml (1)
Remove commented Background setter as it's not needed
The commented Background setter can be safely removed because:
- The Background property is already properly bound in the control template via
Background="{TemplateBinding Background}"- Other controls that need ThemeAlternativeBackgroundBrush (like Menu, MenuItem, FlyoutPresenter) set it explicitly
- TabItem's selected state uses ThemeSelectionBackgroundBrush instead, suggesting a deliberate design choice
🔗 Analysis chain
Line range hint
8-10: Verify the intention behind commented Background setterThe Background setter is commented out but retained in the code. Please verify if this is intentional or if it should be removed entirely.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if Background is set elsewhere for TabItem rg -l "ThemeAlternativeBackgroundBrush.*TabItem"Length of output: 50
Script:
#!/bin/bash # Let's try a broader search for Background property in TabItem files rg "Background" "src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml" -C 3 # Also search for any other TabItem style definitions fd TabItem.axaml # Check if ThemeAlternativeBackgroundBrush is used elsewhere rg "ThemeAlternativeBackgroundBrush"Length of output: 4112
src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBoxItem.axaml (1)
4-5: Good architectural improvement using ControlThemeThe transition from
StyletoControlThemeis a positive change that brings:
- Better type safety through strong typing
- Improved maintainability
- Alignment with modern Avalonia best practices
src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml (2)
Line range hint
8-9: Document the hit testing implementationConsider adding more detailed documentation about why ThemeTransparentBrush is required for hit testing and any potential implications for custom themes.
- <!--ThemeTransparentBrush is used to make hit test visible--> + <!-- ThemeTransparentBrush is required for hit testing to work correctly. + Without this, the control might not receive mouse events in certain scenarios. + Custom themes should ensure they provide this resource. -->
CaretControl needs accessibility improvements
The CaretControl implementation lacks proper accessibility support. The control uses a visual caret indicator (TextBlock and Line) but doesn't expose this information to screen readers. Specific issues:
- No AutomationProperties are set to announce the caret position or focus state
- Missing accessibility labels for the caret visual indicators
- No ARIA attributes or automation peers implementation
To improve accessibility:
- Add AutomationProperties.Name and AutomationProperties.HelpText to expose caret state
- Implement proper automation peer for screen reader support
- Consider adding aria-* attributes for better assistive technology integration
🔗 Analysis chain
Line range hint
4-28: Verify accessibility support with CaretControlThe ControlTheme structure is well-organized, but we should ensure the CaretControl implementation supports accessibility features like screen readers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for accessibility-related implementations in CaretControl ast-grep --pattern 'class CaretControl { $$$ AutomationProperties.$_ { $$$ } $$$ }' # Check for any existing accessibility tests rg -l "accessibility|AutomationProperties" "src/Consolonia.Themes.TurboVision/Tests"Length of output: 262
Script:
#!/bin/bash # Let's first locate the CaretControl implementation fd CaretControl # Then check its content for accessibility features rg -A 5 "class CaretControl" # Also search for any test files related to CaretControl fd -e cs -e axaml Test.*CaretControlLength of output: 911
Script:
#!/bin/bash # Let's check the full implementation of CaretControl cat "src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/CaretControl.cs" # And its XAML template cat "src/Consolonia.Themes.TurboVision/Templates/Controls/CaretControl.axaml" # Also search for any usage of AutomationProperties in the theme rg "AutomationProperties" src/Consolonia.Themes.TurboVision/Length of output: 3816
src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml (2)
109-114: Remove commented-out style settersThere are commented-out foreground setters in both :selected and :focus styles. If these are no longer needed, they should be removed to maintain clean code.
Apply this diff to clean up the commented code:
<Style Selector="^:selected /template/ Border#root"> <Setter Property="Background" Value="{DynamicResource ThemeChooserBackgroundBrush}" /> - <!--<Setter Property="(TemplatedControl.Foreground)" - Value="{DynamicResource ThemeBackgroundBrush}" />--> </Style> <Style Selector="^:focus /template/ Border#root"> <Setter Property="Background" Value="{DynamicResource ThemeActionBackgroundBrush}" /> - <!--<Setter Property="(TemplatedControl.Foreground)" - Value="{DynamicResource ThemeBackgroundBrush}" />--> </Style>Also applies to: 116-121
Line range hint
1-133: Overall theme structure looks goodThe transition from Styles to ResourceDictionary + ControlTheme pattern is well-implemented. The control template is comprehensive and properly structured with appropriate resource references and state handling.
Consider documenting the theme's visual states and customization points in a separate markdown file to help other developers who might need to extend or modify the theme.
src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml (2)
Line range hint
25-40: Document the shadow implementationThe shaded background implementation using
ConsoleBrush Mode=Shadedcreates a shadow effect. Consider adding a comment explaining this pattern for future maintainability.- <!--Panel shade Mode=Shaded--> + <!-- Shadow effect implementation using shaded background. + This creates a classic console-style dialog shadow by offsetting + a shaded border behind the main dialog panel. -->
Line range hint
82-94: Enhance close button accessibilityThe close button uses a unicode × character without proper accessibility support.
Consider these improvements:
<Button Content="×" Background="{x:Null}" IsTabStop="False" Padding="0" + ToolTip.Tip="Close" + AutomationProperties.Name="Close Dialog" Command="{Binding Path=CloseClick, RelativeSource={RelativeSource TemplatedParent}}">src/Consolonia.Themes.TurboVision/Templates/Controls/Menu.axaml (2)
64-64: TODO comment needs clarificationThe TODO comment "Copypaste from MenuItem" is vague. Please clarify the intended action - is this indicating code that needs to be deduplicated, or is it marking a section that still needs to be implemented?
68-69: Remove commented-out codeThese commented-out Foreground setters should either be implemented or removed. Keeping commented code creates maintenance overhead and confusion.
- <!--<Setter Property="(TemplatedControl.Foreground)" - Value="{DynamicResource ThemeBackgroundBrush}" />-->Also applies to: 75-76
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml (3)
Line range hint
5-61: Consider documenting ScrollBarsWidth behaviorThe
ScrollBarsWidthattached property's "Auto" value behavior should be documented for maintainability.Consider adding a comment explaining the behavior:
<Setter Property="(helpers:ScrollViewerExtensions.ScrollBarsWidth)" + <!-- Auto adjusts scrollbar width based on content --> Value="Auto" />Consider simplifying CaretControl visibility logic
The CaretControl visibility could be combined with the IsFocused check.
- <helpers:CaretControl IsCaretShown="{TemplateBinding IsFocused}" - IsVisible="{TemplateBinding Focusable}"> + <helpers:CaretControl IsVisible="{TemplateBinding IsFocused}">
67-123: Consider refactoring repeated MultiBinding logicThe MultiBinding configuration is duplicated for both up and down buttons. This could be extracted to a resource for better maintainability.
Consider creating a reusable resource:
+ <MultiBinding x:Key="ScrollButtonVisibility"> + <Binding RelativeSource="{RelativeSource TemplatedParent}" Path="VerticalScrollBarVisibility" /> + <Binding RelativeSource="{RelativeSource TemplatedParent}" Path="Offset.Y" /> + <Binding RelativeSource="{RelativeSource TemplatedParent}" Path="Extent.Height" /> + <Binding RelativeSource="{RelativeSource TemplatedParent}" Path="Viewport.Height" /> + </MultiBinding>Document magic numbers in converter parameters
The converter parameters "0" and "100" should be documented or extracted as constants.
Consider adding comments explaining these values:
Converter="{x:Static converters:MenuScrollingVisibilityConverter.Instance}" + <!-- 0 represents top scroll threshold --> ConverterParameter="0">
Theme structure is consistent except for UserControl
The ScrollViewer's new ResourceDictionary and ControlTheme structure aligns with the pattern used across most controls in the TurboVision theme. However, UserControl.axaml still uses the old Style-based structure and needs to be updated for consistency.
src/Consolonia.Themes.TurboVision/Templates/Controls/UserControl.axamlneeds to be migrated to use ResourceDictionary and ControlTheme pattern🔗 Analysis chain
Line range hint
1-129: Verify theme structure consistency across controlsLet's ensure this new ResourceDictionary structure is consistently applied across other controls in the TurboVision theme.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistent theme structure across controls echo "Checking theme structure consistency..." rg -l "ResourceDictionary|ControlTheme" "src/Consolonia.Themes.TurboVision/Templates/Controls/" # Verify all controls follow the new pattern echo "Checking for old style patterns..." rg "<Style\s+TargetType=" "src/Consolonia.Themes.TurboVision/Templates/Controls/"Length of output: 3119
Script:
#!/bin/bash # Let's check the structure of a few representative control themes echo "Checking structure of Button control theme..." rg -A 5 "ControlTheme.*Button" "src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml" echo "\nChecking structure of TextBox control theme..." rg -A 5 "ControlTheme.*TextBox" "src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml" echo "\nChecking structure of ListBox control theme..." rg -A 5 "ControlTheme.*ListBox" "src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml" # Check if any files still use the old Style pattern as root element echo "\nChecking for files with Style as root element..." rg -l "^<Style\s+" "src/Consolonia.Themes.TurboVision/Templates/Controls/"Length of output: 1739
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml (1)
49-82: Consider adding accessibility attributesWhile the template is well-structured, consider enhancing accessibility by:
- Adding ARIA attributes for screen readers
- Ensuring keyboard navigation works correctly
- Providing alternative text for the symbol-based controls
src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml (1)
4-5: Well-structured theme definition using modern Avalonia patternsThe transition from Style to ControlTheme is a good architectural choice as it:
- Provides better type safety through explicit targeting
- Improves maintainability and organization of theme resources
- Aligns with Avalonia's recommended theming patterns
src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml (1)
56-59: Clean up commented stylesConsider removing commented-out styles if they're no longer needed, or restore them if they serve a purpose. This will improve code maintainability.
Also applies to: 76-79
src/Consolonia.Themes.TurboVision/Templates/Controls/DataGrid.axaml (1)
Line range hint
234-277: Clean up commented DataGridRowGroupHeader stylesLarge blocks of commented code for DataGridRowGroupHeader styling should be either:
- Removed if the functionality is no longer needed
- Implemented if the functionality is required
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (42)
src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Calendar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDatePicker.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CaretControl.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBoxItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ContentControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/DataGrid.axaml(8 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/DataValidationErrors.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/FlyoutPresenter.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/DataGridExtensions.cs(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ItemsControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/LightDismissOverlaylayer.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Menu.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/OverlayPopupHost.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Panel.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ProgressBar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Separator.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/SymbolsControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TabControl.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TextBlock.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml(2 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/UserControl.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.axaml(1 hunks)src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackTheme.axaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (28)
- src/Consolonia.Themes.TurboVision/Templates/TurboVisionTheme.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/FlyoutPresenter.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/OverlayPopupHost.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/UserControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ContentControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/DataValidationErrors.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/SymbolsControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ItemsControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/CaretControl.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Panel.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarButton.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/LightDismissOverlaylayer.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Separator.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Window.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Popup.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Helpers/DataGridExtensions.cs
- src/Consolonia.Themes.TurboVision/Themes/TurboVisionBlack/TurboVisionBlackTheme.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBox.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/Calendar.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ListBox.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ToggleButton.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarItem.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDatePicker.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/ProgressBar.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/TextBlock.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/TextBox.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/RadioButton.axaml
- src/Consolonia.Themes.TurboVision/Templates/Controls/TabControl.axaml
🔇 Additional comments (39)
src/Consolonia.Themes.TurboVision/Templates/Controls/Button.axaml (3)
1-5: LGTM: Resource Dictionary structure is well-organized
The transition from <Style> to <ResourceDictionary> with <ControlTheme> follows modern XAML best practices for theme organization.
Line range hint 8-10: Clean up commented-out styles
Remove the commented-out Foreground setter if it's no longer needed. Keeping commented code creates maintenance overhead and confusion.
110-117: Clean up and document the clickdelayed state
Remove the commented-out style setters if they're no longer needed. Consider adding a comment explaining the padding adjustment's visual effect.
src/Consolonia.Themes.TurboVision/Templates/Controls/TabItem.axaml (3)
1-5: LGTM: Clean transition to ResourceDictionary and ControlTheme
The restructuring from Styles to ResourceDictionary with ControlTheme follows modern Avalonia theming practices and improves maintainability.
34-41: LGTM: Clean state-based styling implementation
The disabled and selected state styles are well-defined with appropriate theme resources and modern selector syntax.
42-67:
Remove duplicate TabStripPlacement=Right style
The duplicate style issue from the previous review still exists. There are two separate style blocks for TabStripPlacement=Right:
- Lines 42-45 (setting HorizontalContentAlignment)
- Lines 64-67 (setting Margin)
Apply this diff to combine the styles:
- <Style Selector="^[TabStripPlacement=Right]">
- <Setter Property="HorizontalContentAlignment"
- Value="Right" />
- </Style>
<!--TabStripPlacement-->
<Style Selector="^[TabStripPlacement=Top]">
<Setter Property="Margin"
Value="0,0,-1,-1" />
</Style>
<Style Selector="^[TabStripPlacement=Bottom]">
<Setter Property="Margin"
Value="0 -1 -1 0" />
</Style>
<Style Selector="^[TabStripPlacement=Left]">
<Setter Property="Margin"
Value="0 0 -1 -1" />
</Style>
<Style Selector="^[TabStripPlacement=Right]">
+ <Setter Property="HorizontalContentAlignment"
+ Value="Right" />
<Setter Property="Margin"
Value="-1 0 0 -1" />
</Style>Also, consider adding documentation for the margin values:
+<!-- TabStripPlacement styles use negative margins to:
+ - Overlap borders between adjacent tabs
+ - Create seamless connection with the tab content area
+-->
<Style Selector="^[TabStripPlacement=Top]">src/Consolonia.Themes.TurboVision/Templates/Controls/ComboBoxItem.axaml (4)
1-3: LGTM: Proper namespace declarations
The namespace declarations are well-structured and include all necessary references.
36-49: LGTM: Well-structured state handling
The state-specific styles are well-organized with:
- Clear separation of states (selected, focus, selected+focus)
- Proper use of the new
^selector syntax - Consistent use of DynamicResource for theming
Line range hint 1-51: Verify theme consistency across controls
Let's ensure this ComboBoxItem styling is consistent with other controls in the TurboVision theme.
✅ Verification successful
Theme styling is consistent across controls
The verification shows that the ComboBoxItem's styling is consistent with other controls in the TurboVision theme:
- The usage of
ThemeChooserBackgroundBrushandThemeActionBackgroundBrushfollows the same pattern across similar interactive controls (ListBoxItem, MenuItem, etc.) - The control is properly migrated to
ControlTheme, aligning with the theme's overall architecture
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistency in theme implementation across controls
echo "Checking resource usage consistency..."
rg "ThemeChooserBackgroundBrush|ThemeActionBackgroundBrush" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
echo "Checking ControlTheme migration consistency..."
rg "<ControlTheme" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
Length of output: 10017
30-34: Verify intentional removal of pointer-over feedback
The commented-out pointer-over style would provide visual feedback when hovering over items. Consider if this removal aligns with the TurboVision theme's interaction design principles.
✅ Verification successful
Pointer-over styles are consistently commented out across controls
The removal of the pointer-over style appears to be intentional, as multiple controls in the TurboVision theme (ToggleButton, ScrollBar, DataGrid) have their pointer-over styles commented out. This seems to be part of a broader design decision in the theme, likely to maintain consistency with the classic TurboVision UI paradigm which didn't feature hover effects.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other controls in the TurboVision theme implement pointer-over styles
rg -l "pointerover" "src/Consolonia.Themes.TurboVision/Templates/Controls/"
Length of output: 577
Script:
#!/bin/bash
# Check the actual implementation of pointer-over styles in other controls
rg "pointerover" "src/Consolonia.Themes.TurboVision/Templates/Controls/" -A 2
Length of output: 3714
src/Consolonia.Themes.TurboVision/Templates/Controls/CalendarDayButton.axaml (5)
8-11: LGTM: Clean transition to ResourceDictionary pattern
The migration from Styles to ResourceDictionary with ControlTheme follows modern XAML best practices and improves maintainability.
Line range hint 23-57: LGTM: Well-structured control template
The template follows a clean pattern with proper separation of visual states using named rectangles and appropriate template bindings.
89-91: Remove commented-out styles
These commented-out styles appear to be obsolete and should be removed as previously suggested.
Also applies to: 115-118
59-87: LGTM: Clean selector syntax update
The transition to using the caret (^) syntax for selectors improves readability while maintaining the same functionality. The visual state management through IsVisible property is well-organized.
Also applies to: 92-114, 120-127
Line range hint 12-22: Verify the minimal dimensions
The MinWidth of 2 and MinHeight of 1 seem unusually small for a calendar day button. Please verify if these values are intentional for the TurboVision theme.
✅ Verification successful
Small dimensions are consistent with TurboVision theme's console-based design
The minimal dimensions (MinWidth=2, MinHeight=1) are intentional and align with other controls in the TurboVision theme:
- ScrollBar, ComboBox, and ProgressBar all use MinHeight=1
- CalendarButton uses MinWidth=4
- ProgressBar uses MinWidth=1 for indicators
- These small dimensions are appropriate for a console-based UI theme
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar minimal dimensions in other TurboVision controls
rg "MinWidth|MinHeight" --type xaml src/Consolonia.Themes.TurboVision/Templates/Controls/
Length of output: 120
Script:
#!/bin/bash
# Search for similar minimal dimensions in other TurboVision controls without file type restriction
rg "MinWidth|MinHeight" src/Consolonia.Themes.TurboVision/Templates/Controls/
Length of output: 1525
Script:
#!/bin/bash
# Get more context around the MinWidth/MinHeight values in these controls
rg "MinWidth|MinHeight" -B 1 -A 1 src/Consolonia.Themes.TurboVision/Templates/Controls/
Length of output: 3898
src/Consolonia.Themes.TurboVision/Templates/Controls/ListBoxItem.axaml (3)
1-3: LGTM: Namespace declarations are well-structured
The ResourceDictionary declarations and namespace imports are properly configured, including the custom helpers namespace for CaretControl.
36-39: LGTM: Focus state styling is well-implemented
The focus state style follows the new selector syntax pattern and uses appropriate theme resources.
29-34: Clarify the background brush change decision
There's a commented out section showing a switch from ThemeSelectionBackgroundBrush to ThemeChooserBackgroundBrush. Please clarify:
- Why was this change made?
- Should we remove the commented code if it's no longer needed?
✅ Verification successful
Remove commented code and keep ThemeChooserBackgroundBrush
Based on the codebase analysis, this is a consistent design pattern where ThemeChooserBackgroundBrush is used for selection states across multiple controls (ComboBoxItem, MenuItem, Menu, Button, etc.). The commented-out ThemeSelectionBackgroundBrush appears to be an older approach that has been systematically replaced. Both brushes are defined in all theme color files, but ThemeChooserBackgroundBrush is the preferred choice for selection states.
- Remove the commented code as it represents an obsolete implementation
- Keep using
ThemeChooserBackgroundBrushto maintain consistency with other controls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other usages of these brushes to understand the impact
rg "ThemeSelectionBackgroundBrush|ThemeChooserBackgroundBrush" --type xaml
# Look for any related theme documentation
fd -e md -e txt . | xargs rg "Selection.*Background|Chooser.*Background"
Length of output: 180
Script:
#!/bin/bash
# Search for any XAML/AXAML files containing these brush references
rg "ThemeSelectionBackgroundBrush|ThemeChooserBackgroundBrush" -t xml
# Look for brush definitions in resource dictionaries or theme files
fd -e xaml -e axaml . | xargs rg "ThemeSelectionBackgroundBrush|ThemeChooserBackgroundBrush"
# Search for any theme-related resource definitions
fd -e xaml -e axaml . | xargs rg "<.*Resource.*Key=\"Theme.*Background"
Length of output: 6018
src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml (2)
1-8: LGTM: Resource dictionary setup is well-structured
The namespace declarations and converter registration are properly organized.
Line range hint 9-100: Verify custom helper controls implementation
The control template is well-structured, but let's verify the implementation of custom helper controls used in the template.
✅ Verification successful
Custom helper controls are properly implemented and available
The verification confirms that all custom helper controls used in the MenuItem template are properly implemented in the codebase:
CaretControlextends ContentControl with caret visibility functionalitySymbolsControlis a sealed control implementing IDisposable for symbol renderingMenuExtensionsprovides the required FocusOnLoad attached property
The controls are located in the appropriate helpers directory and follow Avalonia's control implementation patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check implementation of custom helper controls
# Check CaretControl implementation
ast-grep --pattern 'class CaretControl {
$$$
}'
# Check SymbolsControl implementation
ast-grep --pattern 'class SymbolsControl {
$$$
}'
# Check MenuExtensions implementation (used in ItemsPresenter styles)
ast-grep --pattern 'class MenuExtensions {
$$$
}'
Length of output: 159
Script:
#!/bin/bash
# Let's try a different approach to find these helper controls
# First, let's locate the files containing these types
fd -e cs -e xaml "CaretControl|SymbolsControl|MenuExtensions"
# Then let's search for their content using ripgrep for more flexibility
rg "class (CaretControl|SymbolsControl)" -A 5
rg "static class MenuExtensions" -A 5
Length of output: 2573
src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml (3)
1-5: LGTM: Resource dictionary declarations are well-structured
The namespace declarations are properly organized and include all necessary references for the dialog window implementation.
6-6: Verify the border margin configuration
The ConsoloniaDialogWindowBorderMargin only specifies horizontal margins (1,0). Please verify if vertical margins are intentionally omitted, as this might affect the layout spacing above and below the dialog borders.
✅ Verification successful
Horizontal margins are intentionally used for dialog window borders
The horizontal-only margins (1,0) in ConsoloniaDialogWindowBorderMargin are intentionally designed this way as part of the TurboVision theme's dialog window layout. This is evidenced by:
- The margin is consistently applied to border elements and content to create uniform horizontal spacing
- The dialog's content panel has its own separate vertical margins (2,1,2,1) to handle vertical spacing
- The design follows a classic text-based UI pattern where horizontal borders need slight offset while vertical alignment is handled by the container
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other dialog-related margin definitions that might complement this
rg "DialogWindow.*Margin" --type xaml
Length of output: 68
Script:
#!/bin/bash
# Search for any usage of the ConsoloniaDialogWindowBorderMargin resource
rg "ConsoloniaDialogWindowBorderMargin"
# Search for dialog-related margin definitions with a broader pattern
rg "Margin.*1,0|DialogWindow.*Margin"
# Find all XAML files that might contain dialog window styles
fd ".*\.axaml$|.*\.xaml$"
Length of output: 11660
Script:
#!/bin/bash
# Let's examine the full DialogWindow template to understand the margin usage in context
cat "src/Consolonia.Themes.TurboVision/Templates/Controls/DialogWindow.axaml"
Length of output: 6607
Line range hint 97-103: Verify margin consistency
The content presenter uses different margin values (2,1,2,1) compared to the border margin (1,0). Please verify if this difference is intentional and necessary for proper layout.
✅ Verification successful
The margin differences are intentional and serve distinct purposes
The different margin values are intentionally designed to create a proper layout structure:
- The outer Panel has a margin of "2,1,2,1" to create proper spacing around the content area
- The ContentPresenter uses the "ConsoloniaDialogWindowBorderMargin" resource which is defined as "1,0" to align with the dialog window's border styling
This is a consistent pattern used throughout the dialog window template, where the same "ConsoloniaDialogWindowBorderMargin" (1,0) is applied to:
- The window border
- The title bar elements
- The content presenter
- The close button
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other content presenter margin definitions in dialog-related files
rg "PART_ContentPresenter.*Margin" --type xaml
Length of output: 77
Script:
#!/bin/bash
# Search for content presenter margin definitions in all files
rg "PART_ContentPresenter.*Margin"
Length of output: 36
Script:
#!/bin/bash
# Search for dialog window templates to understand the margin patterns
rg "DialogWindow.*Template" -A 10
Length of output: 35
Script:
#!/bin/bash
# Find all AXAML files and search for margin patterns
fd -e axaml -x rg "Margin=\"[^\"]*\"" {} -A 2
Length of output: 33285
Script:
#!/bin/bash
# Search for dialog window border margin definitions
rg "ConsoloniaDialogWindowBorderMargin" -B 5 -A 5
Length of output: 9450
Script:
#!/bin/bash
# Search for content presenter margin definitions specifically in dialog window templates
rg "PART_ContentPresenter.*Margin.*2,1,2,1|Panel.*Margin.*2,1,2,1" -A 2 -B 2
Length of output: 710
src/Consolonia.Themes.TurboVision/Templates/Controls/Menu.axaml (3)
1-4: LGTM! Namespace declarations are well-structured.
The namespace declarations follow best practices with proper separation of concerns between Avalonia core, custom helpers, and system types.
91-109: LGTM! Good accessibility support.
The Menu theme implementation is well-structured with proper keyboard navigation support through KeyboardNavigation.TabNavigation="Continue". The use of ItemContainerTheme ensures consistent styling of menu items.
1-110: Verify theme integration
As this file is part of a larger theme transition from Style to ControlTheme, let's verify the integration with other theme files.
✅ Verification successful
Theme integration is consistent
The verification shows that:
- Menu.axaml is properly included in AllControls.axaml
- All menu-related components (Menu, MenuItem, MenuFlyoutPresenter) consistently use ControlTheme
- No conflicting Style-based definitions found
- Theme hierarchy and resource references are properly maintained
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining Style-based menu definitions that might conflict
rg -l "Style.*TargetType=\"MenuItem\"" "src/Consolonia.Themes.TurboVision"
# Verify that this theme is properly included in the theme hierarchy
rg -l "Menu\.axaml" "src/Consolonia.Themes.TurboVision/**/*.axaml"
Length of output: 226
Script:
#!/bin/bash
# Let's try different approaches to verify theme integration
# Check for any Style-based menu definitions
rg "Style.*TargetType=\"MenuItem\"" src/
# Find Menu.axaml references in the theme
fd Menu.axaml
# Look for theme includes
rg -l "Menu\.axaml" src/
# Check if there are any other menu-related theme files
fd -e axaml -e xaml | rg -i "menu"
Length of output: 653
Script:
#!/bin/bash
# Let's check the content of related files to verify theme consistency
# Check how Menu.axaml is included in AllControls.axaml
cat src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml
# Check MenuItem.axaml for any potential conflicts
cat src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml
# Check MenuFlyoutPresenter.axaml for any potential conflicts
cat src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml
Length of output: 14225
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollViewer.axaml (3)
1-4: LGTM: Namespace declarations are well-structured
The ResourceDictionary includes all necessary namespaces with proper scoping.
62-65: LGTM: Disabled state styling follows conventions
The disabled state is properly handled using the new selector syntax and appropriate TurboVision theme brush.
124-127: LGTM: Symbol styling is properly implemented
The style for menu scroller symbols is correctly scoped and uses appropriate theme brushes.
src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml (4)
1-5: Well-structured transition to ControlTheme!
The shift from Styles to ControlTheme is a good architectural decision that aligns with Avalonia's best practices and improves maintainability.
Line range hint 1-153: Verify theme resource definitions
The template uses several dynamic resources. Please ensure these are properly defined:
- ThemeChooserBackgroundBrush
- ThemeSelectionBackgroundBrush
✅ Verification successful
Dynamic resources are properly defined across themes
The resources ThemeChooserBackgroundBrush and ThemeSelectionBackgroundBrush are consistently defined in all theme variants:
- TurboVisionDark theme
- Material theme
- TurboVisionBlack theme
- TurboVision default theme
- Fluent theme
Additionally, these resources are consistently used across multiple control templates in the theme, indicating proper integration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for resource definitions
rg "ThemeChooserBackgroundBrush|ThemeSelectionBackgroundBrush" src/Consolonia.Themes.TurboVision/
Length of output: 5729
118-150: Verify removal of interaction styles
The commented-out styles include important interaction feedback (hover, pressed states). Please verify:
- Was this removal intentional?
- Are these states handled elsewhere?
- How will users get visual feedback during interaction?
✅ Verification successful
Removal of interaction styles appears intentional for TurboVision theme
Looking at the complete ScrollBar template, this appears to be a deliberate design choice for a TurboVision-style UI theme that uses ASCII/Unicode characters (░, ▲, ▼, ◄, ►, │, ─) to create a retro console-like interface. The removal of hover and pressed states aligns with this design as traditional console UIs typically don't have such modern interaction states. The scrollbar's visual feedback is handled through basic foreground/background colors using ThemeChooserBackgroundBrush and ThemeSelectionBackgroundBrush.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these interaction styles are defined elsewhere
rg "ThemeControlHighBrush|ThemeControlVeryHighBrush" src/Consolonia.Themes.TurboVision/
Length of output: 542
Script:
#!/bin/bash
# Let's check for any other styles related to ScrollBar interaction
rg -A 5 "Selector=\"\^.*:(?:pointerover|pressed)" src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml
# Also check the complete ScrollBar template to understand the styling structure
cat src/Consolonia.Themes.TurboVision/Templates/Controls/ScrollBar.axaml
Length of output: 8624
83-117: Review hard-coded dimensions for scaling
The hard-coded dimensions (Width="1") might cause issues with:
- Different DPI settings
- Theme scaling
- High-resolution displays
Consider using relative units or theme resources for better scalability.
src/Consolonia.Themes.TurboVision/Templates/Controls/MenuFlyoutPresenter.axaml (3)
1-3: LGTM: Namespace declarations are properly structured
The ResourceDictionary declarations and namespace imports are well-organized, including the custom helpers namespace that's correctly referenced with its assembly specification.
Line range hint 21-48: Well-structured control template with proper layout and functionality
The template implementation demonstrates good practices:
- Proper use of template bindings for customization
- Correct implementation of scrolling behavior
- Good use of Grid.IsSharedSizeScope for layout coordination
- Appropriate keyboard navigation setup
Line range hint 19-20: Verify shadow effect visibility in high contrast themes
The shadow effect implemented using an offset background border might need verification for accessibility in high contrast themes.
✅ Verification successful
Let me gather more information about the shadow implementation and theme handling.
Let me check one more aspect about the ConsoleBrush implementation to understand how the shading mode works.
Let me check one final aspect about the PixelBackgroundMode enum to understand the shading behavior completely.
Shadow effect implementation is accessible and properly handled
The shadow effect is implemented using ConsoleBrush with a "Shaded" mode, which is consistently defined across all themes (Material, Fluent, TurboVision Dark, and TurboVision Black). The implementation:
- Uses a dedicated PixelBackgroundMode.Shaded enum value for shadow rendering
- Properly handles color blending and contrast as evidenced by the pixel tests
- Provides theme-specific shadow colors (#80767676 in Fluent, #80757575 in Material)
- Falls back to appropriate colors in high-contrast themes (Black in TurboVision Dark, DarkGray in TurboVision Black)
The shadow effect will maintain visibility and proper contrast across all themes, including high-contrast scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for high contrast theme handling in the codebase
rg -l "HighContrast"
# Look for shadow-related theme resources
rg "ThemeShadeBrush"
Length of output: 1746
Script:
#!/bin/bash
# Check the implementation of ThemeShadeBrush in different themes
rg -A 5 'ThemeShadeBrush' src/Consolonia.Themes.TurboVision/Themes/
# Look for any theme-specific customizations or overrides for high contrast
rg -l 'SystemParameters\.HighContrast|Theme.*High.*Contrast'
# Check if there's any specific accessibility handling
rg -l 'Accessibility|AccessibilityView'
Length of output: 3162
Script:
#!/bin/bash
# Look for ConsoleBrush implementation details
rg -A 10 'class ConsoleBrush'
# Check for any shading mode related code
rg -A 5 'Mode="Shaded"'
Length of output: 4718
Script:
#!/bin/bash
# Look for PixelBackgroundMode enum definition
rg -A 10 'enum PixelBackgroundMode'
# Check for any accessibility-related pixel rendering code
rg -A 5 'PixelBackgroundMode.*Shaded'
Length of output: 4978
src/Consolonia.Themes.TurboVision/Templates/Controls/AllControls.axaml (1)
3-44: Verify control theme dependencies
The restructuring to use MergeResourceInclude is a good improvement for resource organization. However, the order of includes could impact theme resolution if there are dependencies between controls.
✅ Verification successful
Resource dependencies are properly organized
Based on the analysis of resource references, the current order of control templates in AllControls.axaml is appropriate:
- All DynamicResource references are to theme-level resources (e.g., ThemeForegroundBrush, ThemeBackgroundBrush) rather than control-specific resources
- The few StaticResource references found are to converters or styles defined elsewhere
- No circular dependencies between controls were detected
- Basic controls (TextBlock, Window, Button) that others might depend on are loaded first
- More complex controls (Calendar, DataGrid) that might depend on basic controls are loaded later
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential circular dependencies in control themes
# Look for cross-references between control templates
# Search for resource references in control templates
rg "DynamicResource" --type xaml
Length of output: 61
Script:
#!/bin/bash
# Search for resource references in control templates without file type restriction
# Looking for potential dependencies between controls
rg "DynamicResource" src/Consolonia.Themes.TurboVision/Templates/Controls/
# Also check for StaticResource references as they can indicate dependencies
rg "StaticResource" src/Consolonia.Themes.TurboVision/Templates/Controls/
Length of output: 25451
src/Consolonia.Themes.TurboVision/Templates/Controls/CheckBox.axaml (1)
80-85: Color contrast needs attention in disabled state
The disabled state still uses ThemeNoDisturbBrush for both foreground and border, which may not provide sufficient contrast against the background colors.
src/Consolonia.Themes.TurboVision/Templates/Controls/DataGrid.axaml (1)
7-7: Address the TODO for Validation and Focus
The TODO comment regarding validation and focus handling should be addressed to ensure proper data grid functionality.
src/Consolonia.Themes.TurboVision/Templates/Controls/MenuItem.axaml
Outdated
Show resolved
Hide resolved
| { | ||
| AvaloniaXamlLoader.Load(sp, this); |
There was a problem hiding this comment.
ooooh. I didn't know there was a IServiceProvider signature for this. That's cool.
There was a problem hiding this comment.
I copypasted most of it from avalonia. Im even not sure how everything is supposed to work. I'm lost between styles, themes, and resource dictionaries.
Mostly tweeks. The weirdest thing was the binding stopped working, i have replaced it with another one. I couldn't debug it quick, looked like avalonia issue. |
There are number of issues, but seems mostly come from other changes.
I will re-test the application entirely and will create bugs whether they come from this PR or others.