Add new Icon infrastructure to BlazorUI (#11684) #11945
Conversation
WalkthroughA new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupItem.cs (1)
59-59: Remove incorrect[Parameter]attributes from non-component class.
BitButtonGroupItemis a plain class that does not inherit fromComponentBase. The[Parameter]attribute onOffIconName,OffText,OffTitle,OnIconName,OnText,OnTitle, andReversedIcon(lines 59, 64, 69, 89, 94, 99, 109) is invalid and has no effect. These attributes should be removed.🔎 Proposed fix
/// <summary> /// The icon of the item when it is not checked in toggle mode. /// </summary> - [Parameter] public string? OffIconName { get; set; } + public string? OffIconName { get; set; } /// <summary> /// The text of the item when it is not checked in toggle mode. /// </summary> - [Parameter] public string? OffText { get; set; } + public string? OffText { get; set; } /// <summary> /// The title of the item when it is not checked in toggle mode. /// </summary> - [Parameter] public string? OffTitle { get; set; } + public string? OffTitle { get; set; }Similar changes needed for
OnIconName,OnText,OnTitle, andReversedIconproperties.
🧹 Nitpick comments (5)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razor.cs (1)
299-316: Consider adding CDN link examples for external icon libraries.The example code shows how to use FontAwesome, Material Icons, and Bootstrap Icons, but doesn't include the required CDN links (unlike example12RazorCode in BitActionButtonDemo). Consider adding comments with the CDN links needed for each library to help users understand the full setup.
🔎 Suggested enhancement with CDN links
private readonly string example6RazorCode = @" +<!-- FontAwesome CSS --> +<link rel=""stylesheet"" href=""https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.5.1/css/all.min.css"" /> + +<!-- Material Icons CSS --> +<link rel=""stylesheet"" href=""https://fonts.googleapis.com/icon?family=Material+Icons"" /> + +<!-- Bootstrap Icons CSS --> +<link rel=""stylesheet"" href=""https://cdn.jsdelivr.net/npm/bootstrap-icons@1.11.0/font/bootstrap-icons.css"" /> + <!-- FontAwesome Icons (requires FontAwesome CSS to be loaded) --> <BitIcon Icon=""@BitIconInfo.FontAwesome(""fa-solid fa-house"")"" Size=""BitSize.Large"" />src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor (1)
68-73: Minor: Extra blank line at line 72.There's an extra blank line at line 72 that appears to be unintentional. While not a functional issue, consider removing it for consistency.
🔎 Proposed cleanup
@if (icon is not null) { <i style="@Styles?.Icon" class="bit-acb-ico @icon.GetCssClasses() @Classes?.Icon" /> } - @bodysrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor (1)
331-340: Potentially redundant FontAwesome loading.Line 336 states "FontAwesome 6 is already loaded in this demo" but line 340 adds another
<link>tag to load it. This could cause confusion or duplicate resource loading. Consider removing either the note or the<link>tag for consistency.🔎 Suggested options
Option 1: Remove the note if the
<link>tag is needed for the demo to work standalone:- <div><b>Note:</b> FontAwesome 6 is already loaded in this demo. The Icon parameter takes precedence over IconName.</div> + <div><b>Note:</b> The Icon parameter takes precedence over IconName.</div>Option 2: Remove the
<link>tag if FontAwesome is already loaded globally:- <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.5.1/css/all.min.css" integrity="sha512-DTOQO9RWCH3ppGqcWaEA1BIZOC6xxalwEsw9c2QQeAIftl+Vegovlnee1c9QX4TctnWMn13TZye+giMm8e2LwA==" crossorigin="anonymous" referrerpolicy="no-referrer" />src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor (1)
363-395: Good demonstration of external icon library integration.The "External Icons" demo effectively showcases FontAwesome usage with clear documentation. The note about Icon parameter precedence over IconName is helpful for users.
One consideration: loading external stylesheets inline via
<link>in a component may cause redundant network requests if this demo is viewed multiple times. For production demos, consider loading the stylesheet once at the app level or using a conditional check.src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIconInfo.cs (1)
139-184: Factory methods provide intuitive API for common icon libraries.The semantic methods (
FontAwesome,Material,Bootstrap) make the API self-documenting. TheCssmethod serves as a generic escape hatch for any CSS-based icon system.Note:
FontAwesome()andCss()have identical implementations, which is intentional for API clarity. Consider adding a brief comment noting this design choice for future maintainers.
📜 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 (34)
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/Bit.BlazorUI/Components/Buttons/Button/BitButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/Button/BitButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupItem.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonItem.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIcon.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIconInfo.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/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/Button/BitButtonDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/Button/BitButtonDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/Button/BitButtonDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.samples.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ToggleButton/BitToggleButtonDemo.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Utilities/Icon/BitIconInfoTests.cs
🧰 Additional context used
🧬 Code graph analysis (12)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonItem.cs (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
BitIconInfo(347-371)src/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor.cs (1)
BitIconInfo(271-285)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/Button/BitButton.razor.cs (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
BitIconInfo(347-371)src/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor.cs (1)
BitIconInfo(271-285)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIcon.razor.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIconInfo.cs (10)
BitIconInfo(7-205)BitIconInfo(12-12)BitIconInfo(18-21)BitIconInfo(37-42)BitIconInfo(139-142)BitIconInfo(153-156)BitIconInfo(167-170)BitIconInfo(181-184)BitIconInfo(197-204)GetCssClasses(83-106)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupItem.cs (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
BitIconInfo(347-371)src/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor.cs (1)
BitIconInfo(271-285)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
BitIconInfo(347-371)src/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor.cs (1)
BitIconInfo(271-285)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonItem.cs (1)
BitMenuButtonItem(3-64)src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs (1)
BitMenuButtonOption(3-92)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.cs (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/Operation.cs (1)
Operation(3-34)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupItem.cs (1)
BitButtonGroupItem(4-135)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIconInfo.cs (9)
BitIconInfo(7-205)BitIconInfo(12-12)BitIconInfo(18-21)BitIconInfo(37-42)BitIconInfo(139-142)BitIconInfo(153-156)BitIconInfo(167-170)BitIconInfo(181-184)BitIconInfo(197-204)
src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Utilities/Icon/BitIconInfoTests.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIconInfo.cs (1)
GetCssClasses(83-106)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIconInfo.cs (1)
src/BlazorUI/Bit.BlazorUI/Extensions/StringExtensions.cs (1)
HasNoValue(12-17)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
BitIconInfo(347-371)
⏰ 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 (45)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ToggleButton/BitToggleButtonDemo.razor.cs (1)
68-138: LGTM! Clear documentation of Icon precedence.The addition of
Icon,OffIcon, andOnIconparameters (typeBitIconInfo?) properly documents their precedence over the correspondingIconNamevariants and clarifies thatIconNameis for built-in Fluent UI icons. This aligns well with the PR's objective to support external icon libraries.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/Button/BitButtonDemo.razor.samples.cs (2)
520-542: LGTM! Excellent FontAwesome integration example.The new
example17RazorCodedemonstrates proper usage of external icons viaBitIconInfo.FontAwesome()with various color and variant combinations. The inclusion of the FontAwesome stylesheet link (line 522) is a helpful reminder for users.
544-659: LGTM! Sample renumbering is correct.The renaming of example blocks (17→18, 18→19) accommodates the new FontAwesome example while maintaining sequential numbering.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs (2)
404-434: LGTM! C# sample code structure is correct.The example12CsharpCode properly defines the Operation class and demonstrates styleClassCustoms with inline styles and custom CSS classes.
437-548: LGTM! Sample renumbering is correct.The renaming of example blocks (12→13, 13→14) maintains sequential ordering after the new external icon example.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor.cs (1)
91-103: LGTM! Consistent Icon parameter addition.The
Iconparameter (typeBitIconInfo?) is properly documented with clear precedence rules and the updatedIconNamedescription correctly specifies "built-in Fluent UI icons". This matches the pattern used across other button components in this PR.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/Button/BitButtonDemo.razor.cs (1)
128-140: LGTM! Consistent Icon parameter addition.The
Iconparameter addition follows the same pattern as other button components, with clear documentation of external icon library support and precedence overIconName. The updatedIconNamedescription properly clarifies it's for built-in Fluent UI icons.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/MenuButton/BitMenuButtonDemo.razor.cs (1)
77-89: LGTM! Clear documentation for the new Icon parameter.The documentation correctly describes the Icon parameter's precedence over IconName and provides helpful examples for external icon libraries (FontAwesome, Material, Custom CSS).
src/BlazorUI/Bit.BlazorUI/Components/Buttons/Button/BitButton.razor.cs (1)
119-132: LGTM! Well-documented Icon parameter.The documentation is comprehensive with clear remarks and practical examples for different icon libraries.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor.cs (1)
93-121: LGTM! Consistent Icon parameter implementation.The Icon parameter is well-documented with clear precedence rules and examples, consistent with BitButton and other components.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonItem.cs (1)
10-28: LGTM! Consistent Icon property implementation.The Icon property is properly documented with clear examples and precedence rules, consistent with the broader icon improvements.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButton.razor (2)
7-7: LGTM! Efficient icon resolution.Computing the icon once at the top of the component is efficient and clean. The
BitIconInfo.Frommethod properly implements the precedence rule (Icon over IconName).
41-46: LGTM! Proper icon rendering with external library support.The refactored rendering correctly uses
icon.GetCssClasses()to support both built-in Fluent UI icons and external icon libraries while maintaining the same HTML structure.src/BlazorUI/Bit.BlazorUI/Components/Buttons/Button/BitButton.razor (3)
7-7: LGTM! Consistent icon resolution pattern.The icon resolution follows the same efficient pattern as BitActionButton, computing the icon once at the component level.
26-33: LGTM! Proper icon rendering with IconUrl fallback.The refactored icon rendering correctly:
- Uses
icon.GetCssClasses()for BitIconInfo-based icons- Preserves the IconUrl fallback for image-based icons
- Maintains the same HTML structure
98-105: LGTM! Consistent anchor rendering.The anchor rendering path correctly mirrors the button rendering logic with the same icon handling and IconUrl fallback.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor (1)
21-25: LGTM!The transition from string-based icon handling to
BitIconInfo?is well-implemented. The null-check replacesHasValue()appropriately, andGetCssClasses()correctly generates the CSS classes for both built-in and external icon libraries.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs (1)
15-28: LGTM!The new
Iconproperty is well-documented with clear examples for external icon libraries. The XML documentation correctly states the precedence overIconNameand aligns with the parallel implementation inBitMenuButtonItem.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ActionButton/BitActionButtonParams.cs (2)
65-73: LGTM!The
Iconproperty documentation is thorough and consistent with other components. The remarks section clearly explains the use case for external icon libraries.
228-231: LGTM!The
UpdateParameterslogic for the newIconproperty correctly follows the established pattern: it propagates the value only ifIconis not null and hasn't been explicitly set on the target component.src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIcon.razor.cs (3)
20-34: LGTM!The
Iconproperty is well-documented with practical examples for FontAwesome, Material, and custom CSS. The[ResetClassBuilder]attribute ensures CSS classes are rebuilt when the icon changes.
49-49: LGTM!The expression-bodied
_iconproperty correctly applies the precedence rule viaBitIconInfo.From(). This approach is appropriate as the resolution logic is lightweight.
78-78: LGTM!Using
_icon?.GetCssClasses()correctly generates the appropriate CSS classes for both built-in and external icons, with null-safety handled properly.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor (3)
38-41: LGTM!The icon handling for selected items correctly uses the new
BitIconInfoapproach with null-check andGetCssClasses()for CSS generation.
50-59: LGTM!The header icon resolution via
BitIconInfo.From(Icon, IconName)correctly implements the precedence rule. The null-check andGetCssClasses()usage is consistent with other icon rendering paths.
121-124: LGTM!Item icon handling follows the same pattern as header and selected item icons, ensuring consistent behavior across all icon rendering scenarios in the menu button.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor (1)
342-362: LGTM!The External Icons demo effectively showcases the new
BitIconInfo-based icon support across different variants and colors, demonstrating the flexibility of the new API for external icon libraries.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ToggleButton/BitToggleButton.razor.cs (4)
54-67: LGTM!The
Iconproperty is well-documented with clear examples for external icon libraries. The documentation style is consistent with other components in this PR.
96-109: LGTM!The
OffIconproperty follows the same documentation pattern asIcon, with appropriate examples and remarks about external library usage.
126-139: LGTM!The
OnIconproperty completes the set of external icon support properties with consistent documentation.
271-285: LGTM!The
GetIcon()method correctly implements the icon resolution logic:
- When checked, resolves
OnIcon/OnIconNameviaBitIconInfo.From()- When not checked and an off-icon exists, returns the
OffIcon/OffIconNameresolution- Falls back to
Icon/IconNameotherwiseThis mirrors the logic in
GetText()andGetTitle()methods, maintaining API consistency.src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Utilities/Icon/BitIconInfoTests.cs (1)
1-355: Comprehensive test coverage for the new BitIconInfo API.The test suite thoroughly covers:
- All constructor overloads and property initialization
- CSS class generation logic with various combinations of BaseClass, Prefix, and Name
- Factory methods for FontAwesome, Material, Bootstrap, and custom CSS icons
- Implicit conversions between string and BitIconInfo
- Edge cases including null, empty, and whitespace values
The tests align well with the
GetCssClasses()implementation shown in the relevant code snippets and will help ensure backward compatibility as the API evolves.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs (1)
16-29: Well-documented Icon properties with consistent API.The new
Icon,OffIcon, andOnIconproperties follow the established pattern and include thorough XML documentation with usage examples. The precedence rules (Icon takes precedence over IconName) are clearly documented, matching theBitIconInfo.From()behavior seen in related components likeBitToggleButton.GetIcon().Also applies to: 46-59, 76-89
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupItem.cs (1)
11-24: New Icon properties are correctly implemented.The new
Icon,OffIcon, andOnIconproperties of typeBitIconInfo?are correctly added without the[Parameter]attribute (since this is a POCO class), and include comprehensive XML documentation consistent withBitButtonGroupOption.Also applies to: 41-54, 71-84
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor.samples.cs (1)
405-437: Sample code accurately demonstrates the new Icon parameter API.The
example12RazorCodeproperly shows:
- Loading FontAwesome CSS from CDN
- Using
BitIconInfo.FontAwesome()factory method- Various icon styles (fa-solid, fa-brands) and color combinations
This provides good reference documentation for users adopting the new external icon support.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/Button/BitButtonDemo.razor (1)
570-611: Well-structured External Icons demonstration for BitButton.The demo effectively shows:
- FontAwesome icons with various styles (fa-solid, fa-brands)
- Different color variants (Primary, Secondary, Tertiary, Error, Success)
- Clear note about Icon parameter precedence
The demonstration is consistent with the pattern used in other button component demos.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (2)
75-88: Icon property follows established pattern with comprehensive documentation.The new
Iconproperty is well-documented with examples for FontAwesome, Material, and custom CSS icons, matching the documentation style used in other button components.
347-371: GetIcon method correctly implements BitIconInfo resolution.The refactored
GetIconmethod properly usesBitIconInfo.From()to merge theIconandIconNameproperties with correct precedence (Icon takes precedence). The implementation pattern is consistent withBitToggleButton.GetIcon().src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor (1)
455-493: External Icons demo for BitActionButton is complete and consistent.The demonstration shows FontAwesome icons across different color variants and follows the same pattern established in the other button component demos. The note about Icon precedence over IconName provides useful guidance.
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupOptionDemo.razor (1)
562-614: Well-structured demo for external icon support.The new "External Icons" section effectively demonstrates the
BitIconInfo.FontAwesome()factory method across different variants and colors. The note aboutIcontaking precedence overIconNameis helpful for developers.One consideration: Loading the FontAwesome CSS via an inline
<link>tag (line 571) works for demo purposes but would cause redundant network requests if the component is rendered multiple times. For production use, the CSS should be loaded in the application's head section.src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIconInfo.cs (5)
7-12: LGTM - Clean class structure with good documentation.The class design allows flexible icon configuration while maintaining backward compatibility with existing
IconNamestring parameters through implicit conversions.
14-42: Constructors provide appropriate flexibility.The three constructor overloads cover the expected use cases: simple built-in icons, and fully customized external icons. Documentation is comprehensive.
83-106: Well-implemented CSS class generation logic.The method correctly handles all combinations of
BaseClassandPrefixvalues, with appropriate defaults for built-in Fluent UI icons while supporting external icon libraries that may not need a base class or prefix.
108-127: Implicit operators ensure backward compatibility.These conversions allow existing code using
stringicon names to continue working without modification, while new code can leverage the fullBitIconInfoAPI.
186-204: Clean resolution logic for Icon/IconName precedence.The
From()method provides a single point of resolution for components that support bothIconandIconNameparameters, ensuring consistent behavior across the component library.
....Demo.Client.Core/Pages/Components/Buttons/ActionButton/BitActionButtonDemo.razor.samples.cs
Outdated
Show resolved
Hide resolved
...rUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.cs
Outdated
Show resolved
Hide resolved
....Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupCustomDemo.razor.samples.cs
Outdated
Show resolved
Hide resolved
...zorUI.Demo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.cs
Outdated
Show resolved
Hide resolved
...mo.Client.Core/Pages/Components/Buttons/ButtonGroup/_BitButtonGroupItemDemo.razor.samples.cs
Outdated
Show resolved
Hide resolved
.../Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razor
Outdated
Show resolved
Hide resolved
msynk
force-pushed
the
11684-blazorui-icons-improvements
branch
from
10d7067 to
4168a88
Compare
msynk
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @src/BlazorUI/Bit.BlazorUI/Components/BitIconInfo.cs:
- Around line 155-160: The Fa method can produce empty tokens when icons
contains extra spaces; update the BitIconInfo Fa(string icons) implementation to
trim the input and split/filter out empty entries (e.g., use
StringSplitOptions.RemoveEmptyEntries or a Where check) before mapping each
token to prepend "fa-" when it doesn't already start with "fa-"; keep building
cssClasses with string.Join(' ', ... ) and return new BitIconInfo(cssClasses,
baseClass: "", prefix: "") unchanged.
🧹 Nitpick comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razor (1)
139-139: Consider moving external CSS links to a central location.Inline
<link>tags within component markup work but are unconventional. For a demo page this is acceptable, but consider adding a note that production apps should load these stylesheets in the document head or via a CSS bundler for better performance and maintainability.Also applies to: 153-153
📜 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 (5)
src/BlazorUI/Bit.BlazorUI/Components/BitIconInfo.cssrc/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIcon.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Utilities/Icon/BitIconInfoTests.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Utilities/Icon/BitIconInfoTests.cs
🧰 Additional context used
🧬 Code graph analysis (1)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIcon.razor.cs (1)
src/BlazorUI/Bit.BlazorUI/Components/BitIconInfo.cs (10)
BitIconInfo(7-201)BitIconInfo(12-12)BitIconInfo(20-23)BitIconInfo(39-44)BitIconInfo(127-130)BitIconInfo(141-144)BitIconInfo(155-160)BitIconInfo(171-174)BitIconInfo(193-200)GetCssClasses(70-90)
🔇 Additional comments (10)
src/BlazorUI/Bit.BlazorUI/Components/Utilities/Icon/BitIcon.razor.cs (2)
16-25: LGTM! Well-documented new Icon parameter.The new
Iconparameter with[ResetClassBuilder]attribute correctly triggers CSS class rebuilding when the icon configuration changes. The documentation clearly explains the precedence behavior and intended use case for external icon libraries.
76-76: No action needed.ClassBuilder.Registeralready gracefully handles null and empty string returns through itsWhere(s => s.HasValue())filter in theBuild()method, which excludes null and whitespace-only values before joining class names. Even whenBitIconInfo.From()returns null (making the null-conditional operator return null), the filter prevents empty class entries from being added.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razor (2)
143-149: Good demonstration of multiple BitIconInfo usage patterns.The examples effectively showcase different approaches:
- Direct string assignment via implicit conversion:
Icon="@("fa-solid fa-house")"BitIconInfo.Css()for raw CSS classesBitIconInfo.Fa()for FontAwesome-specific helperThis provides clear guidance for developers on available options.
139-139: No action required. FontAwesome version 7.0.1 is available on cdnjs and the CDN link is valid and accessible.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Utilities/Icon/BitIconDemo.razor.cs (2)
54-85: Well-structured componentSubClasses documentation.The
BitIconInfosub-class documentation clearly describes each property (Name,BaseClass,Prefix) with practical examples for external icon libraries. This provides excellent API discoverability for developers.
307-321: Example code matches the demo implementation.The
example4RazorCodeaccurately reflects the external icon demonstrations in the.razorfile, showing both FontAwesome and Bootstrap Icons usage patterns.src/BlazorUI/Bit.BlazorUI/Components/BitIconInfo.cs (4)
70-90: Clean implementation of CSS class generation logic.The
GetCssClasses()method handles all combinations ofBaseClassandPrefixcorrectly, returning the appropriate CSS class string for each scenario.
99-104: Implicit conversion from string creates BitIconInfo without Bit defaults.When a string is implicitly converted to
BitIconInfo, it creates an instance with onlyNameset, leavingBaseClassandPrefixasnull. This meansGetCssClasses()will return just the raw string (line 76 branch). However, theFrom()method on line 199 usesBit()which sets the proper defaults.Verify this asymmetry is intentional: direct string assignment to
Iconyields raw CSS classes, whileIconNamegoes throughFrom()and gets Bit defaults.
193-200: Well-designed From() resolver method.The
From()method provides clean resolution logic withIcontaking precedence overIconName. Usinginternalvisibility appropriately restricts this to component-level usage.
72-72: Extension method is correctly accessible.The
HasNoValue()extension method is defined in the same namespace (Bit.BlazorUI) asBitIconInfo, making it directly accessible without any additional using statements. No verification or changes needed.Likely an incorrect or invalid review comment.
msynk
changed the title
2 participants
closes #11684
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.