-
-
Notifications
You must be signed in to change notification settings - Fork 232
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
Add Classes/Styles parameters in the BitMessageBox (#9637) #9638
Add Classes/Styles parameters in the BitMessageBox (#9637) #9638
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces enhanced customization capabilities for the Changes
Assessment against linked issues
Poem
Finishing Touches
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: 0
🧹 Nitpick comments (8)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MessageBox/BitMessageBox.razor (1)
3-4
: Consider using consistent string interpolation for class bindings.While the implementation is correct, consider using string interpolation consistently for class bindings. For example:
-<div style="@Styles?.Root" class="bit-msb @Classes?.Root"> +<div style="@Styles?.Root" class="@($"bit-msb {Classes?.Root}")">This pattern would be more consistent with how class concatenation is typically handled in Blazor components.
Also applies to: 5-6, 8-9, 13-13, 23-23, 27-31
src/BlazorUI/Bit.BlazorUI.Extras/Components/MessageBox/BitMessageBoxClassStyles.cs (1)
3-49
: LGTM! Well-structured styling class with comprehensive documentation.The
BitMessageBoxClassStyles
class provides a clean and well-documented structure for styling customization. Consider for future enhancement:
- Consider adding an interface (e.g.,
IBitMessageBoxClassStyles
) to allow for different styling implementations or mocking in tests- This would enhance testability and allow for future styling strategy implementations
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor (2)
41-59
: Consider enhancing the style examples for better maintainability.While the examples effectively demonstrate both styling approaches, consider these improvements:
- Move the gradient background style to a CSS class for better maintainability:
-Styles="@(new() { Root = "background: linear-gradient(180deg, #222444, transparent) #000", OkButton = new() { Root = "border-radius:1rem" } })" +Classes="@(new() { Root = "gradient-background", OkButton = new() { Root = "rounded-button" } })"
- Add comments explaining the styling approaches and their use cases
- Consider showing an example combining both Classes and Styles for more complex scenarios
43-43
: Fix typo in the example description.-<div>Customize the appearance of BitActionButton using styles and CSS classes.</div> +<div>Customize the appearance of BitMessageBox using styles and CSS classes.</div>src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor.samples.cs (1)
68-72
: Consider using CSS variables for better maintainabilityThe inline styles could be moved to CSS variables for better reusability and maintainability.
- Styles=""@(new() { Root = ""background: linear-gradient(180deg, #222444, transparent) #000"", OkButton = new() { Root = ""border-radius:1rem"" } })"" /> + Styles=""@(new() { Root = ""background: var(--bit-messagebox-gradient)"", OkButton = new() { Root = ""border-radius:var(--bit-button-radius)"" } })"" />src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor.scss (3)
2-4
: Consider using CSS variables for theme colorsMove color values to CSS variables for better theme customization.
- background: linear-gradient(180deg, #3e0f0f, transparent) #000; + background: linear-gradient(180deg, var(--bit-messagebox-gradient-start), transparent) var(--bit-messagebox-background);
6-13
: Optimize button styles with CSS variablesConsider using CSS variables for colors and common values.
.custom-msg-btn { - color: #fff; + color: var(--bit-button-text-color); font-weight: bold; border-radius: 1rem; - border-color: #8f0101; + border-color: var(--bit-button-border-color); transition: background-color 1s; - background: linear-gradient(90deg, #d10000, transparent) #8f0101; + background: linear-gradient(90deg, var(--bit-button-gradient-start), transparent) var(--bit-button-background); }
15-20
: DRY: Reduce duplicate properties in hover stateOnly include properties that actually change in the hover state.
.custom-msg-btn:hover { - color: #fff; - font-weight: bold; - border-color: #8f0101; - background-color: #8f0101; + background-color: var(--bit-button-hover-background); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MessageBox/BitMessageBox.razor
(1 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/MessageBox/BitMessageBox.razor.cs
(2 hunks)src/BlazorUI/Bit.BlazorUI.Extras/Components/MessageBox/BitMessageBoxClassStyles.cs
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor
(2 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor.cs
(3 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor.samples.cs
(1 hunks)src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor.scss
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build and test
🔇 Additional comments (5)
src/BlazorUI/Bit.BlazorUI.Extras/Components/MessageBox/BitMessageBox.razor.cs (1)
12-15
: LGTM! Well-structured parameter additions.The new
Classes
andStyles
parameters are well-documented and follow consistent naming conventions with existing parameters. The nullable type is appropriate for optional styling properties.Also applies to: 27-30
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor.samples.cs (1)
74-78
: LGTM! Good demonstration of class-based stylingThe example effectively demonstrates the new Classes property with custom CSS classes, providing a clear pattern for users to follow.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Extras/MessageBox/BitMessageBoxDemo.razor.cs (3)
14-22
: LGTM! Well-documented Classes parameterThe Classes parameter is properly documented with type, default value, and helpful link to detailed styles documentation.
38-45
: LGTM! Well-documented Styles parameterThe Styles parameter mirrors the Classes parameter with consistent documentation structure.
55-132
: Comprehensive documentation of style customization optionsThe componentSubClasses documentation thoroughly covers all customizable elements of the BitMessageBox. Good job including links to related button styling documentation.
This closes #9637
Summary by CodeRabbit
Release Notes
New Features
BitMessageBox
component with advanced customization optionsImprovements
Documentation