fix(Table): line no support SortableList #6045
Merged
Conversation
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
Contributor
Reviewer's GuideThis PR implements drag-and-drop reordering support in the SortableList sample by adding an OnUpdateTable handler and enabling line numbering for all items; optimizes Table component rendering efficiency by applying @key directives to row elements; corrects unit tests by refreshing checkbox component retrieval between pagination steps; and updates the project version to 9.6.4-beta02. Sequence Diagram: Item Reordering in SortableList TablesequenceDiagram
actor User
participant SL as SortableList Component
participant Page as SortableLists.razor.cs (Page Logic)
participant Data as Items Collection
User->>SL: Drags and drops an item in the table
SL->>Page: Triggers OnUpdate event (with SortableEvent)
activate Page
Page->>Page: Executes OnUpdateTable(SortableEvent)
Page->>Data: Removes item from oldIndex
Page->>Data: Inserts item at newIndex
Page->>Page: Calls StateHasChanged() to refresh UI
deactivate Page
Page->>SL: (Blazor rendering) Re-renders with updated items
Class Diagram: Updates to SortableLists Sample and ComponentsclassDiagram
class SortableLists_Page {
-List~Foo~ Items
+OnUpdateTable(SortableEvent event) Task
}
note for SortableLists_Page "Represents SortableLists.razor.cs. Handles item reordering logic triggered by the SortableList component."
class SortableList_Component {
+SortableListOption Option
+EventCallback~SortableEvent~ OnUpdate
}
note for SortableList_Component "OnUpdate event handler now utilized to callback SortableLists_Page.OnUpdateTable for item reordering."
class Table_Component {
+IEnumerable~Foo~ Items
+bool IsStriped
+bool ShowLineNo
}
note for Table_Component "ShowLineNo parameter now set to true in the sample. Diff also shows @key directive added to rows for rendering optimization (not a new class member)."
SortableLists_Page "1" ..> "1" SortableList_Component : uses
SortableList_Component "1" ..> "1" Table_Component : contains
SortableList_Component --|> SortableLists_Page : OnUpdate event
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances sortable list functionality and improves the Table component’s rendering performance while updating the project version.
- Enhanced sortable lists with an event handler for reordering.
- Improved Table rendering by adding @key to row elements.
- Updated project version from 9.6.4-beta01 to 9.6.4-beta02.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/BootstrapBlazor/Components/Table/Table.razor | Added @key to DynamicElement for improved row tracking. |
| src/BootstrapBlazor/BootstrapBlazor.csproj | Bumped version number to 9.6.4-beta02. |
| src/BootstrapBlazor.Server/Components/Samples/SortableLists.razor.cs | Introduced OnUpdateTable method for sortable list reordering. |
| src/BootstrapBlazor.Server/Components/Samples/SortableLists.razor | Updated SortableList to use OnUpdateTable and show all items with line numbers. |
Co-Authored-By: ChrisHsieh999 <[email protected]>
Member
Author
|
@sourcery-ai review |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6045 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 701 701
Lines 30958 30958
Branches 4378 4378
=========================================
Hits 30958 30958 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @ArgoZhang - I've reviewed your changes - here's some feedback:
- In the OnUpdateTable handler, consider using InvokeAsync(() => StateHasChanged()) to ensure the state change is scheduled on the correct rendering context.
- For the @key directive on rows, prefer using a unique identifier property (e.g. item.Id) instead of the item object itself to guarantee stable keys when item values change.
- The repeated cut.FindComponents<Checkbox>() calls in TableTest could be pulled into a small helper method to reduce duplication and improve test clarity.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| private Task OnUpdateTable(SortableEvent @event) | ||
| { | ||
| var oldItem = Items[@event.OldIndex]; | ||
| Items.Remove(oldItem); |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Validate indices before reordering
Add range checks for @event.OldIndex and @event.NewIndex before calling Remove or Insert to prevent out-of-range exceptions.
Comment on lines
+139
to
+147
| private Task OnUpdateTable(SortableEvent @event) | ||
| { | ||
| var oldItem = Items[@event.OldIndex]; | ||
| Items.Remove(oldItem); | ||
| Items.Insert(@event.NewIndex, oldItem); | ||
|
|
||
| StateHasChanged(); | ||
| return Task.CompletedTask; | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion: Favor InvokeAsync(StateHasChanged) in event handlers
Await InvokeAsync(StateHasChanged) to ensure the UI update runs on the proper synchronization context in sortable event callbacks.
Suggested change
| private Task OnUpdateTable(SortableEvent @event) | |
| { | |
| var oldItem = Items[@event.OldIndex]; | |
| Items.Remove(oldItem); | |
| Items.Insert(@event.NewIndex, oldItem); | |
| StateHasChanged(); | |
| return Task.CompletedTask; | |
| } | |
| private async Task OnUpdateTable(SortableEvent @event) | |
| { | |
| var oldItem = Items[@event.OldIndex]; | |
| Items.Remove(oldItem); | |
| Items.Insert(@event.NewIndex, oldItem); | |
| await InvokeAsync(StateHasChanged); | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Link issues
fixes #6021
Summary By Copilot
This pull request introduces enhancements to the sortable list functionality, improves performance in the
Tablecomponent, and updates the project version. Key changes include adding an event handler for updating sortable lists, ensuring efficient rendering in theTablecomponent, and incrementing the version number.Enhancements to Sortable Lists:
src/BootstrapBlazor.Server/Components/Samples/SortableLists.razor: Added theOnUpdateevent handler toSortableListfor handling item reordering. Updated theTableto show line numbers and include all items instead of a subset.src/BootstrapBlazor.Server/Components/Samples/SortableLists.razor.cs: Implemented theOnUpdateTablemethod to handle reordering logic by updating theItemslist based on the drag-and-drop operation.Performance Improvements in the
TableComponent:src/BootstrapBlazor/Components/Table/Table.razor: Added the@keydirective to table rows to optimize rendering by ensuring proper tracking of row elements during updates. [1] [2]Project Version Update:
src/BootstrapBlazor/BootstrapBlazor.csproj: Updated the project version from9.6.4-beta01to9.6.4-beta02.Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enable SortableList to handle item reordering with full item rendering and line numbers, optimize Table rendering with @key, and update project version
Bug Fixes:
Enhancements:
Build:
Tests: