Add new Icon parameters to BitNavBar (#12111)#12112
Add new Icon parameters to BitNavBar (#12111)#12112msynk merged 2 commits intobitfoundation:developfrom
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 Use the checkbox below for a quick retry:
WalkthroughThe pull request adds support for external icon libraries in the BitNavBar component by introducing a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 6
🧹 Nitpick comments (4)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarOptionDemo.razor (1)
389-389:<link>in component body — prefer<HeadContent>in Blazor.Placing
<link rel="stylesheet">inline in the component body renders the tag into the<body>at the component's location. Blazor provides the<HeadContent>component specifically for injecting<head>content from a component. Using it ensures the stylesheet loads correctly regardless of the rendering model (SSR, WASM, hybrid) and avoids duplicate loads on re-renders.♻️ Proposed fix
- <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css" /> - - <div class="container"> + <HeadContent> + <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css" /> + </HeadContent> + <div class="container">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarOptionDemo.razor` at line 389, Move the inline <link rel="stylesheet"> out of the component body and place it inside a Blazor <HeadContent> block in _BitNavBarOptionDemo.razor so the Font Awesome stylesheet is injected into the document head (ensuring correct behavior for SSR/WASM/hybrid and avoiding duplicates); locate the existing <link href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css" /> in the component markup and wrap it with <HeadContent>...</HeadContent> (or add a single shared HeadContent section if one already exists) so the stylesheet is added to the head instead of rendered into the body.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/MenuItem.cs (1)
6-7: Minor naming inconsistency:ImagevsIconforBitIconInfo?.The library uses
Icon(BitIconInfo?) inBitNavBarItemandBitNavBarOption, but the demo model calls the equivalent propertyImage. While demo-only, this divergence could confuse readers comparing theMenuItem-based custom demo against the standard API. Consider renamingImage→IconandImageName→IconNameto stay aligned with library naming, if there are no naming conflicts with the NameSelectors configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/MenuItem.cs` around lines 6 - 7, The demo model MenuItem has inconsistent property names Image and ImageName for a BitIconInfo?, which diverges from the library usage of Icon/IconName in BitNavBarItem and BitNavBarOption; rename Image → Icon and ImageName → IconName in the MenuItem class and update any references/usages to those properties (including serializers, bindings, and NameSelectors if present) to maintain API naming consistency and avoid conflicts with existing NameSelectors configuration.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarOptionDemo.razor.samples.cs (1)
301-301: Optional: Consider updating the pinned FontAwesome version to the latest.Font Awesome 7.2.0 has been released (February 10) and is the current latest version, whereas the demo pins to
7.0.1. The URL is valid, but pinning to the latest avoids users copying the CDN snippet and getting an outdated release.♻️ Suggested update
-<link rel=""stylesheet"" href=""https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css"" /> +<link rel=""stylesheet"" href=""https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.2.0/css/all.min.css"" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarOptionDemo.razor.samples.cs` at line 301, Update the pinned Font Awesome CDN URL in the Bit NavBar demo sample: find the link element string "<link rel=\"stylesheet\" href=\"https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css\" />" in _BitNavBarOptionDemo.razor.samples.cs and change the version segment from 7.0.1 to the current latest (e.g., 7.2.0) so the href becomes "https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.2.0/css/all.min.css"; ensure the string is updated wherever that exact snippet is used in the file.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarItemDemo.razor (1)
241-241: Use<HeadContent>to inject the stylesheet into<head>Placing a
<link rel="stylesheet">directly inside a Blazor component's render tree (i.e., in the<body>) is non-standard and can cause issues in interactive render modes: each re-render of the parent may re-insert the<link>node into the DOM, leading to duplicate stylesheet downloads. Blazor's<HeadContent>component handles deduplication and proper<head>injection.♻️ Proposed fix
<DemoExample Title="External Icons" RazorCode="@example12RazorCode" CsharpCode="@example12CsharpCode" Id="example12"> <div>External icon libraries like FontAwesome can be used with the <b>Icon</b> parameter.</div> <br /> <br /> - <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css" /> + <HeadContent> + <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.2.0/css/all.min.css" + integrity="sha512-<hash-here>" crossorigin="anonymous" referrerpolicy="no-referrer" /> + </HeadContent> <div class="container">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarItemDemo.razor` at line 241, The stylesheet link currently rendered inside the component’s body in _BitNavBarItemDemo.razor should be moved into a Blazor <HeadContent> block to avoid duplicate <link> injections on re-renders; locate the inline <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css" /> in the BitNavBarItemDemo component and replace it by adding the same href inside a <HeadContent> section so the stylesheet is injected into the document <head> and deduplicated by Blazor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 307-327: GetIcon returns BitIconInfo? but the fallback calls
GetValueFromProperty<string?> causing a type mismatch; update the fallback call
inside GetIcon to use GetValueFromProperty<BitIconInfo?> so it retrieves the
correct type when NameSelectors.Icon.Selector is null—locate the GetIcon method
and replace the generic parameter on the NameSelectors.Icon fallback call to
BitIconInfo? (references: GetIcon, NameSelectors.Icon, GetValueFromProperty).
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarNameSelectors.cs`:
- Around line 15-19: The GetIcon fallback in the BitNavBar partial class uses
the wrong generic for GetValueFromProperty causing a type mismatch: update the
GetValueFromProperty call in BitNavBar.razor.cs (the GetIcon implementation) to
use BitIconInfo? (matching BitNavBarNameSelectors.Icon's generic type) instead
of string? so the name-based lookup returns the correct BitIconInfo? when no
Selector is provided; ensure the call mirrors the selector's declared value type
and compiles against the BitNavBarNameSelectors.Icon definition.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarCustomDemo.razor.samples.cs`:
- Around line 517-540: The sample in example13CsharpCode defines a MenuItem
class with the wrong property name/type; change the MenuItem property "public
BitIconInfo? Icon { get; set; }" to "public string? ImageName { get; set; }" so
it matches the sample instances (basicNavBarCustoms and
basicNavBarCustomsClassStyle) that set ImageName values; ensure the property
name and type align with the actual MenuItem used by the demo.
- Around line 447-466: The Razor sample's NameSelectors uses the wrong key:
change ImageName = { Selector = item => item.Image } to Icon = { Selector = item
=> item.Image } inside the example12RazorCode so it matches the BitIconInfo
usage in example12CsharpCode and the real demo (_BitNavBarCustomDemo.razor),
ensuring external icons bind via the Icon selector rather than ImageName.
- Around line 509-516: Replace the incorrect property name ItemImageName with
the correct ItemIcon on the BitNavBar component instances in the sample; update
both the Styles and Classes anonymous objects (where Styles uses ItemImageName
and Classes uses ItemImageName) to use ItemIcon so they match the
BitNavBarClassStyles API and the NameSelectors usage remains unchanged.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarItemDemo.razor`:
- Line 241: Update the external Font Awesome <link> in _BitNavBarItemDemo.razor
to use the current 7.2.0 CDN URL and add Subresource Integrity and crossorigin
attributes: replace the href version from 7.0.1 to 7.2.0 and add
integrity="sha384-<calculated_hash>" and crossorigin="anonymous"; generate the
SHA-384 hash by fetching the 7.2.0 CSS (e.g., curl -sL
https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.2.0/css/all.min.css |
openssl dgst -sha384 -binary | openssl base64 -A) and prefixing the result with
"sha384-".
---
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarItemDemo.razor`:
- Line 241: The stylesheet link currently rendered inside the component’s body
in _BitNavBarItemDemo.razor should be moved into a Blazor <HeadContent> block to
avoid duplicate <link> injections on re-renders; locate the inline <link
rel="stylesheet"
href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css"
/> in the BitNavBarItemDemo component and replace it by adding the same href
inside a <HeadContent> section so the stylesheet is injected into the document
<head> and deduplicated by Blazor.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarOptionDemo.razor`:
- Line 389: Move the inline <link rel="stylesheet"> out of the component body
and place it inside a Blazor <HeadContent> block in _BitNavBarOptionDemo.razor
so the Font Awesome stylesheet is injected into the document head (ensuring
correct behavior for SSR/WASM/hybrid and avoiding duplicates); locate the
existing <link
href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css"
/> in the component markup and wrap it with <HeadContent>...</HeadContent> (or
add a single shared HeadContent section if one already exists) so the stylesheet
is added to the head instead of rendered into the body.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/_BitNavBarOptionDemo.razor.samples.cs`:
- Line 301: Update the pinned Font Awesome CDN URL in the Bit NavBar demo
sample: find the link element string "<link rel=\"stylesheet\"
href=\"https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.0.1/css/all.min.css\"
/>" in _BitNavBarOptionDemo.razor.samples.cs and change the version segment from
7.0.1 to the current latest (e.g., 7.2.0) so the href becomes
"https://cdnjs.cloudflare.com/ajax/libs/font-awesome/7.2.0/css/all.min.css";
ensure the string is updated wherever that exact snippet is used in the file.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Navs/NavBar/MenuItem.cs`:
- Around line 6-7: The demo model MenuItem has inconsistent property names Image
and ImageName for a BitIconInfo?, which diverges from the library usage of
Icon/IconName in BitNavBarItem and BitNavBarOption; rename Image → Icon and
ImageName → IconName in the MenuItem class and update any references/usages to
those properties (including serializers, bindings, and NameSelectors if present)
to maintain API naming consistency and avoid conflicts with existing
NameSelectors configuration.
closes #12111
Summary by CodeRabbit
Release Notes