Fix item order issues in multi-api components (#12653)#12659
Merged
msynk merged 23 commits intoJul 17, 2026
Conversation
|
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:
WalkthroughSeveral Blazor components now delegate option rendering to dedicated item components, refresh registered options after state changes, preserve conditional option order, and support DOM-based ordering for Breadcrumb and ChoiceGroup. New bUnit tests cover ordering, reordering, and selection behavior. ChangesOption rendering and state refresh
Estimated code review effort: 4 (Complex) | ~75 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ParentComponent
participant OptionComponent
participant ItemComponent
participant DOM
ParentComponent->>OptionComponent: register option
ParentComponent->>OptionComponent: refresh option state
OptionComponent->>ItemComponent: render item with parent and option
ItemComponent->>DOM: emit ordered item markup
ParentComponent->>DOM: read option marker order
DOM-->>ParentComponent: return marker identifiers
ParentComponent->>ParentComponent: reconcile registered item order
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 |
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor (1)
38-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPreserve state and focus with a
@keyattribute.When rendering a list of Blazor components dynamically, it is highly recommended to bind a
@keyattribute. This helps Blazor's diffing algorithm preserve DOM elements, component internal state, and user focus when list items are added, removed, or reordered.♻️ Proposed fix
`@foreach` (var item in _items) { - <_BitChoiceGroupItem Item="item" ChoiceGroup="this" /> + <_BitChoiceGroupItem `@key`="item" Item="item" ChoiceGroup="this" /> }🤖 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/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor` around lines 38 - 41, Update the foreach rendering in BitChoiceGroup to add a stable `@key` for each _BitChoiceGroupItem, using the item’s unique identity, while preserving the existing Item and ChoiceGroup parameters.
🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs`:
- Around line 150-155: Guard the Parent.RegisterOption call in
OnInitializedAsync with a null check, matching BuildRenderTree’s behavior when
no BitButtonGroup parent exists. Update Dispose to call Parent.UnregisterOption
only when Parent is not null, while preserving the existing disposal state
handling.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 211-222: The internal selection update paths do not refresh navbar
options, leaving active styling stale. Add RefreshOptions() after
SetSelectedItemByCurrentUrl() in OnLocationChanged, and after
OnSelectItem.InvokeAsync(item) in SetSelectedItem, before StateHasChanged(),
while preserving the existing selection guards and assignment flow.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor`:
- Around line 38-41: Update the foreach rendering in BitChoiceGroup to add a
stable `@key` for each _BitChoiceGroupItem, using the item’s unique identity,
while preserving the existing Item and ChoiceGroup parameters.
🪄 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: d957eb77-e5ec-486f-be74-324e0b55dd3d
📒 Files selected for processing (64)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs (1)
224-229: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCall
RefreshOptions()when the selection is updated internally.The internal selection update paths do not explicitly refresh the navbar options, leaving the active styling stale. When a user clicks an item or the URL changes in automatic mode, the selection state updates internally, but the child option components are not re-rendered (similar to how
BitNavhandles it).
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs#L224-L229: Add a call toRefreshOptions();right beforeStateHasChanged();inOnLocationChanged.src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs#L257-L266: Add a call toRefreshOptions();right beforeStateHasChanged();inSetSelectedItem.🛠️ Proposed fixes
private void OnLocationChanged(object? sender, LocationChangedEventArgs args) { SetSelectedItemByCurrentUrl(); + RefreshOptions(); StateHasChanged(); }private async Task SetSelectedItem(TItem item) { if (item == SelectedItem && Reselectable is false) return; if (await AssignSelectedItem(item) is false) return; await OnSelectItem.InvokeAsync(item); + RefreshOptions(); StateHasChanged(); }🤖 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/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs` around lines 224 - 229, Refresh navbar option components after internal selection updates: in src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs lines 224-229, add RefreshOptions() before StateHasChanged() in OnLocationChanged; make the same change in SetSelectedItem at lines 257-266.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
339-342: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCheck
Optionsas an alias forChildContent.The
Optionsparameter is documented as an alias forChildContent. If a user providesOptionsinstead ofChildContent,ChildContentwill benull. Consequently, this logic will bypass the early return and overwrite the_itemscollection withItems, discarding any options registered via the<BitMenuButtonOption>elements.🐛 Proposed fix
- if (ChildContent is not null || Items.Any() is false || Items == _oldItems) return; + if (ChildContent is not null || Options is not null || Items.Any() is false || Items == _oldItems) return; _oldItems = Items;🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs` around lines 339 - 342, Update the items-refresh guard in the relevant menu-button lifecycle method to treat the documented Options parameter as an alias for ChildContent. When either ChildContent or Options is provided, return before replacing _items; preserve the existing empty-Items and unchanged-reference checks.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cs (1)
272-283: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove
asyncfromHandleChangeto resolve compiler warning.The
HandleChangemethod is marked asasync Taskbut contains noawaitexpressions, which will cause a CS1998 compiler warning. Because the method performs only synchronous state updates, consider changing the return type tovoidor explicitly returningTask.CompletedTask.♻️ Proposed refactor to use
void- internal async Task HandleChange(TItem item) + internal void HandleChange(TItem item) { if (IsEnabled is false || ReadOnly || GetIsEnabled(item) is false) return;🤖 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/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cs` around lines 272 - 283, Update HandleChange to remove the unnecessary async modifier and use a synchronous return type, preserving its existing state-update logic and call behavior.
🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs`:
- Around line 78-83: Guard the Parent calls in
BitMenuButtonOption.OnInitializedAsync and Dispose with null-conditional
invocation: update RegisterOption and UnregisterOption so both lifecycle methods
safely handle a missing Parent. Apply this in
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs
at lines 78-83 and 103-110.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 570-576: Update ShouldRenderOptionItem to avoid calling
GetSearchedItems for every option. When SearchFunction is not provided, evaluate
the current item directly against _searchText using the existing item-text
comparison logic; retain the SearchFunction path for custom filtering and
preserve the no-search behavior.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs`:
- Around line 95-100: Guard nullable Parent access in
BitDropdownOption.OnInitializedAsync by registering with
Parent?.RegisterOption(this), and apply the same null-safe access in Dispose
using Parent?.UnregisterOption(this). Update both affected locations in
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs (lines
95-100 and 126-133).
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Line 21: The class attribute in _BitTimelineItem should separate the base
bit-tln-itm class from the classes returned by Timeline.GetItemClasses(Item).
Add a literal space before that interpolation while preserving the existing
Timeline.Classes?.Item concatenation.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs`:
- Around line 339-342: Update the items-refresh guard in the relevant
menu-button lifecycle method to treat the documented Options parameter as an
alias for ChildContent. When either ChildContent or Options is provided, return
before replacing _items; preserve the existing empty-Items and
unchanged-reference checks.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 224-229: Refresh navbar option components after internal selection
updates: in src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs
lines 224-229, add RefreshOptions() before StateHasChanged() in
OnLocationChanged; make the same change in SetSelectedItem at lines 257-266.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cs`:
- Around line 272-283: Update HandleChange to remove the unnecessary async
modifier and use a synchronous return type, preserving its existing state-update
logic and call 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: 1494a02e-8424-4729-8773-f6830c453d1c
📒 Files selected for processing (64)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 14 minutes. |
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/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs (1)
224-229: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCall
RefreshOptions()when the selection is updated internally.
RefreshOptions()correctly ensures that option components re-render when the navbar state changes. However, becauseOnSetSelectedItemis only triggered when theSelectedItemparameter is updated by a parent component (via the[CallOnSet]attribute), internal selection updates bypass this refresh mechanism.When a user clicks an item (triggering
SetSelectedItem) or the URL changes in automatic mode (triggeringOnLocationChanged), the selection state updates internally but the option components are not explicitly refreshed. As a result, the active/selected CSS class will fail to update on the rendered UI items.Apply the following changes to ensure internal state updates also propagate to the options (similar to how
BitNav.razor.cshandles it):🐛 Proposed fix
private void OnLocationChanged(object? sender, LocationChangedEventArgs args) { SetSelectedItemByCurrentUrl(); + RefreshOptions(); StateHasChanged(); }private async Task SetSelectedItem(TItem item) { if (item == SelectedItem && Reselectable is false) return; if (await AssignSelectedItem(item) is false) return; await OnSelectItem.InvokeAsync(item); + RefreshOptions(); StateHasChanged(); }Also applies to: 257-266
🤖 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/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs` around lines 224 - 229, Update the internal selection flows in BitNavBar, specifically SetSelectedItem and OnLocationChanged, to call RefreshOptions() after SetSelectedItemByCurrentUrl or other selection state updates. Preserve the existing StateHasChanged behavior and ensure both user-triggered selection and automatic URL-driven selection refresh the option components.
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor (1)
21-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a space before the
@(Timeline.GetItemClasses(Item))call.Without a space, the base class
bit-tln-itmwill concatenate directly with the first class returned byGetItemClasses(e.g., resulting inbit-tln-itmcustom-class), breaking both the base styling and the applied class.🐛 Proposed fix
- class="bit-tln-itm@(Timeline.GetItemClasses(Item)) `@Timeline.Classes`?.Item"> + class="bit-tln-itm @(Timeline.GetItemClasses(Item)) `@Timeline.Classes`?.Item">🤖 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/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor` at line 21, Update the class attribute in _BitTimelineItem to include a separating space between the base class “bit-tln-itm” and the output of Timeline.GetItemClasses(Item), preserving the existing Timeline.Classes?.Item expression.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor (1)
17-22: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd
@keyto the looped items to prevent rendering bugs.In Blazor, looping over components without a
@keycan lead to state leakage or DOM patching bugs when items are added, removed, or reordered. Consider adding a@key(e.g.,@key="item") to_BitButtonGroupItem, similar to how it was done safely inBitChoiceGroup.razor.⚡ Proposed fix
else { `@foreach` (var item in _items) { - <_BitButtonGroupItem ButtonGroup="this" Item="item" /> + <_BitButtonGroupItem `@key`="item" ButtonGroup="this" Item="item" /> }🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor` around lines 17 - 22, Update the foreach rendering of _BitButtonGroupItem in the ButtonGroup component to include a stable `@key` based on item, preserving correct component state and DOM updates when _items changes.
🤖 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/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 1051-1052: Remove RefreshOptions() from OnAfterRenderAsync, then
invoke it synchronously at the end of OnParametersSetAsync and immediately after
internal state mutations in SearchItems, ClearSearchBox, ClearComboBoxInput,
AddOrRemoveSelectedItem, UpdateSelectedItemsFromValues, and HandleOnClearClick,
following the established BitChoiceGroup pattern.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs`:
- Around line 122-133: The child components receive stable parameter references
and do not refresh when parent-driven state changes. In
src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs lines
122-133, update BuildRenderTree and its _BitTimelineItem usage to provide a
changing parameter or explicit refresh trigger; in
src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor lines 5-21,
apply the equivalent change to _BitNavChild so childItems.Any() is reevaluated
after nested options register and the display wrapper updates correctly.
---
Outside diff comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 224-229: Update the internal selection flows in BitNavBar,
specifically SetSelectedItem and OnLocationChanged, to call RefreshOptions()
after SetSelectedItemByCurrentUrl or other selection state updates. Preserve the
existing StateHasChanged behavior and ensure both user-triggered selection and
automatic URL-driven selection refresh the option components.
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Line 21: Update the class attribute in _BitTimelineItem to include a
separating space between the base class “bit-tln-itm” and the output of
Timeline.GetItemClasses(Item), preserving the existing Timeline.Classes?.Item
expression.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor`:
- Around line 17-22: Update the foreach rendering of _BitButtonGroupItem in the
ButtonGroup component to include a stable `@key` based on item, preserving correct
component state and DOM updates when _items changes.
🪄 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: c6aa8ff5-80eb-460b-ac21-d96453e0b6b5
📒 Files selected for processing (64)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs (1)
216-229: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCall
RefreshOptions()when the selection is updated internally.
RefreshOptions()correctly ensures that option components re-render when the navbar state changes. However, becauseOnSetSelectedItemis only triggered when theSelectedItemparameter is updated by a parent component (via the[CallOnSet]attribute), internal selection updates bypass this refresh mechanism.When a user clicks an item (triggering
SetSelectedItem) or the URL changes in automatic mode (triggeringOnLocationChanged), the selection state updates internally but the option components are not explicitly refreshed. As a result, the active/selected CSS class will fail to update on the rendered UI items.Apply the following changes to ensure internal state updates also propagate to the options (similar to how
BitNav.razor.cshandles it).🐛 Proposed fix for internal selection updates
private void OnLocationChanged(object? sender, LocationChangedEventArgs args) { SetSelectedItemByCurrentUrl(); + RefreshOptions(); StateHasChanged(); }Also, apply a similar change to
SetSelectedItemfurther down in the file:private async Task SetSelectedItem(TItem item) { if (item == SelectedItem && Reselectable is false) return; if (await AssignSelectedItem(item) is false) return; await OnSelectItem.InvokeAsync(item); RefreshOptions(); // <-- Add this StateHasChanged(); }🤖 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/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs` around lines 216 - 229, Update the internal selection flows to refresh navbar options after state changes: call RefreshOptions() in SetSelectedItem after OnSelectItem.InvokeAsync succeeds and before StateHasChanged, and call it in OnLocationChanged after SetSelectedItemByCurrentUrl(). Preserve the existing parent-driven OnSetSelectedItem behavior and event ordering.src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs (1)
1051-1052: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftMove
RefreshOptions()out ofOnAfterRenderAsyncto avoid UI flicker and double rendering.Calling
StateHasChanged()(viaRefreshOptions()) on child components from withinOnAfterRenderAsyncforces them to re-render after the browser has already painted the parent component's updates. This causes an unnecessary double-render cycle on every parent render and will introduce a visible one-frame UI flicker when search text or selections change.Remove this call from
OnAfterRenderAsync. Instead, invokeRefreshOptions()synchronously at the end ofOnParametersSetAsync()and exactly at the sites where the internal state mutates (such as insideSearchItems(),ClearSearchBox(),ClearComboBoxInput(),AddOrRemoveSelectedItem(),UpdateSelectedItemsFromValues(), andHandleOnClearClick()). This matches the pattern safely implemented inBitChoiceGroup.razor.cs.🤖 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/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs` around lines 1051 - 1052, Remove RefreshOptions() from OnAfterRenderAsync, then invoke it synchronously at the end of OnParametersSetAsync and immediately after internal state changes in SearchItems, ClearSearchBox, ClearComboBoxInput, AddOrRemoveSelectedItem, UpdateSelectedItemsFromValues, and HandleOnClearClick, following the established BitChoiceGroup pattern.
🤖 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/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs`:
- Around line 122-133: Make the child item components refreshable by passing a
changing state parameter to _BitChoiceGroupItem in BitChoiceGroupOption.cs
(lines 122-133) and to _BitTimelineItem in BitTimelineOption.cs (lines 122-133).
Add or reuse a StateId value incremented by InternalStateHasChanged(), and pass
it alongside the existing Parent and Item parameters so each parent-driven state
update bypasses parameter equality checks.
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 1051-1052: Remove RefreshOptions() from OnAfterRenderAsync, then
invoke it synchronously at the end of OnParametersSetAsync and immediately after
internal state changes in SearchItems, ClearSearchBox, ClearComboBoxInput,
AddOrRemoveSelectedItem, UpdateSelectedItemsFromValues, and HandleOnClearClick,
following the established BitChoiceGroup pattern.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 216-229: Update the internal selection flows to refresh navbar
options after state changes: call RefreshOptions() in SetSelectedItem after
OnSelectItem.InvokeAsync succeeds and before StateHasChanged, and call it in
OnLocationChanged after SetSelectedItemByCurrentUrl(). Preserve the existing
parent-driven OnSetSelectedItem behavior and event ordering.
🪄 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: 2edcbc57-cddd-43ac-abde-9a00a9304a4b
📒 Files selected for processing (64)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ 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/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs (1)
67-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnsure option rendering updates when
SingleExpandis false.When
SingleExpandis false,ToggleItemupdates the item's expansion state but skips callingRefreshOptions()andStateHasChanged(). As a result, the UI will not re-render the nested option children to reflect the toggled expansion state. Move the refresh calls outside theif (SingleExpand)block so that both modes correctly trigger a re-render.💡 Proposed fix
- RefreshOptions(); - StateHasChanged(); - _currentItem = Item; } else { SetItemExpanded(Item, isExpanded); } + RefreshOptions(); + StateHasChanged(); + await OnItemToggle.InvokeAsync(Item);🤖 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/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs` around lines 67 - 78, Update ToggleItem so RefreshOptions() and StateHasChanged() execute after both the SingleExpand and non-SingleExpand branches, ensuring expansion changes re-render nested options in either mode. Keep the existing _currentItem assignment limited to the SingleExpand path.
♻️ Duplicate comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs (1)
122-130: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the child item components refreshable.
The child components receive stable parameter references and will not refresh when parent-driven state changes.
_BitChoiceGroupItemand_BitTimelineItemreceive the sameItem="this"and parent references on each render. WhenInternalStateHasChanged()is called, Blazor's parameter diffing sees that the references haven't changed and skips re-rendering the inner component.To force a refresh, provide a changing parameter (such as a
StateIdinteger that increments inInternalStateHasChanged) or trigger an explicit refresh method on the inner component.
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs#L122-L130: pass a changing parameter (e.g.,StateId) to bypass equality checks for_BitChoiceGroupItem.src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs#L124-L132: pass a changing parameter to bypass equality checks for_BitTimelineItem.🤖 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/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs` around lines 122 - 130, Child item components do not refresh when parent state changes because their parameter references remain stable. In BitChoiceGroupOption.cs lines 122-130, add and pass a changing StateId (or equivalent refresh trigger) to _BitChoiceGroupItem, and ensure InternalStateHasChanged increments it; apply the same change to _BitTimelineItem in BitTimelineOption.cs lines 124-132. Preserve existing ChoiceGroup and Item parameters while ensuring each internal state update changes the refresh parameter.src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor (1)
21-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a space before the
@(Timeline.GetItemClasses(Item))call.Without a space, the base class
bit-tln-itmwill concatenate directly with the first class returned byGetItemClasses(e.g., resulting inbit-tln-itmcustom-class), breaking both the base styling and the applied class.🐛 Proposed fix
- class="bit-tln-itm@(Timeline.GetItemClasses(Item)) `@Timeline.Classes`?.Item"> + class="bit-tln-itm @(Timeline.GetItemClasses(Item)) `@Timeline.Classes`?.Item">🤖 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/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor` at line 21, Add a separating space after the base class `bit-tln-itm` in the class attribute of `_BitTimelineItem`, before the `Timeline.GetItemClasses(Item)` output, so the base and dynamic classes remain distinct.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs (1)
330-339: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsolidate redundant
RefreshOptions()calls.The
OnSetSelectedItemmethod callsRefreshOptions()twice. SinceToggleItemAndParentsonly updates the internal dictionary state without yielding or blocking, a singleRefreshOptions()call at the end is sufficient to apply both the new selection state and the updated expansion states to the rendered options.♻️ Proposed refactor
private void OnSetSelectedItem() { - // The selection affects the previously and newly selected items, which render themselves in - // the options mode, so push a re-render to all of them. - RefreshOptions(); - - if (SelectedItem is null) return; - - ToggleItemAndParents(_items, SelectedItem, true); + if (SelectedItem is not null) + { + ToggleItemAndParents(_items, SelectedItem, true); + } + // The selection affects the previously and newly selected items, which render themselves in + // the options mode, so push a re-render to all of them. RefreshOptions(); }🤖 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/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs` around lines 330 - 339, Remove the first RefreshOptions() call in OnSetSelectedItem and retain a single call after ToggleItemAndParents. Preserve the existing null-selection early return and ensure the remaining refresh applies both selection and expansion-state 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/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor`:
- Around line 18-21: Update the foreach loop rendering _BitTimelineItem in the
Timeline component to include `@key`="item", preserving stable component mapping
and state when _items changes.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cs`:
- Around line 246-266: The breadcrumb update flow can render after disposal. In
BitBreadcrumb.razor.cs, add an IsDisposed guard in UnregisterOptions (lines
148-155) and check IsDisposed after the JS await in OnAfterRenderAsync (lines
246-266), before ReorderOptions executes; leave the existing exception handling
and ordering behavior unchanged.
---
Outside diff comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs`:
- Around line 67-78: Update ToggleItem so RefreshOptions() and StateHasChanged()
execute after both the SingleExpand and non-SingleExpand branches, ensuring
expansion changes re-render nested options in either mode. Keep the existing
_currentItem assignment limited to the SingleExpand path.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs`:
- Around line 122-130: Child item components do not refresh when parent state
changes because their parameter references remain stable. In
BitChoiceGroupOption.cs lines 122-130, add and pass a changing StateId (or
equivalent refresh trigger) to _BitChoiceGroupItem, and ensure
InternalStateHasChanged increments it; apply the same change to _BitTimelineItem
in BitTimelineOption.cs lines 124-132. Preserve existing ChoiceGroup and Item
parameters while ensuring each internal state update changes the refresh
parameter.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Line 21: Add a separating space after the base class `bit-tln-itm` in the
class attribute of `_BitTimelineItem`, before the
`Timeline.GetItemClasses(Item)` output, so the base and dynamic classes remain
distinct.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs`:
- Around line 330-339: Remove the first RefreshOptions() call in
OnSetSelectedItem and retain a single call after ToggleItemAndParents. Preserve
the existing null-selection early return and ensure the remaining refresh
applies both selection and expansion-state updates.
🪄 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: a9ff4aef-05c6-4ca4-b6da-3e7b7890a59d
📒 Files selected for processing (64)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
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/Navs/NavBar/BitNavBar.razor.cs (1)
224-229: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCall
RefreshOptions()when the selection is updated internally.Because
OnSetSelectedItemis only triggered when theSelectedItemparameter is updated by a parent component, internal selection updates bypass the refresh mechanism. This leaves the active styling stale on the rendered UI items. CallRefreshOptions()after updating the selection state to ensure option components re-render.
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs#L224-L229: AddRefreshOptions();afterSetSelectedItemByCurrentUrl();inOnLocationChanged.src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs#L257-L266: AddRefreshOptions();afterawait OnSelectItem.InvokeAsync(item);inSetSelectedItem.🐛 Proposed fixes
Update
OnLocationChanged:private void OnLocationChanged(object? sender, LocationChangedEventArgs args) { SetSelectedItemByCurrentUrl(); + RefreshOptions(); StateHasChanged(); }Update
SetSelectedItem:private async Task SetSelectedItem(TItem item) { if (item == SelectedItem && Reselectable is false) return; if (await AssignSelectedItem(item) is false) return; await OnSelectItem.InvokeAsync(item); + RefreshOptions(); StateHasChanged(); }🤖 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/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs` around lines 224 - 229, Internal selection updates in BitNavBar bypass option refreshes, leaving active styling stale. In src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs lines 224-229, update OnLocationChanged to call RefreshOptions() after SetSelectedItemByCurrentUrl(); at lines 257-266, update SetSelectedItem to call RefreshOptions() after await OnSelectItem.InvokeAsync(item).
♻️ Duplicate comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs (1)
122-130: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMake the delegated inner item components refreshable.
The option and item components receive stable object references (
this,Parent, oritem) as parameters. When parent state changes and explicitly triggersInternalStateHasChanged()on the options (e.g., viaRefreshOptions()), Blazor's parameter diffing (ReferenceEquals) sees unchanged parameters and skips re-rendering the inner item components (_BitChoiceGroupItem, etc.). Consequently, parent-driven updates (like selection changes or disable toggles) will fail to reflect in the DOM.
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs#L122-L130: Add a changing state parameter (e.g., aStateIdinteger that increments inInternalStateHasChanged) to_BitChoiceGroupItemor trigger an explicit refresh method to bypass Blazor's parameter equality skip.src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs#L124-L132: Pass a changing state parameter to_BitTimelineItemto ensure it refreshes on parent state changes.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs#L87-L95: Pass a changing state parameter to_BitMenuButtonItemto ensure it refreshes on parent state changes.src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor#L38-L41: Pass the same changing state parameter to_BitChoiceGroupItemin theItemsloop to ensure it re-renders when parent parameters change.🤖 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/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs` around lines 122 - 130, The delegated item components currently receive unchanged references, so parent-triggered refreshes skip their renders. Add and maintain a changing state parameter (such as a StateId incremented by InternalStateHasChanged), then pass it to _BitChoiceGroupItem in src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs lines 122-130 and BitChoiceGroup.razor lines 38-41, to _BitTimelineItem in src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs lines 124-132, and to _BitMenuButtonItem in src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs lines 87-95, ensuring all parent-driven state changes re-render the inner items.src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs (1)
1054-1055: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftMove
RefreshOptions()out ofOnAfterRenderAsyncto avoid UI flicker and double rendering.Calling
StateHasChanged()(viaRefreshOptions()) on child components from withinOnAfterRenderAsyncforces them to re-render after the browser has already painted the parent component's updates. This causes an unnecessary double-render cycle on every parent render and will introduce a visible one-frame UI flicker when search text or selections change.Remove this call from
OnAfterRenderAsync. Instead, invokeRefreshOptions()synchronously at the end ofOnParametersSetAsync()and exactly at the sites where the internal state mutates (such as insideSearchItems(),ClearSearchBox(),ClearComboBoxInput(),AddOrRemoveSelectedItem(),UpdateSelectedItemsFromValues(), andHandleOnClearClick()). This matches the pattern safely implemented inBitChoiceGroup.razor.csand correctly applied in this PR withinBitTimeline.razor.cs.🤖 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/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs` around lines 1054 - 1055, The RefreshOptions call in OnAfterRenderAsync causes an unnecessary post-paint render; remove it there and invoke RefreshOptions synchronously at the end of OnParametersSetAsync and at each internal state mutation site, including SearchItems, ClearSearchBox, ClearComboBoxInput, AddOrRemoveSelectedItem, UpdateSelectedItemsFromValues, and HandleOnClearClick.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor (1)
19-22: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePrefer using a stable key property instead of the item reference.
For better rendering stability and consistency with other components (like
BitAccordionList), consider usingGetItemKey(item)as the loop key instead of the item reference itself. Using a consistent primitive key prevents unnecessary element recreation if object references change during updates.♻️ Proposed refactor
`@foreach` (var item in _items) { - <_BitButtonGroupItem `@key`="item" ButtonGroup="this" Item="item" /> + <_BitButtonGroupItem `@key`="GetItemKey(item)" ButtonGroup="this" Item="item" /> }🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor` around lines 19 - 22, Update the foreach rendering in BitButtonGroup to use GetItemKey(item) for the `@key` value instead of the item reference. Preserve the existing ButtonGroup and Item bindings while ensuring the key is a stable primitive consistent with other components.
🤖 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/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs`:
- Around line 28-29: Update the public CollapseAll and ExpandAll methods in
BitNav so each calls StateHasChanged() immediately after RefreshOptions(); apply
the change at both specified locations in
src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs (lines 28-29 and
40-41).
---
Outside diff comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 224-229: Internal selection updates in BitNavBar bypass option
refreshes, leaving active styling stale. In
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs lines
224-229, update OnLocationChanged to call RefreshOptions() after
SetSelectedItemByCurrentUrl(); at lines 257-266, update SetSelectedItem to call
RefreshOptions() after await OnSelectItem.InvokeAsync(item).
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs`:
- Around line 122-130: The delegated item components currently receive unchanged
references, so parent-triggered refreshes skip their renders. Add and maintain a
changing state parameter (such as a StateId incremented by
InternalStateHasChanged), then pass it to _BitChoiceGroupItem in
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs
lines 122-130 and BitChoiceGroup.razor lines 38-41, to _BitTimelineItem in
src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs lines
124-132, and to _BitMenuButtonItem in
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs
lines 87-95, ensuring all parent-driven state changes re-render the inner items.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 1054-1055: The RefreshOptions call in OnAfterRenderAsync causes an
unnecessary post-paint render; remove it there and invoke RefreshOptions
synchronously at the end of OnParametersSetAsync and at each internal state
mutation site, including SearchItems, ClearSearchBox, ClearComboBoxInput,
AddOrRemoveSelectedItem, UpdateSelectedItemsFromValues, and HandleOnClearClick.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor`:
- Around line 19-22: Update the foreach rendering in BitButtonGroup to use
GetItemKey(item) for the `@key` value instead of the item reference. Preserve the
existing ButtonGroup and Item bindings while ensuring the key is a stable
primitive consistent with other components.
🪄 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: 8d9f3b39-5a7b-4f46-a095-b6623feac125
📒 Files selected for processing (64)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
Member
Author
|
@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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs (1)
311-324: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the early return to ensure toggle state updates for custom items.
When
NameSelectorsis null, the method returns early and completely skipsawait UpdateItemToggle(item);. As a result, clicking an item will not update the toggle state when using custom objects withoutNameSelectors.Consider nesting the selector logic inside a null check instead of returning early from the method.
🐛 Proposed fix to prevent early return
else { - if (NameSelectors is null) return; - - if (NameSelectors.OnClick.Selector is not null) - { - NameSelectors.OnClick.Selector!(item)?.Invoke(item); - } - else - { - item.GetValueFromProperty<Action<TItem>?>(NameSelectors.OnClick.Name)?.Invoke(item); - } + if (NameSelectors is not null) + { + if (NameSelectors.OnClick.Selector is not null) + { + NameSelectors.OnClick.Selector!(item)?.Invoke(item); + } + else + { + item.GetValueFromProperty<Action<TItem>?>(NameSelectors.OnClick.Name)?.Invoke(item); + } + } }🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs` around lines 311 - 324, Update the NameSelectors handling in the button click method so a null NameSelectors skips only the selector callback logic rather than returning from the method. Preserve both selector branches when NameSelectors is present, and ensure await UpdateItemToggle(item) still executes for custom items without selectors.
♻️ Duplicate comments (3)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs (1)
85-96: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winPass a changing parameter to trigger child component re-renders.
Although the parent components now invoke
RefreshOptions()andInternalStateHasChanged(), Blazor's parameter diffing will still skip re-rendering the inner_BitMenuButtonItemand_BitAccordionListItemcomponents. BecauseMenuButton/AccordionListandItemare reference types that do not change object references across state updates, Blazor assumes the child parameters are unchanged and halts render propagation. This breaks UI updates (e.g., expanding an item, updating selection) for bothItemsandOptionsmodes. A shared primitiveStateIdparameter is required to force Blazor to re-render these items.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs#L85-L96: Pass a changingStateIdparameter (e.g.,Parent._stateId) when building the_BitMenuButtonItemcomponent.src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs#L99-L110: Pass a changingStateIdparameter (e.g.,Parent._stateId) when building the_BitAccordionListItemcomponent.src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor#L16-L21: PassStateId="_stateId"in the@foreachloop to ensure items re-render.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cs#L3-L8: Add[Parameter] public int StateId { get; set; }so the item can receive the changing state parameter (do the same for_BitAccordionListItem.razor.cs).🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs` around lines 85 - 96, Pass the changing primitive StateId through all menu-button and accordion-list item render paths: use Parent._stateId in BitMenuButtonOption and BitAccordionListOption, pass StateId="_stateId" in BitAccordionList.razor, and add a public [Parameter] int StateId property to both _BitMenuButtonItem and _BitAccordionListItem components.src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs (1)
122-133: 🎯 Functional Correctness | 🟠 MajorMake the child item components refreshable
_BitTimelineItemreceives the sameItem="this"reference on each render, so timeline-driven updates can stay stale unless the child gets a changing parameter or an explicit refresh trigger.🤖 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/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs` around lines 122 - 133, Update BuildRenderTree in BitTimelineOption so each rendered _BitTimelineItem receives a changing refresh parameter or explicit refresh trigger, rather than relying only on the stable Item="this" reference. Reuse the existing timeline/item update mechanism if available and preserve the current Parent null guard and markup-order rendering behavior.src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor (1)
24-24: 🎯 Functional Correctness | 🟡 MinorAdd a space before the
@(Timeline.GetItemClasses(Item))call.Without a space, the base class
bit-tln-itmwill concatenate directly with the first class returned byGetItemClasses(e.g., resulting inbit-tln-itmcustom-class), breaking both the base styling and the applied class.🐛 Proposed fix
- class="bit-tln-itm@(Timeline.GetItemClasses(Item)) `@Timeline.Classes`?.Item"> + class="bit-tln-itm @(Timeline.GetItemClasses(Item)) `@Timeline.Classes`?.Item">🤖 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/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor` at line 24, Update the class attribute in the Timeline item markup around Timeline.GetItemClasses(Item) to insert a space after the base class bit-tln-itm, ensuring returned classes remain separate CSS class tokens while preserving the existing Timeline.Classes?.Item classes.
🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs`:
- Around line 311-324: Update the NameSelectors handling in the button click
method so a null NameSelectors skips only the selector callback logic rather
than returning from the method. Preserve both selector branches when
NameSelectors is present, and ensure await UpdateItemToggle(item) still executes
for custom items without selectors.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs`:
- Around line 85-96: Pass the changing primitive StateId through all menu-button
and accordion-list item render paths: use Parent._stateId in BitMenuButtonOption
and BitAccordionListOption, pass StateId="_stateId" in BitAccordionList.razor,
and add a public [Parameter] int StateId property to both _BitMenuButtonItem and
_BitAccordionListItem components.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Line 24: Update the class attribute in the Timeline item markup around
Timeline.GetItemClasses(Item) to insert a space after the base class
bit-tln-itm, ensuring returned classes remain separate CSS class tokens while
preserving the existing Timeline.Classes?.Item classes.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs`:
- Around line 122-133: Update BuildRenderTree in BitTimelineOption so each
rendered _BitTimelineItem receives a changing refresh parameter or explicit
refresh trigger, rather than relying only on the stable Item="this" reference.
Reuse the existing timeline/item update mechanism if available and preserve the
current Parent null guard and markup-order rendering behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: af702a54-d7c6-486f-8b9a-6fcf8c785f72
📒 Files selected for processing (64)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
💤 Files with no reviewable changes (1)
- src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 16 minutes. |
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs (1)
85-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the child item components refreshable.
The child item components receive stable parameter references and will not automatically refresh when parent-driven state changes. When
InternalStateHasChanged()triggers a re-render on the option component, Blazor's parameter diffing sees that theParentandItemreferences haven't changed and skips re-rendering the inner component. To force a refresh, provide a changing parameter (such as aStateIdinteger that increments wheneverInternalStateHasChangedis called) to the child component.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs#L85-L96: Pass a changing parameter to bypass equality checks for_BitMenuButtonItem.src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs#L99-L110: Pass a changing parameter to bypass equality checks for_BitAccordionListItem.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs#L157-L168: Pass a changing parameter to bypass equality checks for_BitButtonGroupItem.🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs` around lines 85 - 96, Make the child item components refreshable by adding a changing StateId-style parameter that increments whenever InternalStateHasChanged is called, then pass it alongside the stable Parent and Item parameters. Apply this to _BitMenuButtonItem in src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs lines 85-96, _BitAccordionListItem in src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs lines 99-110, and _BitButtonGroupItem in src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs lines 157-168.
🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor`:
- Around line 19-23: Update the `@key` on _BitButtonGroupItem in the
BitButtonGroup render loop to use a stable identifier derived from item instead
of the index-based $"{UniqueId}-{i}" value. Reuse the existing GetItemKey(item,
fallback) mechanism if available, passing the index string only as its fallback,
or use another stable item property while preserving the current rendering
behavior.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor`:
- Around line 17-23: Update the virtualized option registration flow around
RegisterOption and the Virtualize branch so conditionally rendered options
retain their declared ChildContent/Options order rather than being appended to
the end of Items. Add marker-based reconciliation or position-aware registration
for hidden options, and reconcile Items before refreshing the virtualized list.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Around line 16-22: The _BitTimelineItem event bindings currently invoke
callbacks even when the parent Timeline is disabled because disabled is
ineffective on a div. Gate both the `@onclick` and `@onkeydown` handlers using the
combined isEnabled value, while preserving the existing enabled behavior and
keyboard default-prevention handling.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cs`:
- Around line 155-159: Update the Items change-detection logic in the
BitTimeline component so it compares the current collection contents against a
stored snapshot rather than relying on reference inequality. Preserve the
existing conditions and refresh _items whenever Items is mutated in place, then
store an independent snapshot in _oldItems after copying.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cs`:
- Around line 163-188: Synchronize registered navigation collections with
declaration/render order before automatic URL matching: in
BitNavOption.razor.cs, update root _items and nested ChildItems around
OnInitializedAsync so both reflect declared option order before flattening; in
BitNavBar.razor.cs, align _items with rendered option order before its
FirstOrDefault URL match. Preserve existing registration and selection behavior
after ordering is corrected.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs`:
- Around line 85-96: Make the child item components refreshable by adding a
changing StateId-style parameter that increments whenever
InternalStateHasChanged is called, then pass it alongside the stable Parent and
Item parameters. Apply this to _BitMenuButtonItem in
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs
lines 85-96, _BitAccordionListItem in
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs
lines 99-110, and _BitButtonGroupItem in
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs
lines 157-168.
🪄 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: d5da0182-e2b0-4345-bff6-4732012d4146
📒 Files selected for processing (71)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/_BitChoiceGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticSelectionTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
💤 Files with no reviewable changes (1)
- src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs (1)
85-95: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBlazor skips re-rendering delegated item components due to stable references.
Both the Options API and Items API now delegate rendering to
_BitMenuButtonItemand_BitButtonGroupItem. However, because Blazor compares reference parameters by object identity, it will skip re-rendering these child components when the parent forces an update (e.g., duringRefreshOptions()or item toggling) because theMenuButton/ButtonGroupandItemreferences haven't changed. This breaks dynamic UI updates like styling changes, enabled state, or selection updates.To fix this, add a primitive parameter (e.g.,
[Parameter] public int StateId { get; set; }) to the child item components, and pass an incrementing integer from the parent whenever a refresh is needed:
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs#L85-L95: pass an incrementing state parameter (e.g.,_stateId) to_BitMenuButtonItemto force re-rendering.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs#L157-L167: pass an incrementing state parameter to_BitButtonGroupItem.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor#L107-L111: pass an incrementing state parameter to_BitMenuButtonItemfrom the parent's loop in the Items API.🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs` around lines 85 - 95, Blazor skips delegated item re-renders because their reference parameters remain unchanged. Add an incrementing primitive state parameter to _BitMenuButtonItem and _BitButtonGroupItem, update it whenever the respective parent refreshes, and pass it through the delegated renders: update BitMenuButtonOption.cs lines 85-95, BitButtonGroupOption.cs lines 157-167, and the Items API loop in BitMenuButton.razor lines 107-111 so each refresh supplies the current state value.
🤖 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/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cs`:
- Around line 149-157: Update BitChoiceGroup.UnregisterOption to return
immediately when IsDisposed is true, before removing the option or calling
SetIndexItems and StateHasChanged. Match the existing disposal guard pattern
used by BitDropdown.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Around line 16-18: The timeline item currently relies on the previously
rendered _preventKeyDownDefault value, so HandleOnItemKeyDown cannot prevent the
current Space key event. Update _BitTimelineItem.razor and HandleOnItemKeyDown
in _BitTimelineItem.razor.cs to use a native button with type="button", or
implement synchronous key-based prevention via JavaScript/capture-phase
handling; preserve the existing item-click behavior.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cs`:
- Around line 155-161: Update the synchronization logic around _oldItems and
Items.Any() so a transition from populated Items to an empty collection clears
_items. Ensure empty Items still trigger comparison and assign an empty
snapshot, while preserving the existing update behavior for non-empty
collections.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs`:
- Around line 85-95: Blazor skips delegated item re-renders because their
reference parameters remain unchanged. Add an incrementing primitive state
parameter to _BitMenuButtonItem and _BitButtonGroupItem, update it whenever the
respective parent refreshes, and pass it through the delegated renders: update
BitMenuButtonOption.cs lines 85-95, BitButtonGroupOption.cs lines 157-167, and
the Items API loop in BitMenuButton.razor lines 107-111 so each refresh supplies
the current state value.
🪄 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: f4d349e0-0ea8-4b67-abd7-2da67bea4e1c
📒 Files selected for processing (71)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/_BitChoiceGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticSelectionTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
💤 Files with no reviewable changes (1)
- src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs (1)
263-269: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRecalculate selection when
Itemschanges in automatic mode.When the
Itemsparameter is updated (for example, when items are loaded asynchronously),_itemsis refreshed but the selection is not recalculated against the current URL. This leaves the newly active item visually unselected until the URL changes again.Call
SetSelectedItemByCurrentUrl()when the items collection changes.💡 Proposed Fix
private void OnSetParameters() { if (ChildContent is not null || Options is not null || Items == _oldItems) return; _items = Items?.ToList() ?? []; _oldItems = Items; + + SetSelectedItemByCurrentUrl(); }🤖 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/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs` around lines 263 - 269, Update OnSetParameters so that after refreshing _items and _oldItems when Items changes, it calls SetSelectedItemByCurrentUrl() to recalculate the active selection against the current URL in automatic mode.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs (3)
159-163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear the toggle state when the toggled option is unregistered.
When a toggled
BitButtonGroupOptionis removed (e.g., conditionally removed from the render tree), its boundToggleKeyshould be updated and the internal_toggleItemreference cleared so it doesn't leave behind a dangling reference or stale state. This aligns with theUnregisterOptionbehavior found inBitAccordionList.💡 Proposed fix
- internal void UnregisterOption(BitButtonGroupOption option) - { - _items.Remove((option as TItem)!); - StateHasChanged(); - } + internal void UnregisterOption(BitButtonGroupOption option) + { + var item = (option as TItem)!; + _items.Remove(item); + + if (Toggle && _toggleItem == item) + { + _ = UpdateItemToggle(null); + } + + StateHasChanged(); + }🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs` around lines 159 - 163, Update UnregisterOption to detect when the removed option is the currently toggled item, clear the bound ToggleKey, and set the internal _toggleItem reference to null before calling StateHasChanged. Preserve the existing removal behavior for options that are not toggled.
221-246: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent initialization of
_itemsfromItemswhenOptionsorChildContentis used.
_itemsis unconditionally initialized fromItems. If a developer provides bothItemsandOptions,_itemswill initially containItems, and then the registered options will be appended to it. Subsequent updates toItemsare correctly ignored inOnParametersSetAsync, but the initial render will erroneously mix both APIs.To ensure consistent behavior, only populate
_itemsfromItemswhenOptionsandChildContentare null, matching the logic inBitAccordionList.💡 Proposed fix
protected override async Task OnInitializedAsync() { - _items = Items is not null ? [.. Items] : []; - - if (Toggle && Items is not null && Items.Any()) + _items = (ChildContent is null && Options is null && Items is not null) ? [.. Items] : []; + + if (Toggle && _items.Any()) { var toggleKey = string.Empty; if (ToggleKeyHasBeenSet) { toggleKey = ToggleKey; _internalToggleKey = ToggleKey; } else if (DefaultToggleKey.HasValue()) { toggleKey = DefaultToggleKey; } if (toggleKey.HasValue()) { - var item = Items.FirstOrDefault(i => GetItemKey(i) == toggleKey); + var item = _items.FirstOrDefault(i => GetItemKey(i) == toggleKey); await UpdateItemToggle(item, false); } } await base.OnInitializedAsync(); }🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs` around lines 221 - 246, Update OnInitializedAsync so _items is populated from Items only when both Options and ChildContent are null; otherwise initialize it empty for the registered-options/child-content path. Keep the existing Items copy behavior and toggle initialization intact, matching the precedence logic used by BitAccordionList.
126-131: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a monotonic seed for auto-generated option keys.
Relying on
_items.Countto auto-generate keys can lead to key collisions when options are conditionally removed and subsequently added. This exact issue was cleanly addressed inBitAccordionListby using a monotonic seed.Please apply the same pattern here to ensure keys remain strictly unique across the component's lifetime.
💡 Proposed fix
First, declare a seed field at the top of the class:
private int _optionKeySeed;Then update
RegisterOption:- internal void RegisterOption(BitButtonGroupOption option) - { - if (option.Key.HasNoValue()) - { - option.Key = _items.Count.ToString(); - } + internal void RegisterOption(BitButtonGroupOption option) + { + if (option.Key.HasNoValue()) + { + var key = (_optionKeySeed++).ToString(); + while (_items.Any(i => GetItemKey(i) == key)) + { + key = (_optionKeySeed++).ToString(); + } + option.Key = key; + }🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs` around lines 126 - 131, Replace the _items.Count-based key generation in RegisterOption with a private monotonic _optionKeySeed field, incrementing the seed whenever an option without a key is registered. Preserve explicitly supplied keys and ensure generated keys remain unique across removals and additions during the component’s lifetime.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs (1)
612-629: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRe-render the root before the sticky-selection early return.
CloseCallout()changesIsOpen, but Line 616 can return before Line 629 refreshes the parent renderer, leaving open-state classes stale. Refresh immediately after closing or usefinally.Proposed fix
await CloseCallout(); + StateHasChanged(); if (Sticky) { if (await AssignSelectedItem(item) is false) return; @@ - // The click handler runs on the clicked item's renderer, so the root element (open state - // classes) needs an explicit re-render here. - StateHasChanged();🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs` around lines 612 - 629, Update the item-click flow around CloseCallout and AssignSelectedItem so the root component re-renders immediately after CloseCallout changes IsOpen, before the Sticky branch can return early. Preserve the existing click and selection behavior, and avoid relying solely on the later StateHasChanged call.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs (1)
253-253: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOptimize sequence equality check.
Consider adding a
ReferenceEqualscheck before callingSequenceEqualto avoid iterating over the list when the exact same instance is re-evaluated, matching the optimization used inBitAccordionList.♻️ Proposed fix
- if (_oldItems is null || Items.SequenceEqual(_oldItems) is false) + if (_oldItems is null || (ReferenceEquals(Items, _oldItems) is false && Items.SequenceEqual(_oldItems) is false))🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs` at line 253, Optimize the change-detection condition in the ButtonGroup update logic by checking whether Items and _oldItems reference the same instance before invoking SequenceEqual. Preserve the existing null handling and sequence comparison for different instances, matching the established BitAccordionList pattern.
🤖 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/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs`:
- Around line 258-263: Replace the index-based fallback key loop in the
ButtonGroup key-assignment logic with an AssignItemKeys method matching the
BitAccordionList approach: collect all existing keys first, then assign each
missing key using the lowest available deterministic numeric candidate, checking
and recording used keys to prevent collisions. Invoke this method wherever the
current loop performs automatic key assignment.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor`:
- Around line 6-17: Update the item button’s accessibility and interaction state
in the _BitMenuButtonItem template to combine MenuButton.IsEnabled with the
per-item isEnabled value: make tabindex, disabled, and aria-disabled reflect the
item as disabled whenever either state is false, while preserving enabled
behavior only when both are true.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 608-625: The breadcrumb remains stale when BitBreadcrumbOption
parameters change because the parent is not notified. In
src/BlazorUI/Bit.BlazorUI/Components/Navs/BitBreadcrumbOption.cs lines 85-102,
notify the owning BitBreadcrumb when relevant parameters such as Text, Href,
Icon, or IsEnabled change, following the existing notification pattern in
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs lines
102-124. The anchor at
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs lines
608-625 requires no direct change and only demonstrates the analogous
option-invalidation mechanism.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cs`:
- Around line 173-180: Update RefreshOptions and the option registration
lifecycle to track rendered/registered BitTimelineOption instances separately
from _items, which represents Items data. When refreshing after switching to
ChildContent/Options, iterate only the registered-options collection and invoke
InternalStateHasChanged on those instances; do not cast or refresh stale _items
entries.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cs`:
- Around line 3-8: Add the RenderRevision parameter to _BitNavBarItem, add
internal _renderRevision to BitNavBar, and increment it before each
RefreshOptions call in OnParametersSet, OnAfterRender when _selectionDirty is
true, OnSetSelectedItem, OnLocationChanged, and SetSelectedItem; pass the
revision through the BitNavBar.razor item loop and
BitNavBarOption.BuildRenderTree so child instances receive the updated value.
Affected sites: _BitNavBarItem.razor.cs lines 3-8 add the parameter;
BitNavBar.razor.cs lines 228-248 add the field and increments; BitNavBar.razor
lines 22-25 pass RenderRevision; BitNavBarOption.cs lines 109-120 pass the same
parameter through the render tree.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs`:
- Around line 159-163: Update UnregisterOption to detect when the removed option
is the currently toggled item, clear the bound ToggleKey, and set the internal
_toggleItem reference to null before calling StateHasChanged. Preserve the
existing removal behavior for options that are not toggled.
- Around line 221-246: Update OnInitializedAsync so _items is populated from
Items only when both Options and ChildContent are null; otherwise initialize it
empty for the registered-options/child-content path. Keep the existing Items
copy behavior and toggle initialization intact, matching the precedence logic
used by BitAccordionList.
- Around line 126-131: Replace the _items.Count-based key generation in
RegisterOption with a private monotonic _optionKeySeed field, incrementing the
seed whenever an option without a key is registered. Preserve explicitly
supplied keys and ensure generated keys remain unique across removals and
additions during the component’s lifetime.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cs`:
- Around line 612-629: Update the item-click flow around CloseCallout and
AssignSelectedItem so the root component re-renders immediately after
CloseCallout changes IsOpen, before the Sticky branch can return early. Preserve
the existing click and selection behavior, and avoid relying solely on the later
StateHasChanged call.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 263-269: Update OnSetParameters so that after refreshing _items
and _oldItems when Items changes, it calls SetSelectedItemByCurrentUrl() to
recalculate the active selection against the current URL in automatic mode.
---
Nitpick comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cs`:
- Line 253: Optimize the change-detection condition in the ButtonGroup update
logic by checking whether Items and _oldItems reference the same instance before
invoking SequenceEqual. Preserve the existing null handling and sequence
comparison for different instances, matching the established BitAccordionList
pattern.
🪄 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: de02e99c-c053-4e44-b119-91dcb75d3041
📒 Files selected for processing (71)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/_BitChoiceGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticSelectionTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
💤 Files with no reviewable changes (1)
- src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 608-620: Update NotifyOptionParametersChanged and the
option-parameter change flow so notifications occur only when relevant
parameters, such as Text, actually change. When Virtualize is enabled and search
is active, invalidate or refresh the filtered data by scheduling the parent
render or invoking RefreshDataAsync instead of relying solely on
_optionsVersion; preserve the existing non-virtualized RefreshOptions behavior.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cs`:
- Around line 93-107: The parameter-change detection in OnParametersSet must
also track Template and OverflowTemplate delegate references, since they affect
parent-rendered output. Extend the existing snapshot/signature logic to compare
both references by identity alongside the scalar values, and notify Parent when
either template or any existing parameter changes.
🪄 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: 47e79cf4-1593-4d01-bc31-3d45cf5477f2
📒 Files selected for processing (71)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/_BitChoiceGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticSelectionTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
💤 Files with no reviewable changes (1)
- src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cs
Member
Author
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs (1)
88-124: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate option refreshes into the dedicated item components.
Rerendering each option wrapper still supplies stable parent/item references to its child, allowing Blazor to skip the child update. Pass a changing refresh version or directly invalidate the item component.
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs#L88-L124: pass a version changed byInternalStateHasChanged()to_BitDropdownItem.src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs#L110-L133: apply the same trigger to_BitChoiceGroupItem.src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs#L108-L131: apply the same trigger to_BitTimelineItem.#!/bin/bash set -euo pipefail ast-grep outline \ src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs \ src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs \ src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs \ --view expanded rg -n -C4 'SetParametersAsync|InternalStateHasChanged|AddComponentParameter' \ src/BlazorUI/Bit.BlazorUI/Components🤖 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/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs` around lines 88 - 124, Option wrappers rerender without propagating refreshes to their dedicated item components. In BitDropdownOption.cs lines 88-124, BitChoiceGroupOption.cs lines 110-133, and BitTimelineOption.cs lines 108-131, add a refresh version changed by InternalStateHasChanged and pass it as a changing parameter to the corresponding _BitDropdownItem, _BitChoiceGroupItem, and _BitTimelineItem components.src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cs (1)
3-8: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winChange detection boundary blocks UI updates in
_BitNavBarItem.As previously flagged, because
_BitNavBarItemreceives only complex object references (NavBarandItem), Blazor's change detection will skip re-rendering it when the navbar's internal state (e.g.,SelectedItemorIconOnly) changes. This leaves the visual state stale.To ensure visual updates propagate across this component boundary, introduce a revision counter in
BitNavBarand pass it as a new parameter (e.g.,[Parameter] public int RenderRevision { get; set; }) to this component to force parameter invalidation.🤖 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/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cs` around lines 3 - 8, Update _BitNavBarItem<TItem> to accept an integer RenderRevision parameter, and update BitNavBar to maintain and increment that revision whenever internal state changes require descendant UI updates, passing it through each _BitNavBarItem instance. Keep the existing NavBar and Item parameters unchanged.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs (1)
85-96: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winChange detection boundary blocks UI updates in delegated item components.
As previously flagged, because the inner item components (like
_BitMenuButtonItem,_BitNavBarItem, and_BitButtonGroupItem) receive only complex object references (ParentandItem), Blazor's parameter change detection considers them unchanged and skips re-rendering them when parent-driven state (likeSelectedItemorIsToggled) changes. This breaks UI updates even whenInternalStateHasChanged()orRefreshOptions()runs.To ensure visual updates propagate across this component boundary, introduce an integer revision counter (e.g.,
StateIdorRenderRevision) that increments on state changes, and pass it as a parameter to the inner item components.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs#L85-L96: Increment a local_stateIdinInternalStateHasChanged()and pass it to_BitMenuButtonItemusingbuilder.AddComponentParameter().src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cs#L3-L8: Add[Parameter] public int RenderRevision { get; set; }to receive the invalidation signal fromBitNavBar.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs#L157-L168: Increment a_stateIdinInternalStateHasChanged()and pass it to_BitButtonGroupItemduringBuildRenderTree.🤖 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/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs` around lines 85 - 96, Propagate explicit render revisions across delegated item components. In src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs:85-96, increment a local state counter in InternalStateHasChanged() and pass it to _BitMenuButtonItem during BuildRenderTree. In src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cs:3-8, add a RenderRevision parameter. In src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs:157-168, increment its state counter in InternalStateHasChanged() and pass it to _BitButtonGroupItem.
🤖 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/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cs`:
- Around line 330-332: Separate registered option instances from the Items API
snapshot in BitAccordionList: make RefreshOptions and expansion operations use
only the dedicated registered-options collection, excluding stale Items entries.
In
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cs
lines 330-332, update the refresh path accordingly; in
src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cs lines
173-181, refresh only currently registered BitTimelineOption instances rather
than Items.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs`:
- Around line 85-96: Propagate explicit render revisions across delegated item
components. In
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs:85-96,
increment a local state counter in InternalStateHasChanged() and pass it to
_BitMenuButtonItem during BuildRenderTree. In
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cs:3-8,
add a RenderRevision parameter. In
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs:157-168,
increment its state counter in InternalStateHasChanged() and pass it to
_BitButtonGroupItem.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs`:
- Around line 88-124: Option wrappers rerender without propagating refreshes to
their dedicated item components. In BitDropdownOption.cs lines 88-124,
BitChoiceGroupOption.cs lines 110-133, and BitTimelineOption.cs lines 108-131,
add a refresh version changed by InternalStateHasChanged and pass it as a
changing parameter to the corresponding _BitDropdownItem, _BitChoiceGroupItem,
and _BitTimelineItem components.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cs`:
- Around line 3-8: Update _BitNavBarItem<TItem> to accept an integer
RenderRevision parameter, and update BitNavBar to maintain and increment that
revision whenever internal state changes require descendant UI updates, passing
it through each _BitNavBarItem instance. Keep the existing NavBar and Item
parameters 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: c59d0bea-82cc-45b9-9119-39033b5c0c3e
📒 Files selected for processing (71)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionList.razor.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cssrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razorsrc/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/_BitMenuButtonItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/_BitChoiceGroupItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razorsrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimeline.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumb.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Breadcrumb/BitBreadcrumbOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/_BitNavChild.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBarOption.cssrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razorsrc/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/_BitNavBarItem.razor.cssrc/BlazorUI/Bit.BlazorUI/Extensions/JsInterop/UtilsJsRuntimeExtensions.cssrc/BlazorUI/Bit.BlazorUI/Scripts/Utils.tssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/Button/BitButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/MenuButton/BitMenuButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ToggleButton/BitToggleButtonTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Extras/AccordionList/BitAccordionListOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/ChoiceGroup/BitChoiceGroupOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Inputs/Dropdown/BitDropdownOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Lists/Timeline/BitTimelineOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderOptionsTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbAutoReorderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Breadcrumb/BitBreadcrumbOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/Nav/BitNavOptionsOrderTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticSelectionTests.cssrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarAutomaticTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarOptionsTest.razorsrc/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Navs/NavBar/BitNavBarTests.cs
💤 Files with no reviewable changes (1)
- src/BlazorUI/Tests/Bit.BlazorUI.Tests/Components/Buttons/ButtonGroup/BitButtonGroupTests.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #12653
Summary by CodeRabbit
New Features
AutoReorderOptions.Bug Fixes
Tests