-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve ScrollBar style #128
Conversation
WalkthroughThe changes add new style resources to the Avalonia desktop application. In the main application file, a Changes
Possibly related PRs
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
vATIS.Desktop/Ui/Styles/ScrollViewerStyles.axaml (2)
15-19
: Consider using consistent dimensions for vertical and horizontal thumbs.The vertical thumb width (8) is significantly smaller than the horizontal thumb height (8 vs 30). This might create an inconsistent visual experience.
-<x:Double x:Key="ScrollBarHorizontalThumbHeight">8</x:Double> +<x:Double x:Key="ScrollBarHorizontalThumbHeight">8</x:Double>
42-44
: Consider adding a transition for opacity changes.The opacity binding would benefit from a smooth transition when the pointer leaves the ScrollViewer.
-<Setter Property="Opacity" Value="{Binding $parent[ScrollViewer].IsPointerOver}" /> +<Setter Property="Opacity" Value="{Binding $parent[ScrollViewer].IsPointerOver}" /> +<Setter Property="Transitions"> + <Transitions> + <DoubleTransition Property="Opacity" Duration="0:0:0.1" /> + </Transitions> +</Setter>vATIS.Desktop/Ui/Styles/ScrollBarResources.axaml (2)
122-122
: Consider adding tooltip for improved accessibility.The hitbox areas could benefit from tooltips to indicate scrolling functionality for screen readers.
-<Border x:Name="Hitbox" Margin="-4" Background="Transparent"/> +<Border x:Name="Hitbox" Margin="-4" Background="Transparent" + ToolTip.Tip="Use mouse wheel or drag to scroll"/>Also applies to: 203-203
149-149
: Consider using XAML Path data for better maintainability.The arrow icons are defined using complex path data. Consider moving these to a resource dictionary for better maintainability and reuse.
Example:
<ResourceDictionary> <x:String x:Key="ScrollBarUpArrow">M 19.091797 14.970703 L 10 5.888672 L 0.908203 14.970703 L 0.029297 14.091797 L 10 4.111328 L 19.970703 14.091797 Z</x:String> <!-- Add other arrow paths --> </ResourceDictionary>Then reference them:
-Data="M 19.091797 14.970703 L 10 5.888672 L 0.908203 14.970703 L 0.029297 14.091797 L 10 4.111328 L 19.970703 14.091797 Z" +Data="{StaticResource ScrollBarUpArrow}"Also applies to: 187-187, 230-230, 267-267
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
vATIS.Desktop/App.axaml
(1 hunks)vATIS.Desktop/Ui/Styles/ScrollBarResources.axaml
(1 hunks)vATIS.Desktop/Ui/Styles/ScrollViewerStyles.axaml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (cpp)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (cpp)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (1)
vATIS.Desktop/App.axaml (1)
16-16
: LGTM!The new style includes are properly added and follow the existing pattern.
Also applies to: 29-29
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
vATIS.Desktop/Ui/Styles/ScrollBarResources.axaml (1)
149-151
: Consider moving PathIcon data to resources.The arrow paths could be moved to resources for better maintainability and reusability.
+ <PathGeometry x:Key="ScrollBarUpArrow">M 19.091797 14.970703 L 10 5.888672 L 0.908203 14.970703 L 0.029297 14.091797 L 10 4.111328 L 19.970703 14.091797 Z</PathGeometry> + <PathGeometry x:Key="ScrollBarDownArrow">M 18.935547 4.560547 L 19.814453 5.439453 L 10 15.253906 L 0.185547 5.439453 L 1.064453 4.560547 L 10 13.496094 Z</PathGeometry> + <PathGeometry x:Key="ScrollBarLeftArrow">M 14.091797 19.970703 L 4.111328 10 L 14.091797 0.029297 L 14.970703 0.908203 L 5.888672 10 L 14.970703 19.091797 Z</PathGeometry> + <PathGeometry x:Key="ScrollBarRightArrow">M 5.029297 19.091797 L 14.111328 10 L 5.029297 0.908203 L 5.908203 0.029297 L 15.888672 10 L 5.908203 19.970703 Z</PathGeometry> - <PathIcon - Data="M 19.091797 14.970703 L 10 5.888672 L 0.908203 14.970703 L 0.029297 14.091797 L 10 4.111328 L 19.970703 14.091797 Z" + <PathIcon + Data="{StaticResource ScrollBarUpArrow}" Width="{DynamicResource ScrollBarButtonArrowIconFontSize}" Height="{DynamicResource ScrollBarButtonArrowIconFontSize}" />Also applies to: 187-189, 230-232, 267-269
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
vATIS.Desktop/Ui/Styles/ScrollBarResources.axaml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (macos-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze (cpp)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (cpp)
- GitHub Check: Analyze (csharp)
🔇 Additional comments (3)
vATIS.Desktop/Ui/Styles/ScrollBarResources.axaml (3)
4-7
: LGTM! Scrollbar dimensions follow modern UI guidelines.The dimensions are well-balanced for both vertical and horizontal scrollbars, ensuring good usability while maintaining a modern, slim appearance.
9-24
: LGTM! Comprehensive design preview covering all scrollbar variations.The preview section thoroughly tests all combinations of:
- Orientations (horizontal/vertical)
- States (enabled/disabled)
- Visibility modes (auto-hide/always-visible)
26-296
: LGTM! Well-structured control themes with proper state handling.The themes are well-implemented with:
- Smooth transitions between states
- Proper use of dynamic resources
- Comprehensive state handling (hover, pressed, disabled)
- Clean and maintainable templates
Summary by CodeRabbit