Fix item order issues in multi-api components (#12653)#12659
Open
msynk wants to merge 20 commits into
Open
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
|
✅ 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 (2)
src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs (1)
121-133: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard against disposal and refresh options on selection changes.
There are two issues in these registration methods related to the new option rendering lifecycle:
UnregisterOptioncan be called during teardown after the parent is disposed, which will throw when it reachesStateHasChanged(). Add anif (IsDisposed) return;guard.- Both methods call
SetSelectedItemByCurrentUrl(), which can alter the internally trackedSelectedItem. Without explicitly callingRefreshOptions(), existing options won't re-render to reflect the newly active selection state, leaving their CSS classes stale.🐛 Proposed fix
internal void RegisterOption(BitNavBarOption option) { _items.Add((option as TItem)!); SetSelectedItemByCurrentUrl(); + RefreshOptions(); StateHasChanged(); } internal void UnregisterOption(BitNavBarOption option) { + if (IsDisposed) return; + _items.Remove((option as TItem)!); SetSelectedItemByCurrentUrl(); + 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 121 - 133, Update RegisterOption and UnregisterOption to return immediately from UnregisterOption when IsDisposed is true before performing state updates. In both methods, call RefreshOptions() after SetSelectedItemByCurrentUrl() so existing options re-render with the current selection state, while preserving the existing registration, selection, and StateHasChanged behavior.src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs (1)
93-98: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard option unregistration against updates after disposal.
During component teardown, if child options are disposed after the parent, this method will call
StateHasChanged()on a disposed component. Add anif (IsDisposed) return;guard (similar to the one successfully added inBitBreadcrumb) to prevent teardown exceptions.🛡️ Proposed fix
internal void UnregisterOption(BitNavOption option) { + if (IsDisposed) return; + _items.Remove((option as TItem)!); 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/Nav/BitNav.razor.cs` around lines 93 - 98, Add an early IsDisposed guard at the start of BitNav.UnregisterOption, returning before modifying _items or calling StateHasChanged when the parent component has been disposed. Preserve the existing unregistration behavior for active components.
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs (1)
1069-1070: 🚀 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()), following the establishedBitChoiceGrouppattern.🤖 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 1069 - 1070, Remove RefreshOptions() from OnAfterRenderAsync and invoke it synchronously at the end of OnParametersSetAsync and immediately after internal state mutations in SearchItems(), ClearSearchBox(), ClearComboBoxInput(), AddOrRemoveSelectedItem(), UpdateSelectedItemsFromValues(), and HandleOnClearClick(), following the 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/Buttons/ButtonGroup/_BitButtonGroupItem.razor`:
- Around line 9-15: Update the button element in _BitButtonGroupItem.razor to
explicitly set type="button", preserving the existing click handling and
attributes so ButtonGroup item clicks do not submit an enclosing form.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Around line 16-21: Update the style attribute in the Timeline item markup to
separate Timeline.GetStyle(Item) and Timeline.Styles?.Item with a semicolon,
preserving both existing style values.
---
Outside diff comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNav.razor.cs`:
- Around line 93-98: Add an early IsDisposed guard at the start of
BitNav.UnregisterOption, returning before modifying _items or calling
StateHasChanged when the parent component has been disposed. Preserve the
existing unregistration behavior for active components.
In `@src/BlazorUI/Bit.BlazorUI/Components/Navs/NavBar/BitNavBar.razor.cs`:
- Around line 121-133: Update RegisterOption and UnregisterOption to return
immediately from UnregisterOption when IsDisposed is true before performing
state updates. In both methods, call RefreshOptions() after
SetSelectedItemByCurrentUrl() so existing options re-render with the current
selection state, while preserving the existing registration, selection, and
StateHasChanged behavior.
---
Duplicate comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 1069-1070: Remove RefreshOptions() from OnAfterRenderAsync and
invoke it synchronously at the end of OnParametersSetAsync and immediately after
internal state mutations in SearchItems(), ClearSearchBox(),
ClearComboBoxInput(), AddOrRemoveSelectedItem(),
UpdateSelectedItemsFromValues(), and HandleOnClearClick(), following the
BitChoiceGroup 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: d3b88e6e-9be0-4538-899f-e971cda12c13
📒 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. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor (1)
21-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
GetItemKeyfor the@keyattribute to ensure stable component tracking.The
foreachloop currently uses@key="item", which relies on object identity. If theItemscollection is updated with new instances that represent the same logical items, the component will lose element tracking and unnecessarily re-create the DOM elements.Consistent with
BitAccordionList.razor, utilizing the explicitly managed string keys preserves element state across renders correctly.💚 Proposed fix
- <_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` at line 21, Update the foreach rendering in BitButtonGroup to use the existing GetItemKey method for the _BitButtonGroupItem `@key` value instead of the item object, preserving stable component tracking when equivalent item instances are recreated.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor (1)
107-110: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse a
forloop to prevent allocations during rendering.Using LINQ
.Select()in the render tree builder creates a new enumerator and closure on every render cycle. Since_itemsis inherently aList, you can use a simpleforloop to avoid these allocations and slightly improve rendering performance.⚡ Proposed refactor
- `@foreach` (var (item, idx) in _items.Select((item, idx) => (item, idx))) - { - <_BitMenuButtonItem `@key`="`@GetItemKey`(item, idx.ToString())" MenuButton="this" Item="item" /> - } + `@for` (var i = 0; i < _items.Count; i++) + { + var item = _items[i]; + <_BitMenuButtonItem `@key`="`@GetItemKey`(item, i.ToString())" MenuButton="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/MenuButton/BitMenuButton.razor` around lines 107 - 110, Replace the LINQ Select-based foreach in the menu item rendering block with an indexed for loop over _items, passing each item and its index to GetItemKey and _BitMenuButtonItem while preserving the existing rendering and key 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.
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroup.razor`:
- Line 21: Update the foreach rendering in BitButtonGroup to use the existing
GetItemKey method for the _BitButtonGroupItem `@key` value instead of the item
object, preserving stable component tracking when equivalent item instances are
recreated.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButton.razor`:
- Around line 107-110: Replace the LINQ Select-based foreach in the menu item
rendering block with an indexed for loop over _items, passing each item and its
index to GetItemKey and _BitMenuButtonItem while preserving the existing
rendering and key behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bce56625-f844-4bdd-a45c-2d7dc064c8a0
📒 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. |
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 (2)
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs (1)
82-110: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftChild item components will not re-render due to reference equality.
When
RefreshOptions()callsInternalStateHasChanged()on an option, the option'sBuildRenderTreeexecutes and outputs the child component (e.g.,_BitAccordionListItem) with the exact sameParentandthisobject references. Because Blazor's diffing engine uses reference equality for objects, it will conclude that the parameters haven't changed and will skip rendering the child component.As a result, parent-driven state changes (like expanding an accordion item or changing button group states) will leave the previously active items' UI completely stale, because their item components are bypassed by the renderer.
To force Blazor to re-render the child component, pass an explicitly changing primitive parameter (like a render count) whenever the option needs to refresh. You should apply this pattern across all delegated item components in this PR (e.g., ButtonGroup, ChoiceGroup, Dropdown, etc.):
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs#L82-L110: Add an_internalRenderCountfield, increment it insideInternalStateHasChanged(), and pass it to the child inBuildRenderTree().src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cs#L3-L8: Add[Parameter] public int RenderCount { get; set; }to receive the changing value.src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs#L108-L133: Apply the same render count pattern here.🛠️ Proposed fix pattern for Option components
For example, in
BitAccordionListOption.cs:+ private int _internalRenderCount; + internal void InternalStateHasChanged() { + _internalRenderCount++; StateHasChanged(); } ... protected override void BuildRenderTree(RenderTreeBuilder builder) { if (Parent is null) return; builder.OpenComponent<_BitAccordionListItem<BitAccordionListOption>>(0); builder.AddComponentParameter(1, nameof(_BitAccordionListItem<BitAccordionListOption>.AccordionList), Parent); builder.AddComponentParameter(2, nameof(_BitAccordionListItem<BitAccordionListOption>.Item), this); + builder.AddComponentParameter(3, "RenderCount", _internalRenderCount); builder.CloseComponent(); }🤖 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.Extras/Components/AccordionList/BitAccordionListOption.cs` around lines 82 - 110, Add an incrementing internal render-count parameter so delegated child components re-render when options refresh. In src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs lines 82-110, add and increment the count in InternalStateHasChanged and pass it from BuildRenderTree; in src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cs lines 3-8, add the corresponding RenderCount parameter; apply the same pattern in src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs lines 108-133.src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs (1)
615-628: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
UnregisterOptionmissingIsDisposedguard.Unlike
BitBreadcrumb.UnregisterOptions,BitNav.UnregisterOption, andBitNavBar.UnregisterOption(all updated in this PR to guard against post-disposalStateHasChanged()calls), this method has noIsDisposedcheck, so it can still mutate state and callStateHasChanged()during teardown races.🛡️ Proposed fix
internal void UnregisterOption(BitDropdownOption<TValue> option) { + if (IsDisposed) return; + var item = (option as TItem)!; Items!.Remove(item); _searchedItemsCache = null;🤖 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 615 - 628, Update BitDropdown.UnregisterOption to return immediately when IsDisposed is true, before mutating Items, _selectedItems, or invoking StateHasChanged. Preserve the existing unregister behavior for active components.
♻️ Duplicate comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs (1)
120-131: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winChild item components will not re-render when the option's state changes.
Because
_BitChoiceGroupItemand_BitNavChildreceive only stable parameter references (such asParent,this, andDepth), Blazor's parameter equality checks will evaluate them as unchanged. Consequently, whenStateHasChanged()is invoked on the option, the nested item component will skip re-rendering, causing internal state updates to not reflect in the UI.To force a refresh, bypass parameter equality checks by passing a changing parameter (such as an incrementing
StateIdinteger) to the child component, or expose an explicit refresh method on the inner component to manually invoke.
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs#L120-L131: Pass a changing parameter to_BitChoiceGroupItem.src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor#L3-L21: Pass a changing parameter to_BitNavChild.🤖 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 120 - 131, The child components receive only stable parameters and therefore skip updates when their option state changes. In BitChoiceGroupOption.BuildRenderTree, pass a changing refresh parameter such as an incrementing StateId to _BitChoiceGroupItem; apply the same changing-parameter approach to _BitNavChild in src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor lines 3-21, preserving the existing rendering behavior.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, 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 the BitTimelineItem markup to insert a space between the base class `bit-tln-itm` and the `Timeline.GetItemClasses(Item)` result, preserving separate CSS class tokens while leaving the remaining class expressions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 573-595: Reset _searchedItemsCache in OnParametersSet whenever
component parameters are applied, so changes to Items cannot reuse search
results from a previous parameter set. Update the comment above
ShouldRenderOptionItem to describe this explicit refresh and parameter-driven
cache invalidation flow.
---
Outside diff comments:
In
`@src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs`:
- Around line 82-110: Add an incrementing internal render-count parameter so
delegated child components re-render when options refresh. In
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/BitAccordionListOption.cs
lines 82-110, add and increment the count in InternalStateHasChanged and pass it
from BuildRenderTree; in
src/BlazorUI/Bit.BlazorUI.Extras/Components/AccordionList/_BitAccordionListItem.razor.cs
lines 3-8, add the corresponding RenderCount parameter; apply the same pattern
in src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/BitTimelineOption.cs
lines 108-133.
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 615-628: Update BitDropdown.UnregisterOption to return immediately
when IsDisposed is true, before mutating Items, _selectedItems, or invoking
StateHasChanged. Preserve the existing unregister behavior for active
components.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs`:
- Around line 120-131: The child components receive only stable parameters and
therefore skip updates when their option state changes. In
BitChoiceGroupOption.BuildRenderTree, pass a changing refresh parameter such as
an incrementing StateId to _BitChoiceGroupItem; apply the same
changing-parameter approach to _BitNavChild in
src/BlazorUI/Bit.BlazorUI/Components/Navs/Nav/BitNavOption.razor lines 3-21,
preserving the existing rendering behavior.
In `@src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor`:
- Line 21: Update the class attribute in the BitTimelineItem markup to insert a
space between the base class `bit-tln-itm` and the
`Timeline.GetItemClasses(Item)` result, preserving separate CSS class tokens
while leaving the remaining class expressions 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: 0554e8b5-8ad1-440c-b666-5cc66f41bcdf
📒 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs (1)
143-168: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the child item components refreshable.
The child item components receive stable parameter references (
ParentandItem). Since these object references do not change, Blazor's parameter diffing will skip re-rendering the inner components even whenInternalStateHasChanged()is triggered on the option. This prevents state updates (such as toggle states) from visually updating. To force a refresh, pass a changing parameter (such as an incrementing_stateId) to bypass the equality checks.
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs#L143-L168: Increment a_stateIdinInternalStateHasChangedand pass it as a parameter to_BitButtonGroupItem.src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs#L71-L96: Increment a_stateIdinInternalStateHasChangedand pass it as a parameter to_BitMenuButtonItem.src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cs#L3-L8: Add[Parameter] public int StateId { get; set; }to receive the changing parameter. (Note: Ensure_BitMenuButtonItemis updated similarly).💡 Proposed fixes
src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs
+ private int _stateId; + internal void InternalStateHasChanged() { + _stateId++; StateHasChanged(); }protected override void BuildRenderTree(RenderTreeBuilder builder) { if (Parent is null) return; builder.OpenComponent<_BitButtonGroupItem<BitButtonGroupOption>>(0); builder.AddComponentParameter(1, nameof(_BitButtonGroupItem<BitButtonGroupOption>.ButtonGroup), Parent); builder.AddComponentParameter(2, nameof(_BitButtonGroupItem<BitButtonGroupOption>.Item), this); + builder.AddComponentParameter(3, nameof(_BitButtonGroupItem<BitButtonGroupOption>.StateId), _stateId); builder.CloseComponent(); }src/BlazorUI/Bit.BlazorUI/Components/Buttons/MenuButton/BitMenuButtonOption.cs
+ private int _stateId; + internal void InternalStateHasChanged() { + _stateId++; StateHasChanged(); }protected override void BuildRenderTree(RenderTreeBuilder builder) { if (Parent is null) return; builder.OpenComponent<_BitMenuButtonItem<BitMenuButtonOption>>(0); builder.AddComponentParameter(1, nameof(_BitMenuButtonItem<BitMenuButtonOption>.MenuButton), Parent); builder.AddComponentParameter(2, nameof(_BitMenuButtonItem<BitMenuButtonOption>.Item), this); + builder.AddComponentParameter(3, nameof(_BitMenuButtonItem<BitMenuButtonOption>.StateId), _stateId); builder.CloseComponent(); }src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/_BitButtonGroupItem.razor.cs
public partial class _BitButtonGroupItem<TItem> where TItem : class { [Parameter] public TItem Item { get; set; } = default!; [Parameter] public BitButtonGroup<TItem> ButtonGroup { get; set; } = default!; + + [Parameter] public int StateId { get; set; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs` around lines 143 - 168, Make the child item components refreshable by adding and incrementing a _stateId in InternalStateHasChanged and passing it to _BitButtonGroupItem in BitButtonGroupOption.cs (lines 143-168) and to _BitMenuButtonItem in BitMenuButtonOption.cs (lines 71-96). Add a StateId parameter to _BitButtonGroupItem in _BitButtonGroupItem.razor.cs (lines 3-8) and add the equivalent parameter to _BitMenuButtonItem so the changing value bypasses parameter equality checks.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 (
Parentandthis) and will not automatically re-render whenInternalStateHasChanged()triggers a render on the option. Blazor's parameter diffing sees that the references haven't changed and skips re-rendering the inner component. The shared root cause is the lack of a changing parameter or explicit method call to bypass equality diffing.
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs#L122-L130: pass a changing parameter (such as aStateIdinteger that increments inInternalStateHasChanged) to_BitChoiceGroupItemor invoke a refresh method on it.src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdownOption.cs#L106-L116: pass a similar changing parameter to_BitDropdownItemor invoke a refresh method on it.🤖 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 child item components do not refresh when their option parents re-render because stable references are passed to them. Update BitChoiceGroupOption.cs lines 122-130 and BitDropdownOption.cs lines 106-116 to pass a changing state parameter, such as an incrementing StateId from InternalStateHasChanged, to _BitChoiceGroupItem and _BitDropdownItem respectively, or invoke an explicit refresh method on each child.
🧹 Nitpick comments (1)
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs (1)
1442-1445: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRemove redundant
RefreshOptions()call.
UpdateSelectedItemsFromValues()already invokesRefreshOptions()internally at the end of its execution. Calling it again here results in an unnecessary double-refresh of all option components.♻️ Proposed fix
UpdateSelectedItemsFromValues(); - - 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/Inputs/Dropdown/BitDropdown.razor.cs` around lines 1442 - 1445, Remove the redundant RefreshOptions() invocation immediately after UpdateSelectedItemsFromValues() in the surrounding dropdown update flow. Keep UpdateSelectedItemsFromValues() as the sole refresh trigger, since it already refreshes the option components internally.
🤖 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/_BitTimelineItem.razor`:
- Around line 16-19: Update the clickable div in BitTimelineItem to expose
button semantics and handle keyboard activation: add role="button", invoke
Timeline.HandleOnItemClick(Item) for Enter and Space, and prevent the default
Space behavior to avoid scrolling. Preserve the existing tabindex, disabled,
aria-disabled, and pointer-click behavior.
---
Duplicate comments:
In
`@src/BlazorUI/Bit.BlazorUI/Components/Buttons/ButtonGroup/BitButtonGroupOption.cs`:
- Around line 143-168: Make the child item components refreshable by adding and
incrementing a _stateId in InternalStateHasChanged and passing it to
_BitButtonGroupItem in BitButtonGroupOption.cs (lines 143-168) and to
_BitMenuButtonItem in BitMenuButtonOption.cs (lines 71-96). Add a StateId
parameter to _BitButtonGroupItem in _BitButtonGroupItem.razor.cs (lines 3-8) and
add the equivalent parameter to _BitMenuButtonItem so the changing value
bypasses parameter equality checks.
In
`@src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs`:
- Around line 122-130: The child item components do not refresh when their
option parents re-render because stable references are passed to them. Update
BitChoiceGroupOption.cs lines 122-130 and BitDropdownOption.cs lines 106-116 to
pass a changing state parameter, such as an incrementing StateId from
InternalStateHasChanged, to _BitChoiceGroupItem and _BitDropdownItem
respectively, or invoke an explicit refresh method on each child.
---
Nitpick comments:
In `@src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs`:
- Around line 1442-1445: Remove the redundant RefreshOptions() invocation
immediately after UpdateSelectedItemsFromValues() in the surrounding dropdown
update flow. Keep UpdateSelectedItemsFromValues() as the sole refresh trigger,
since it already refreshes the option components internally.
🪄 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: f3188003-e379-4936-94f5-6647ad7c647d
📒 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. |
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
Comment on lines
149
to
157
| internal void UnregisterOption(BitChoiceGroupOption<TValue> option) | ||
| { | ||
| _items.Remove((option as TItem)!); | ||
| _optionsOrderDirty = true; | ||
|
|
||
| SetIndexItems(); | ||
|
|
||
| StateHasChanged(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C8 'UnregisterOption|Parent\?\.UnregisterOption|IsDisposed' \
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cs \
src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs
base_file="$(fd -a '^BitComponentBase\.cs$' | head -n1)"
[ -z "$base_file" ] || rg -n -C8 'IsDisposed|StateHasChanged|DisposeAsync' "$base_file"Repository: bitfoundation/bitplatform
Length of output: 7249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the ChoiceGroup component around registration/disposal and selection logic
sed -n '1,260p' src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroup.razor.cs | cat -n
echo
echo "---- BitChoiceGroupOption ----"
sed -n '1,220p' src/BlazorUI/Bit.BlazorUI/Components/Inputs/ChoiceGroup/BitChoiceGroupOption.cs | cat -n
echo
echo "---- sibling patterns ----"
for f in \
src/BlazorUI/Bit.BlazorUI/Components/Inputs/Dropdown/BitDropdown.razor.cs \
src/BlazorUI/Bit.BlazorUI/Components/Navigation/Breadcrumb/BitBreadcrumb.razor.cs
do
if [ -f "$f" ]; then
echo "FILE: $f"
rg -n -C4 'IsDisposed|StateHasChanged|DisposeAsync|Unregister|Register' "$f"
fi
doneRepository: bitfoundation/bitplatform
Length of output: 21033
Skip unregister work after the parent is disposed. BitChoiceGroupOption.Dispose() can hit this during subtree teardown, and StateHasChanged() after disposal can throw. Add the same IsDisposed guard used in BitDropdown.
🤖 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 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.
Comment on lines
+16
to
+18
| <div @onclick="() => Timeline.HandleOnItemClick(Item)" | ||
| @onkeydown="HandleOnItemKeyDown" | ||
| @onkeydown:preventDefault="_preventKeyDownDefault" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant files and map their structure.
git ls-files 'src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor' \
'src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cs'
echo
echo '--- outline: _BitTimelineItem.razor.cs ---'
ast-grep outline src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cs --view expanded
echo
echo '--- file: _BitTimelineItem.razor ---'
cat -n src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor
echo
echo '--- file: _BitTimelineItem.razor.cs ---'
cat -n src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.csRepository: bitfoundation/bitplatform
Length of output: 4570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for similar dynamic preventDefault usages to compare pattern and scope.
rg -n "`@onkeydown`:preventDefault|preventDefault=\"" src/BlazorUI/Bit.BlazorUI -g '*.razor' -g '*.razor.cs' -g '*.cs'Repository: bitfoundation/bitplatform
Length of output: 549
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Blazor semantics in the repository docs/comments, if any.
rg -n "preventDefault|keydown" README.md src/BlazorUI/Bit.BlazorUI -g '*.md' -g '*.razor' -g '*.razor.cs' -g '*.cs'Repository: bitfoundation/bitplatform
Length of output: 3162
🌐 Web query:
Blazor @onkeydown:preventDefault dynamic binding evaluated on render event handler current event one event late documentation
💡 Result:
The "one event late" behavior often perceived when using dynamic binding for @on{EVENT}:preventDefault is not a bug, but rather a consequence of how Blazor processes event handlers and rendering updates [1][2]. ### Understanding the Behavior When you bind @onkeydown:preventDefault="myBool", Blazor evaluates the expression myBool and updates the DOM's event listener settings [2]. In Blazor WebAssembly, it is technically possible to change the value of myBool within the same event handler, and because the component re-renders (and updates the browser's event listener state) before the browser event handler completes, the preventDefault status can update synchronously [1][2]. However, this behavior relies on synchronous execution and is not guaranteed across all hosting models [1]. In Blazor Server and Blazor Hybrid, the process is inherently asynchronous [3][4]. The browser event must be sent to the server (or the.NET context), processed, and then a response must be sent back to update the UI [4]. By the time your.NET event handler logic runs and updates myBool, the browser has already finished executing the default action associated with the event [3][4]. Consequently, attempting to change the preventDefault state during the handler for the current event will not affect that specific event; it will only take effect for subsequent events [4]. ### Recommended Approaches 1. Use Static Prevention: If the default behavior should always be prevented, use @onkeydown:preventDefault without a value (or with a constant true) [5][6]. 2. Use JavaScript Interop for Dynamic Logic: If you need to conditionally prevent default behavior based on specific event details (e.g., preventing the default action only for the "Enter" key while allowing other keys), the standard recommendation is to handle this logic in JavaScript [3]. You can attach a JavaScript event listener that evaluates the condition and calls event.preventDefault() synchronously, then invokes your Blazor event handler via DotNetObjectReference if necessary [7][3][4]. 3. Avoid Depending on Synchronous Rendering: Microsoft advises against writing logic that relies on the synchronous nature of Blazor WebAssembly's event handling, as it creates inconsistencies between hosting models [1]. Official documentation does not recommend this pattern for cross-platform compatibility [1]. ### Summary of Documentation Status Official Microsoft documentation explains the use of the @on{EVENT}:preventDefault directive but explicitly avoids recommending dynamic evaluation based on event handler logic to prevent default actions, as it cannot be guaranteed to work consistently across Blazor's supported execution models [5][1][3].
Citations:
- 1: Clarify intentions for dynamic preventDefault dotnet/aspnetcore#48193
- 2: Example demonstrating conditionally prevent default. dotnet/AspNetCore.Docs#28692
- 3: Programmatically set preventDefault and stopPropagation dotnet/aspnetcore#54782
- 4: How to preventDefault when handle event like js dotnet/aspnetcore#33422
- 5: https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling?view=aspnetcore-10.0
- 6: https://devblogs.microsoft.com/dotnet/asp-net-core-updates-in-net-core-3-1-preview-2/
- 7: https://stackoverflow.com/questions/61373674/how-to-conditionally-preventdefault-inside-the-input-component-in-blazor
🌐 Web query:
site:learn.microsoft.com Blazor preventDefault event handler render tree modifier keydown
💡 Result:
To prevent the default behavior of a keydown event in Blazor, you can use the @onkeydown:preventDefault directive attribute [1][2]. Basic usage: <input @onkeydown:preventDefault /> This directive can be applied even when you are also binding a C# event handler. You can specify the attribute multiple times or use it alongside your handler [1][3]. For example: <input @onkeydown="KeyHandler" @onkeydown:preventDefault /> Conditional prevention: You can also bind the preventDefault behavior to a boolean field or expression to control it dynamically [1]: <input @onkeydown:preventDefault="shouldPreventDefault" /> @code { private bool shouldPreventDefault = true; } If you need to perform more complex logic involving modifier keys (such as checking for Shift, Ctrl, or Alt) while preventing the default action, you can inspect the KeyboardEventArgs in your C# handler [4]. Note that the @onkeydown:preventDefault attribute itself is evaluated by the Blazor event system before the handler is invoked, ensuring the browser's default action (such as typing a character or scrolling) is suppressed as intended [1]. Top results: [1], [2], [4]
Citations:
- 1: https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling?view=aspnetcore-10.0
- 2: https://learn.microsoft.com/en-us/aspnet/core/blazor/components/event-handling?view=aspnetcore-9.0
- 3: https://learn.microsoft.com/en-us/answers/questions/1329338/how-to-suppress-scrolling-by-mouse-wheel-under-cer
- 4: https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.components.web.keyboardeventargs.key?view=aspnetcore-10.0
Use a native button or prevent the current keydown synchronously. @onkeydown:preventDefault="_preventKeyDownDefault" uses the last rendered value, so setting _preventKeyDownDefault inside HandleOnItemKeyDown won’t stop the current Space press from scrolling. A <button type="button"> would handle Space/Enter automatically; otherwise move the prevention to synchronous JS/capture-phase logic keyed off e.Key.
📍 Affects 2 files
src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor#L16-L18(this comment)src/BlazorUI/Bit.BlazorUI/Components/Lists/Timeline/_BitTimelineItem.razor.cs#L11-L22
🤖 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`
around lines 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.
Comment on lines
+155
to
+161
| if (ChildContent is null && Options is null && Items.Any() && | ||
| (_oldItems is null || Items.SequenceEqual(_oldItems) is false)) | ||
| { | ||
| _oldItems = Items; | ||
| _items = Items.ToList(); | ||
| // Store an independent snapshot so a later in-place mutation of the same Items instance is | ||
| // still detected (a reference copy would compare the collection against itself). | ||
| _oldItems = _items.ToList(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear _items when Items becomes empty.
The Items.Any() guard skips synchronization when a populated collection is cleared, leaving the previous timeline entries rendered.
Proposed fix
- if (ChildContent is null && Options is null && Items.Any() &&
- (_oldItems is null || Items.SequenceEqual(_oldItems) is false))
+ if (ChildContent is null && Options is null)
{
- _items = Items.ToList();
- // Store an independent snapshot so a later in-place mutation of the same Items instance is
- // still detected (a reference copy would compare the collection against itself).
- _oldItems = _items.ToList();
+ var items = Items.ToList();
+
+ if (_oldItems is null || items.SequenceEqual(_oldItems) is false)
+ {
+ _items = items;
+ _oldItems = items.ToList();
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (ChildContent is null && Options is null && Items.Any() && | |
| (_oldItems is null || Items.SequenceEqual(_oldItems) is false)) | |
| { | |
| _oldItems = Items; | |
| _items = Items.ToList(); | |
| // Store an independent snapshot so a later in-place mutation of the same Items instance is | |
| // still detected (a reference copy would compare the collection against itself). | |
| _oldItems = _items.ToList(); | |
| if (ChildContent is null && Options is null) | |
| { | |
| var items = Items.ToList(); | |
| if (_oldItems is null || items.SequenceEqual(_oldItems) is false) | |
| { | |
| _items = items; | |
| _oldItems = items.ToList(); | |
| } |
🤖 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/BitTimeline.razor.cs`
around lines 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.
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, keeping breadcrumb items aligned with DOM order.Bug Fixes
Tests