Add new Icon parameter to BitAccordion (#12232)#12233
Add new Icon parameter to BitAccordion (#12232)#12233msynk merged 6 commits intobitfoundation:developfrom
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:
WalkthroughBitAccordion gained customizable expander icon support via new parameters ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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.
Pull request overview
Adds configurable expander icon support to BitAccordion so consumers can use either Fluent UI icon names or external icon libraries via BitIconInfo.
Changes:
- Added
ExpanderIcon(BitIconInfo?) andExpanderIconName(string?) parameters and updated rendering to useBitIconInfo.From(...)with aChevronRightdefault. - Renamed the expander icon styling hook from
ChevronDownIcontoExpanderIcon(including the related CSS selector rename). - Updated the Accordion demo to document and showcase the new parameters.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.razor.cs | Introduces the new expander icon parameters on the component API. |
| src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.razor | Uses BitIconInfo to render the expander icon with fallback behavior. |
| src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordionClassStyles.cs | Renames the public class/styles hook for the expander icon. |
| src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.scss | Renames the internal expander icon CSS class selector. |
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor | Adds a new demo example for ExpanderIcon*. |
| src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor.cs | Updates demo metadata/docs and adds the sample snippet string. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor.cs (1)
62-75: LGTM with a minor observation.The parameter metadata is comprehensive and correctly documents the precedence and default behavior.
One small inconsistency: Line 74 refers to "built-in Fluent UI icons" while
BitIconInfo.csdocuments these as "bit BlazorUI icons". Consider aligning terminology for consistency.📝 Optional terminology alignment
new() { Name = "ExpanderIconName", Type = "string?", DefaultValue = "null", - Description = "Gets or sets the name of the icon to display as expander from the built-in Fluent UI icons. Defaults to ChevronRight if not set." + Description = "Gets or sets the name of the icon to display as expander from the built-in bit BlazorUI icons. Defaults to ChevronRight if not set." },🤖 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/Surfaces/Accordion/BitAccordionDemo.razor.cs` around lines 62 - 75, Update the Description text for the ExpanderIconName parameter in BitAccordionDemo.razor.cs so terminology matches the rest of the project (use the same term used in BitIconInfo.cs, e.g., "Bit BlazorUI icons" instead of "built-in Fluent UI icons"); locate the ExpanderIcon and ExpanderIconName entries and change only the descriptive string for ExpanderIconName to the consistent phrase while keeping the precedence and default behavior wording intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor.cs`:
- Around line 62-75: Update the Description text for the ExpanderIconName
parameter in BitAccordionDemo.razor.cs so terminology matches the rest of the
project (use the same term used in BitIconInfo.cs, e.g., "Bit BlazorUI icons"
instead of "built-in Fluent UI icons"); locate the ExpanderIcon and
ExpanderIconName entries and change only the descriptive string for
ExpanderIconName to the consistent phrase while keeping the precedence and
default behavior wording intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 35ade3ee-d7c3-4473-9242-22d6d214a2c5
📒 Files selected for processing (6)
src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.razorsrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.scsssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordionClassStyles.cssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor.cs
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor (1)
244-245: Load external icon styles once at app/page head instead of inside demo body.Injecting CDN
<link>tags directly in example body can lead to duplicate loads/reflows when the demo rerenders. Prefer central head-level inclusion for these assets.Also applies to: 272-273
🤖 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/Surfaces/Accordion/BitAccordionDemo.razor` around lines 244 - 245, The demo page BitAccordionDemo.razor currently injects an external Font Awesome <link> tag in its markup (the CDN link at lines shown in the diff); remove that <link> tag(s) from the BitAccordionDemo.razor demo body (and any other demos that repeat it, e.g., the instances around 272-273) and instead include the stylesheet once at the application shell/head (for Blazor WASM use index.html, for Blazor Server use _Host.cshtml or the shared layout like MainLayout) so the icon CSS is loaded centrally and not re-inserted on component rerenders.
🤖 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/Surfaces/Accordion/BitAccordion.razor`:
- Line 40: In BitAccordion.razor fix the missing space between class tokens so
the expanded icon class is properly applied: update the class attribute where
expanderIcon?.GetCssClasses(), the literal "bit-acd-eic" and the conditional
(IsExpanded ? Classes?.ExpandedIcon : "") are concatenated to ensure a delimiter
(space) is inserted before Classes?.ExpandedIcon when IsExpanded is true (while
preserving existing Classes?.ExpanderIcon), e.g., add a leading space before the
expanded class in the interpolation so IsExpanded yields " bit-acd-eic
<expanded-class>" instead of merging names.
In `@src/BlazorUI/Bit.BlazorUI/Styles/general.scss`:
- Line 1: The SCSS import in general.scss uses a file extension which violates
the scss/load-partial-extension rule; update the import in general.scss to omit
the “.scss” extension (e.g., change the `@import` "functions.scss" statement to
`@import` "functions") so the partial (_functions.scss) is imported without its
extension and the stylelint rule is satisfied.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/DemoExample.razor.scss`:
- Around line 1-3: In DemoExample.razor.scss replace the three `@import`
references that include leading underscores and “.scss” (the imports for
_functions.scss, _media-queries.scss, and _bit-css-variables.scss) with imports
that omit the partial leading underscore and the .scss extension (i.e., import
the partial names functions, media-queries, and bit-css-variables instead) so
they comply with the Stylelint rules scss/load-no-partial-leading-underscore and
scss/load-partial-extension.
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor.cs`:
- Line 216: The demo uses a misspelled sub-parameter name "ExpanderIconWapper";
update the Name assignment in BitAccordionDemo (where Name =
"ExpanderIconWapper") to the correct public API name "ExpanderIconWrapper" so
the demo matches the component's property (ExpanderIconWrapper) and the docs
remain consistent.
---
Nitpick comments:
In
`@src/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor`:
- Around line 244-245: The demo page BitAccordionDemo.razor currently injects an
external Font Awesome <link> tag in its markup (the CDN link at lines shown in
the diff); remove that <link> tag(s) from the BitAccordionDemo.razor demo body
(and any other demos that repeat it, e.g., the instances around 272-273) and
instead include the stylesheet once at the application shell/head (for Blazor
WASM use index.html, for Blazor Server use _Host.cshtml or the shared layout
like MainLayout) so the icon CSS is loaded centrally and not re-inserted on
component rerenders.
🪄 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: 10e80735-9d11-4a7b-a0db-d445df73cdd3
📒 Files selected for processing (9)
src/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.razorsrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordion.scsssrc/BlazorUI/Bit.BlazorUI/Components/Surfaces/Accordion/BitAccordionClassStyles.cssrc/BlazorUI/Bit.BlazorUI/Styles/general.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Components/DemoExample.razor.scsssrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razorsrc/BlazorUI/Demo/Client/Bit.BlazorUI.Demo.Client.Core/Pages/Components/Surfaces/Accordion/BitAccordionDemo.razor.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Surfaces/Accordion/BitAccordionTests.cs
closes #12232
Summary by CodeRabbit