doc(Table): update dyanmic datatable sample#8103
Merged
Merged
Conversation
Contributor
Reviewer's GuideRefactors the dynamic DataTable sample to use multiple explicit DataTableDynamicContext instances (including a cached paging example), updates the Razor demo wiring and docs accordingly, and performs a small cleanup in the dynamic object sample helper. Sequence diagram for the updated paging DataTableDynamicContext flowsequenceDiagram
actor User
participant Pagination
participant TablesDynamic
participant DataTableDynamicContext as _dataTableDynamicContext4
participant DataTable as _pageDataTable
User->>Pagination: click page link
Pagination->>TablesDynamic: OnPageLinkClick(pageIndex)
activate TablesDynamic
TablesDynamic->>TablesDynamic: PageIndex = pageIndex
TablesDynamic->>TablesDynamic: UpdatePageDataContext()
activate TablesDynamic
TablesDynamic->>DataTableDynamicContext: get DataTable
DataTableDynamicContext-->>TablesDynamic: _dataTableDynamicContext4.DataTable
TablesDynamic->>DataTable: Rows.Clear()
loop for each f in _pageData.Skip(...).Take(...)
TablesDynamic->>DataTable: Rows.Add(f.Id, f.DateTime, f.Name, f.Count)
end
TablesDynamic->>DataTable: AcceptChanges()
deactivate TablesDynamic
TablesDynamic->>TablesDynamic: StateHasChanged()
TablesDynamic-->>Pagination: Task.CompletedTask
deactivate TablesDynamic
File-Level Changes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The four separate
DataTableDynamicContextfields (_dataTableDynamicContext1–4) are a bit opaque; consider either using more descriptive names (e.g.,_listContext,_editContext,_dynamicColumnsContext,_pagingContext) or grouping them into a small struct/class to clarify their distinct purposes. - In
OnAddColumnandOnRemoveColumnyou rely on_dataTableDynamicContext3!; if this context is required for those interactions it may be clearer to enforce non-nullability (e.g., via initialization guarantees or a guard) instead of relying on the null-forgiving operator.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The four separate `DataTableDynamicContext` fields (`_dataTableDynamicContext1`–`4`) are a bit opaque; consider either using more descriptive names (e.g., `_listContext`, `_editContext`, `_dynamicColumnsContext`, `_pagingContext`) or grouping them into a small struct/class to clarify their distinct purposes.
- In `OnAddColumn` and `OnRemoveColumn` you rely on `_dataTableDynamicContext3!`; if this context is required for those interactions it may be clearer to enforce non-nullability (e.g., via initialization guarantees or a guard) instead of relying on the null-forgiving operator.
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamic.razor.cs" line_range="15-18" />
<code_context>
- private DataTableDynamicContext? DataTableDynamicContext { get; set; }
-
- private DataTable UserData { get; } = new DataTable();
+ private DataTableDynamicContext? _dataTableDynamicContext1;
+ private DataTableDynamicContext? _dataTableDynamicContext2;
+ private DataTableDynamicContext? _dataTableDynamicContext3;
+ private DataTableDynamicContext? _dataTableDynamicContext4;
private List<DynamicObject> SelectedItems { get; set; } = [];
</code_context>
<issue_to_address>
**suggestion:** Numbered context fields make the usage harder to follow and more error-prone as the file grows.
The `_dataTableDynamicContext1`…`4` naming makes it hard to know which context belongs to which table scenario without tracing references through the file, and increases the chance of wiring the wrong context in the `.razor` file. Please rename these to scenario-specific identifiers (e.g. `_mainContext`, `_editableContext`, `_dynamicColumnsContext`, `_pagedContext`) so the intent and bindings are clearer.
Suggested implementation:
```csharp
public partial class TablesDynamic
{
// Context for the primary/basic table scenario
private DataTableDynamicContext? _mainContext;
// Context for the editable table scenario
private DataTableDynamicContext? _editableContext;
// Context for the dynamic columns table scenario
private DataTableDynamicContext? _dynamicColumnsContext;
// Context for the paged table scenario
private DataTableDynamicContext? _pagedContext;
private List<DynamicObject> SelectedItems { get; set; } = [];
```
To fully apply this refactor, you should also:
1. Replace all usages of `_dataTableDynamicContext1` with `_mainContext` within `TablesDynamic.razor.cs` and `TablesDynamic.razor` (e.g. `Context="_mainContext"`).
2. Replace all usages of `_dataTableDynamicContext2` with `_editableContext`.
3. Replace all usages of `_dataTableDynamicContext3` with `_dynamicColumnsContext`.
4. Replace all usages of `_dataTableDynamicContext4` with `_pagedContext`.
5. If any of the contexts actually map to different scenarios than "main/editable/dynamicColumns/paged", adjust the field names and comments accordingly to match the real usage.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Table Dynamic sample documentation and implementation to better demonstrate DataTableDynamicContext usage across multiple scenarios, including more efficient pagination updates and new localized explanatory notes.
Changes:
- Refactors the dynamic
DataTablesample to use multipleDataTableDynamicContextinstances and shared creation helpers. - Improves the pagination demo to update the existing
DataTablerows instead of recreating the dynamic context on each page change (withUseCache = false). - Adds new localized “Notes” content for the dynamic table sample (en-US / zh-CN), plus minor cleanup in the dynamic object sample.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor.Server/Locales/zh-CN.json | Adds new zh-CN localized strings for dynamic table notes and “new row” default text. |
| src/BootstrapBlazor.Server/Locales/en-US.json | Adds new en-US localized strings for dynamic table notes and “new row” default text. |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamicObject.razor.cs | Minor refactor/cleanup of random generation and dynamic row data construction. |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamic.razor.cs | Refactors sample contexts, adds paging context update method, and adjusts dynamic context creation logic. |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesDynamic.razor | Expands the demo page with new notes + inline sample code and updates bindings to new contexts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8103 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 766 766
Lines 34204 34204
Branches 4696 4696
=========================================
Hits 34204 34204
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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 #8102
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Refine the dynamic DataTable sample implementation and documentation to better demonstrate multi-context usage and efficient pagination updates.
Enhancements:
Documentation: