-
-
Notifications
You must be signed in to change notification settings - Fork 258
Add new Icon parameter to BitActionButton (#11958) #11959
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 new Icon parameter to BitActionButton (#11958) #11959
Conversation
WalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
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 (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor.cs (1)
93-105: Clarify FontAwesome usage pattern in documentation examples.The XML documentation example shows:
Icon="BitIconInfo.Fa("solid house")"However, the demo file shows both shorthand and full class patterns:
- Shorthand:
BitIconInfo.Fa("solid rocket")- Full:
BitIconInfo.Fa("fa-brands fa-github")For consistency and clarity, consider either:
- Documenting both supported formats with examples, or
- Standardizing on one format throughout documentation and demos
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor (1)
455-514: Consider moving external CSS links to app-level resources.The inline
<link>tags on Lines 461 and 489 load external CSS each time this demo component renders. For a demo page this is acceptable, but consider moving these to a shared location (e.g.,index.html,App.razor, or_Host.cshtml) to avoid potential duplication and improve loading performance.The Icon parameter examples effectively demonstrate the new feature with FontAwesome and Bootstrap Icons using various BitIconInfo helper methods.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButtonParams.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor.samples.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ActionButton/BitActionButtonTests.cs
🧰 Additional context used
🧬 Code graph analysis (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButtonParams.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/BitComponentBase.cs (1)
HasNotBeenSet(349-352)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ActionButton/BitActionButtonTests.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor.cs (1)
BitActionButton(9-300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build Bit.BlazorUI
🔇 Additional comments (15)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor.cs (1)
108-121: LGTM! IconName documentation updated appropriately.The updated documentation clearly directs users to the new
Iconparameter for external libraries while maintaining backward compatibility guidance for built-in Fluent UI icons.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor.samples.cs (1)
280-379: LGTM! Sample code accurately reflects demo changes.The new
example12RazorCodedemonstrates external icon usage with both FontAwesome and Bootstrap Icons, and the subsequent examples are correctly renumbered to maintain consistency with the demo page structure.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButtonParams.cs (2)
66-73: LGTM! Icon property properly defined.The
Iconproperty is correctly typed asBitIconInfo?with comprehensive documentation that matches the main component's parameter definition.
228-231: LGTM! Icon parameter propagation follows established pattern.The
UpdateParameterslogic correctly propagates theIconproperty when set, following the same pattern used for other parameters with appropriate null andHasNotBeenSetchecks.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor.cs (3)
91-98: LGTM! Clear and comprehensive documentation.The Icon parameter documentation is well-structured and clearly explains the precedence behavior over IconName. The link to the BitIconInfo subclass documentation provides good navigation.
104-104: LGTM! Clarifies the distinction between IconName and Icon.The updated description clearly indicates that IconName is for built-in Fluent UI icons, helping users understand when to use each parameter.
233-261: LGTM! Comprehensive BitIconInfo documentation.The BitIconInfo subclass documentation is thorough and includes helpful examples for both built-in and external icon libraries. The descriptions clearly explain the purpose of each property.
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ActionButton/BitActionButtonTests.cs (5)
53-71: LGTM! Well-structured test for CSS class rendering.The test correctly verifies that CSS classes from the Icon parameter are rendered on the icon element. Using DataRow to test multiple icon library formats (FontAwesome, Bootstrap Icons) provides good coverage.
73-85: LGTM! Validates the Css helper method.The test correctly verifies that
BitIconInfo.Css()renders the provided CSS classes on the icon element.
87-99: LGTM! Validates the FontAwesome helper method.The test correctly verifies that
BitIconInfo.Fa()transforms the input into proper FontAwesome CSS class format (fa-solid fa-rocket).
101-113: LGTM! Validates the Bootstrap Icons helper method.The test correctly verifies that
BitIconInfo.Bi()generates proper Bootstrap Icons CSS classes (biandbi-github).
115-133: LGTM! Critical test for precedence behavior.This test correctly verifies that the Icon parameter takes precedence over IconName when both are set, which is a key requirement of the new feature. The negative assertions ensure IconName classes are not rendered.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor (3)
68-71: LGTM! Anchor path mirrors button path icon rendering.The icon rendering logic in the anchor (
<a>) element correctly mirrors the changes in the button path (lines 41-44), ensuring consistent behavior whether the component renders as a button or link.
7-7: BitIconInfo.From implementation and precedence behavior verified.The
BitIconInfo.From(Icon, IconName)method correctly implements the expected behavior: it returnsnullwhen neither parameter is provided, gives precedence toIconoverIconName, and falls back to creating a standard Bit icon when onlyiconNameis supplied.
41-44: Icon rendering logic correctly updated for new Icon parameter.The changes properly handle both the new
Iconparameter and the existingIconNameby checkingicon is not null(whereiconis derived from both parameters). The dynamic CSS class generation viaicon.GetCssClasses()enables support for external icon libraries like FontAwesome and Bootstrap Icons.
closes #11958
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.