Improve Iconography page (#12576)#12577
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe Iconography page now provides searchable icon selection, a details panel with previews and metadata, glyph loading, reusable copy rows, clipboard feedback, accessibility labels, styling, and updated CSS output mapping. ChangesIcon details panel feature
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant IconographyPage
participant IconDetailRow
participant HttpClient
participant JSInterop
User->>IconographyPage: select icon
IconographyPage->>HttpClient: load icon CSS glyph mappings
HttpClient-->>IconographyPage: return glyph data
IconographyPage-->>User: display details and previews
User->>IconDetailRow: click copy
IconDetailRow->>IconographyPage: invoke OnCopy
IconographyPage->>JSInterop: copy detail text
JSInterop-->>IconographyPage: complete clipboard action
IconographyPage-->>IconDetailRow: update copied feedback
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.scss`:
- Around line 17-26: The IconDetailRow.razor.scss rule for
.icon-detail-row-value uses multi-word font-family names without quotes, which
Stylelint flags. Update the font-family declaration in the
.icon-detail-row-value block so any family names containing spaces are quoted,
while keeping the same fallback stack and monospace intent intact.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs`:
- Around line 9-18: The RazorIconInfo property in IconInfo is producing
copy-paste snippets with invalid Razor attribute quoting because the nested
quotes around BitIconInfo.Bit(Value) are not escaped for the outer double-quoted
attribute. Update RazorIconInfo so the generated string is valid Razor syntax
when pasted into a .razor file, and verify the resulting snippet still matches
the style used by ConstantReference, CssClass, and RazorIconName.
- Around line 92-102: The icon panel is clearing selectedIcon too early, which
can blank the panel body before BitPanel finishes its close animation. Update
CloseIconPanel and its call path from HandleIconPanelDismissed so selectedIcon
is only cleared after the panel has fully dismissed, while still setting
isIconPanelOpen to false immediately. Use the existing IconographyPage component
state flow to defer the content reset until after the close transition
completes.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss`:
- Around line 14-18: The .icon-name rule uses the deprecated word-break:
break-word value, so update the IconographyPage.razor.scss styles to use a
current text-wrapping approach instead. Locate the .icon-name selector and
replace the deprecated declaration with the appropriate modern CSS
property/value combination that preserves label wrapping behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 21b8f43a-f81e-497b-8090-1a455232d70b
📒 Files selected for processing (6)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor (1)
12-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate stale
PageOutletdescription to reflect the new details panel behavior.The description still says "copy any icon name with a click," but the click now opens an icon details panel. The in-page note (lines 44–45) was updated; this should be too.
✏️ Proposed fix
- Description="Browse and search the full set of Fabric (MDL2) icons available in bit BlazorUI, and copy any icon name with a click." /> + Description="Browse and search the full set of Fabric (MDL2) icons available in bit BlazorUI. Click any icon to view its details and copy usage snippets." />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor` around lines 12 - 14, Update the Description attribute of the IconographyPage PageOutlet to describe that clicking an icon opens its details panel, replacing the stale claim that users can copy the icon name with a click.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs`:
- Around line 42-47: Remove the unused cascadingParams field from the
IconographyPage code-behind; the Razor markup already references
_cascadingParams defined in its `@code` block, so do not alter the existing markup
or that field.
- Around line 111-118: The GetGlyphCode method currently formats the literal
backslash from CSS-escaped glyph values instead of the actual codepoint. Decode
or remove the leading backslash from the glyph string before calling
char.ConvertToUtf32, then format the resulting codepoint with the existing
escape prefix and hexadecimal formatting.
---
Outside diff comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor`:
- Around line 12-14: Update the Description attribute of the IconographyPage
PageOutlet to describe that clicking an icon opens its details panel, replacing
the stale claim that users can copy the icon name with a click.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a12bad27-7c83-4281-8a74-ea0d515995de
📒 Files selected for processing (6)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs (1)
111-118: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
GetGlyphCodeproduces incorrect glyph codes — it formats the CSS backslash, not the codepoint.CSS escape values captured by the regex (e.g.,
\E498) start with a literal backslash.char.ConvertToUtf32(glyph, 0)reads that backslash character (U+005C = 92), so the output becomes\005Cinstead of\E498. This was flagged in a prior review and remains unaddressed.🐛 Proposed fix
private string? GetGlyphCode(IconInfo icon) { if (iconGlyphs is null) return null; if (iconGlyphs.TryGetValue(icon.Value, out var glyph) is false || string.IsNullOrEmpty(glyph)) return null; - return $"\\{char.ConvertToUtf32(glyph, 0):X4}"; + var hex = glyph.TrimStart('\\'); + return int.TryParse(hex, System.Globalization.NumberStyles.HexNumber, null, out var code) + ? $"\\{code:X4}" + : glyph; }#!/bin/bash # Verify the CSS glyph format to confirm the backslash issue. scss=$(fd -t f "bit.blazorui.icons.scss" src/ 2>/dev/null | head -1) if [ -n "$scss" ]; then echo "== Sample content declarations from SCSS source ==" rg -n 'content:' "$scss" | head -10 echo "" echo "== Checking for backslash-escape sequences ==" rg -qP 'content:\s*"\\' "$scss" && echo "CSS uses \\HEX escape sequences (confirms bug)" || echo "No backslash escapes found" else echo "SCSS source not found; searching for compiled CSS..." fd -t f "bit.blazorui.icons.css" 2>/dev/null | head -5 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs` around lines 111 - 118, The GetGlyphCode method incorrectly converts the leading CSS backslash instead of the glyph codepoint. Update GetGlyphCode to strip the leading backslash from glyph before parsing or returning the escape, then preserve the actual hexadecimal codepoint (such as E498) with a single CSS backslash prefix; retain the existing null and empty-value handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs`:
- Around line 111-118: The GetGlyphCode method incorrectly converts the leading
CSS backslash instead of the glyph codepoint. Update GetGlyphCode to strip the
leading backslash from glyph before parsing or returning the escape, then
preserve the actual hexadecimal codepoint (such as E498) with a single CSS
backslash prefix; retain the existing null and empty-value handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a12bad27-7c83-4281-8a74-ea0d515995de
📒 Files selected for processing (6)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor (1)
14-14: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the stale
PageOutletdescription.The description still says "copy any icon name with a click," but clicking an icon now opens a details panel. Update to reflect the new behavior.
✏️ Proposed fix
- Description="Browse and search the full set of Fabric (MDL2) icons available in bit BlazorUI, and copy any icon name with a click." /> + Description="Browse and search the full set of Fabric (MDL2) icons available in bit BlazorUI. Click an icon to preview sizes, colors, and copy usage snippets." />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor` at line 14, Update the PageOutlet Description in IconographyPage to state that clicking an icon opens its details panel instead of claiming that it copies the icon name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor`:
- Line 14: Update the PageOutlet Description in IconographyPage to state that
clicking an icon opens its details panel instead of claiming that it copies the
icon name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0d8f6049-59e0-4e9b-91d5-245e1925b2dc
📒 Files selected for processing (6)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
msynk
changed the title
Iconography page (#12576)
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs (1)
10-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
RazorIconInfostill generates invalid Razor/HTML attribute syntax — unescaped nested quotes.
$"Icon=\"@BitIconInfo.Bit(\"{Value}\")\""decodes toIcon="@BitIconInfo.Bit("+ Value +")", e.g.Icon="@BitIconInfo.Bit("Accept")"— the inner"Accept"breaks out of the outer double-quoted attribute. This exact pattern was flagged in a prior review round (with a marked resolution), but the current code still matches the pre-fix version, not the proposed fix. It's also consumed verbatim as the "BitIconInfo" usage snippet inIconographyPage.razor(~lines 224-226), so the copy-to-clipboard snippet users get for this row is broken syntax — directly undermining this PR's goal of providing working, copyable usage snippets.🐛 Proposed fix — use single quotes as the outer delimiter
- public string RazorIconInfo => $"Icon=\"`@BitIconInfo.Bit`(\"{Value}\")\""; + public string RazorIconInfo => $"Icon='`@BitIconInfo.Bit`(\"{Value}\")'";Verification of the decoded output (no repo access needed):
#!/bin/bash python3 - <<'EOF' value = "Accept" current = 'Icon="`@BitIconInfo.Bit`("' + value + '")"' fixed = "Icon='`@BitIconInfo.Bit`(\"" + value + "\")'" print("Current (buggy):", current) print("Fixed: ", fixed) EOF🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs` around lines 10 - 19, Update IconInfo.RazorIconInfo so the generated Razor attribute uses single quotes as the outer delimiter, preserving the inner double quotes around Value and producing valid copyable BitIconInfo syntax. Leave the other IconInfo properties unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor`:
- Around line 67-77: Update the icon tile interactive element in the
IconographyPage loop around OpenIconPanel to support keyboard activation,
preferably by replacing BitStack with BitButton while preserving its layout,
styling, selection state, and click behavior; ensure Enter and Space can open
the icon panel.
---
Duplicate comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs`:
- Around line 10-19: Update IconInfo.RazorIconInfo so the generated Razor
attribute uses single quotes as the outer delimiter, preserving the inner double
quotes around Value and producing valid copyable BitIconInfo syntax. Leave the
other IconInfo properties unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5d66bc36-d6c6-40bf-8091-9d114ec18f68
📒 Files selected for processing (5)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs (1)
16-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
RazorIconInfostill generates invalid Razor syntax — the previously-flagged fix appears to have regressed.
RazorIconInfointerpolates asIcon="@BitIconInfo.Bit("Value")": the attribute is delimited by", and the embedded"Value"breaks out of it mid-attribute. This is byte-for-byte the same defect flagged in an earlier review round (marked "✅ Addressed"), but the code shown here doesn't contain the proposed single-quote fix. Given the PR's goal is letting users copy working usage snippets, this directly undermines that for theBitIconInfoexample (rendered viaCodeBoxinIconographyPage.razoraround lines 227-229).Please confirm whether the earlier fix was reverted (e.g., during a rebase) and re-apply it.
🐛 Proposed fix
- public string RazorIconInfo => $"Icon=\"`@BitIconInfo.Bit`(\"{Value}\")\""; + public string RazorIconInfo => $"Icon='`@BitIconInfo.Bit`(\"{Value}\")'";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs` around lines 16 - 19, Update the RazorIconInfo property to generate valid Razor attribute syntax by using single quotes around the embedded BitIconInfo.Bit argument, matching the previously intended fix. Preserve the existing BitIconInfo.Bit call and output structure so copied CodeBox snippets remain functional.
🧹 Nitpick comments (3)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cs (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMark required parameters with
[EditorRequired].
Label,Value,CopyKey, andOnCopyare effectively mandatory (defaulted with= default!/ no-opEventCallback), but nothing enforces this at compile/dev time. A future caller that forgetsCopyKeyorOnCopygets a silently broken copy button instead of an analyzer warning.♻️ Proposed fix
- [Parameter] public string Label { get; set; } = default!; + [Parameter, EditorRequired] public string Label { get; set; } = default!; - [Parameter] public string Value { get; set; } = default!; + [Parameter, EditorRequired] public string Value { get; set; } = default!; - [Parameter] public string CopyKey { get; set; } = default!; + [Parameter, EditorRequired] public string CopyKey { get; set; } = default!; [Parameter] public string? CopyFeedbackKey { get; set; } - [Parameter] public EventCallback<(string Text, string Key)> OnCopy { get; set; } + [Parameter, EditorRequired] public EventCallback<(string Text, string Key)> OnCopy { get; set; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cs` around lines 5 - 13, Mark the mandatory parameters Label, Value, CopyKey, and OnCopy in IconDetailRow with the Blazor [EditorRequired] attribute so callers receive analyzer warnings when they omit them. Leave the optional nullable CopyFeedbackKey parameter unchanged.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs (1)
135-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the
"name"copy-feedback key into a shared constant.
"name"is hardcoded here and re-checked as a literal twice inIconographyPage.razor(the hero "Copy name" button'sIconName/label conditions). A typo or rename in either place silently breaks the "Copied" indicator with no compile-time signal.♻️ Proposed fix
+ private const string NameCopyKey = "name"; + - private Task CopyIconName() => CopyToClipboard(selectedIcon!.Value, "name"); + private Task CopyIconName() => CopyToClipboard(selectedIcon!.Value, NameCopyKey);And in
IconographyPage.razor, referenceNameCopyKeyinstead of the literal"name"in the twocopyFeedbackKey == "name"checks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs` around lines 135 - 152, Introduce a shared NameCopyKey constant in the IconographyPage code-behind and use it in CopyIconName instead of the hardcoded "name" value. Update both hero "Copy name" feedback conditions in IconographyPage.razor to reference NameCopyKey, preserving the existing feedback behavior.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor (1)
65-79: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider adding
@keyto the icon tile loop.The keyboard-activation issue from the earlier review is resolved by switching to
BitButton. Separately, this list can have hundreds of items and now carries selection state (selectedclass); adding@key="icon.Value"helps Blazor's diff correctly track/reuse tiles across search-filter and selection changes instead of a positional diff.♻️ Proposed fix
foreach (var icon in filteredIcons) { <BitButton Variant="BitVariant.Text" + `@key`="icon.Value" Title="`@icon.Value`"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor` around lines 65 - 79, Add `@key`="icon.Value" to the icon tile element within the filteredIcons foreach loop in IconographyPage, using the icon's stable value to preserve tile identity across filtering and selection updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor`:
- Around line 13-18: Add an AriaLabel attribute to the icon-only BitButton in
IconDetailRow, using the same conditional “Copied”/“Copy” label as Title. Keep
the existing icon, title, and HandleCopy behavior unchanged.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss`:
- Around line 24-41: Update the interactive state selectors in the iconography
styles to include &:focus-visible so keyboard-focused icons reveal .icon-name
with opacity: 1. Consolidate &:active with the existing &:hover and &.selected
shared color and background styles, while keeping the selected-only outline rule
unchanged.
---
Duplicate comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs`:
- Around line 16-19: Update the RazorIconInfo property to generate valid Razor
attribute syntax by using single quotes around the embedded BitIconInfo.Bit
argument, matching the previously intended fix. Preserve the existing
BitIconInfo.Bit call and output structure so copied CodeBox snippets remain
functional.
---
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cs`:
- Around line 5-13: Mark the mandatory parameters Label, Value, CopyKey, and
OnCopy in IconDetailRow with the Blazor [EditorRequired] attribute so callers
receive analyzer warnings when they omit them. Leave the optional nullable
CopyFeedbackKey parameter unchanged.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor`:
- Around line 65-79: Add `@key`="icon.Value" to the icon tile element within the
filteredIcons foreach loop in IconographyPage, using the icon's stable value to
preserve tile identity across filtering and selection updates.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs`:
- Around line 135-152: Introduce a shared NameCopyKey constant in the
IconographyPage code-behind and use it in CopyIconName instead of the hardcoded
"name" value. Update both hero "Copy name" feedback conditions in
IconographyPage.razor to reference NameCopyKey, preserving the existing feedback
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ca554e86-5764-4334-bdf7-6ff5948fde72
📒 Files selected for processing (5)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs (1)
10-19: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
RazorIconInfostill emits invalid Razor syntax — prior fix not present in this version.
$"Icon=\"@BitIconInfo.Bit(\"{Value}\")\""renders asIcon="@BitIconInfo.Bit("Accept")"— the inner quotes around the argument aren't escaped/delimited differently from the outer attribute quotes, so pasting this into an actual.razorfile breaks attribute parsing. This exact issue was flagged in a prior review and marked as addressed, but the code shown here still has the unescaped nesting (likely lost when this file moved intoPages/Iconography/). It also feeds the "BitIconInfo" usage snippet rendered inIconographyPage.razor(@($"<BitButton {selectedIcon.RazorIconInfo} />")), so the copy‑paste feature this PR is meant to deliver remains broken.🐛 Proposed fix
- public string RazorIconInfo => $"Icon=\"`@BitIconInfo.Bit`(\"{Value}\")\""; + public string RazorIconInfo => $"Icon='`@BitIconInfo.Bit`(\"{Value}\")'";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs` around lines 10 - 19, Update the IconInfo.RazorIconInfo property to emit valid Razor attribute syntax by using a differently delimited or properly escaped string for the BitIconInfo.Bit argument, so the generated snippet can be pasted into a .razor file without conflicting nested quotes. Preserve the existing BitIconInfo.Bit(Value) expression and the surrounding Icon attribute format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs`:
- Around line 79-84: Reset copyFeedbackKey at the start of OpenIconPanel when
switching icons, before updating selectedIcon or opening the panel, so any
active copy feedback from the previous icon cannot appear for the newly selected
icon.
---
Duplicate comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs`:
- Around line 10-19: Update the IconInfo.RazorIconInfo property to emit valid
Razor attribute syntax by using a differently delimited or properly escaped
string for the BitIconInfo.Bit argument, so the generated snippet can be pasted
into a .razor file without conflicting nested quotes. Preserve the existing
BitIconInfo.Bit(Value) expression and the surrounding Icon attribute format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1518e8f6-39ae-4428-bcdb-18b63b70e961
📒 Files selected for processing (9)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (3)
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs (1)
18-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
RazorIconInfostill produces invalid Razor attribute syntax (unresolved).
$"Icon=\"@BitIconInfo.Bit(\"{Value}\")\""evaluates toIcon="@BitIconInfo.Bit("Accept")"— the inner double quotes aroundValuearen't escaped for the outer double-quoted attribute, so pasting this generated snippet into a.razorfile breaks attribute parsing. This is the same defect flagged on a prior commit (noted "Addressed"), but the current code still exhibits it — the fix appears not to have landed here.🐛 Proposed fix
- public string RazorIconInfo => $"Icon=\"`@BitIconInfo.Bit`(\"{Value}\")\""; + public string RazorIconInfo => $"Icon='`@BitIconInfo.Bit`(\"{Value}\")'";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs` at line 18, Update the RazorIconInfo property to generate valid Razor attribute syntax by escaping or otherwise preserving the inner quotes around Value within the outer Icon attribute quotes. Verify the resulting string can be pasted into a .razor file and produces a valid BitIconInfo.Bit invocation.
🧹 Nitpick comments (2)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconDetailRow.razor.cs (1)
5-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
[EditorRequired]on mandatory parameters.
Label,Value, andCopyKeyare effectively required (defaulted todefault!) but aren't annotated, so misuse only surfaces at runtime instead of via IDE/analyzer warnings.♻️ Proposed fix
- [Parameter] public string Label { get; set; } = default!; + [Parameter, EditorRequired] public string Label { get; set; } = default!; - [Parameter] public string Value { get; set; } = default!; + [Parameter, EditorRequired] public string Value { get; set; } = default!; - [Parameter] public string CopyKey { get; set; } = default!; + [Parameter, EditorRequired] public string CopyKey { get; set; } = default!;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconDetailRow.razor.cs` around lines 5 - 13, Annotate the mandatory component parameters Label, Value, and CopyKey in IconDetailRow with [EditorRequired] so consumers receive design-time warnings when they omit them; leave the optional CopyFeedbackKey and OnCopy parameters unchanged.src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs (1)
136-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCopy-key strings are magic literals shared across two files.
"name","constant","css","glyph","name-row"are duplicated as bare strings between this file andIconographyPage.razor. A typo in either place would silently break the copy-feedback match (copyFeedbackKey == feedbackKey) with no compiler warning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs` around lines 136 - 153, The copy-feedback keys used by CopyIconName, HandleDetailCopy, and CopyToClipboard are duplicated magic strings across the code-behind and markup. Define shared named constants for "name", "constant", "css", "glyph", and "name-row" in the IconographyPage component, then replace every matching literal in both IconographyPage.razor.cs and IconographyPage.razor with those constants so feedback matching remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs`:
- Line 18: Update the RazorIconInfo property to generate valid Razor attribute
syntax by escaping or otherwise preserving the inner quotes around Value within
the outer Icon attribute quotes. Verify the resulting string can be pasted into
a .razor file and produces a valid BitIconInfo.Bit invocation.
---
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconDetailRow.razor.cs`:
- Around line 5-13: Annotate the mandatory component parameters Label, Value,
and CopyKey in IconDetailRow with [EditorRequired] so consumers receive
design-time warnings when they omit them; leave the optional CopyFeedbackKey and
OnCopy parameters unchanged.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cs`:
- Around line 136-153: The copy-feedback keys used by CopyIconName,
HandleDetailCopy, and CopyToClipboard are duplicated magic strings across the
code-behind and markup. Define shared named constants for "name", "constant",
"css", "glyph", and "name-row" in the IconographyPage component, then replace
every matching literal in both IconographyPage.razor.cs and
IconographyPage.razor with those constants so feedback matching remains
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 494d9ed6-d810-4e79-8256-b9a98075c49c
📒 Files selected for processing (9)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconDetailRow.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconDetailRow.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Iconography/IconographyPage.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/compilerconfig.json
💤 Files with no reviewable changes (3)
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.scss
- src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/IconographyPage.razor.cs
2 participants
Close #12576
Summary by CodeRabbit
/iconographydemo with a searchable, selectable icon grid.