feat(Table): add ClearTableColumnClientStatus instance method#8005
Merged
Conversation
Contributor
Reviewer's GuideAdds an instance method on Table to clear client-side column state, wires it to a new JS interop function that removes the table’s persisted state from localStorage, adjusts how column display names and widths are restored from cached state, and covers the behavior with a new unit test while doing minor cleanup in table rendering and samples. Sequence diagram for clearing table column client statussequenceDiagram
actor User
participant Table
participant JSRuntime
participant Browser
User->>Table: ClearTableColumnClientStatus()
alt ClientTableName not null or empty
Table->>JSRuntime: InvokeVoidAsync(clearColumnStates, ClientTableName)
JSRuntime->>Browser: clearColumnStates(tableName)
Browser->>Browser: localStorage.removeItem(bb-table-{tableName})
end
Table->>Table: _tableColumnStateCache.Clear()
Table->>Table: _resetColumns = true
Table->>Table: _invoke = true
Table->>Table: StateHasChanged()
File-Level Changeselements in Table.razor to avoid using the column object itself as a key
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 new TableColumnClientStatusExtensions class does not use valid C# extension syntax (
extension(TableColumnClientStatus status)); this should be rewritten as a standard static extension method (e.g.,public static void Clear(this TableColumnClientStatus status)) or removed if not needed. - The new clearTableColumnClientStatus instance method only calls the JS
clearColumnStatesfunction, which removesbb-table-{tableName}but not the existing column width key handled byremoveColumnWidthState; consider also clearing the width state (or reusingremoveColumnWidthState) so the client state is fully reset.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new TableColumnClientStatusExtensions class does not use valid C# extension syntax (`extension(TableColumnClientStatus status)`); this should be rewritten as a standard static extension method (e.g., `public static void Clear(this TableColumnClientStatus status)`) or removed if not needed.
- The new clearTableColumnClientStatus instance method only calls the JS `clearColumnStates` function, which removes `bb-table-{tableName}` but not the existing column width key handled by `removeColumnWidthState`; consider also clearing the width state (or reusing `removeColumnWidthState`) so the client state is fully reset.
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor/Extensions/TableColumnClientStatusExtensions.cs" line_range="12-14" />
<code_context>
+/// <para lang="zh">TableColumnClientStatus 扩展方法</para>
+/// <para lang="en">TableColumnClientStatus extensions</para>
+/// </summary>
+public static class TableColumnClientStatusExtensions
+{
+ extension(TableColumnClientStatus status)
+ {
+ /// <summary>
</code_context>
<issue_to_address>
**issue (bug_risk):** The extension method syntax is invalid and will not compile.
In C#, extension methods must be `public static` methods on a `public static` class, and the first parameter must be marked with `this`. The `extension(TableColumnClientStatus status)` block and nested `public void Clear()` are not valid syntax. Refactor to a proper extension method, e.g.:
```csharp
public static class TableColumnClientStatusExtensions
{
/// <summary>
/// <para lang="zh">清除方法</para>
/// <para lang="en">Clear method</para>
/// </summary>
public static void Clear(this TableColumnClientStatus status)
{
status.Columns.Clear();
status.TableWidth = 0;
}
}
```
</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
This PR adds an instance API to the Table<TItem> component to clear persisted client-side column state (localStorage) and refresh the table’s column layout, addressing #8004.
Changes:
- Added
Table<TItem>.ClearTableColumnClientStatus()to clear persisted column state and reset the table columns. - Added JS interop support (
clearColumnStates) to remove the table column-state entry fromlocalStorage. - Adjusted column-state/display-name handling and added a unit test covering the new instance method.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTest/Components/TableTest.cs | Adds a unit test validating that clearing client column state restores the configured column width. |
| src/BootstrapBlazor/Extensions/TableColumnClientStatusExtensions.cs | Introduces a TableColumnClientStatus.Clear() helper used when clearing cached state. |
| src/BootstrapBlazor/Components/Table/Table.razor.Toolbar.cs | Updates display-name assignment during visible-columns cache rebuild. |
| src/BootstrapBlazor/Components/Table/Table.razor.js | Adds clearColumnStates(tableName) JS export to delete persisted column state from localStorage. |
| src/BootstrapBlazor/Components/Table/Table.razor.cs | Adds ClearTableColumnClientStatus() and adjusts column-state construction (notably DisplayName). |
| src/BootstrapBlazor/Components/Table/Table.razor | Removes @key from the <col> loop in the <colgroup>. |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesColumnDrag.razor | Adds (commented-out) sample code intended to demonstrate the new API. |
Comments suppressed due to low confidence (2)
src/BootstrapBlazor/Components/Table/Table.razor.cs:1395
CreateTableColumnStateno longer initializesTableColumnState.DisplayName, but the column list UI renders@item.DisplayName(see Table.razor RenderColumnListItem). This makes display text null/blank for columns (especially those currently not visible) unless it’s set elsewhere. Consider settingDisplayName = col.GetDisplayName()here (or ensuring it’s populated for all_tableColumnStates, not only visible ones).
This issue also appears on line 1886 of the same file.
private TableColumnState CreateTableColumnState(ITableColumn col) => new TableColumnState()
{
Name = col.GetFieldName(),
Width = col.Width,
Visible = col.GetVisible(_screenSize)
};
src/BootstrapBlazor/Components/Table/Table.razor.cs:1900
ClearTableColumnClientStatusclears_tableColumnStateCache, but it doesn’t reset the currentColumnsmodel back to its original widths/visibility. For auto-generated/dynamic columns (InternalTableColumn) theWidth/Visiblevalues can remain mutated from the previously applied client state, so clearing may not actually restore defaults. Consider rebuilding the column definitions (e.g., re-run the column build pipeline) or keeping an “initial column state” snapshot to restoreColumnsfrom when clearing client status.
public async Task ClearTableColumnClientStatus()
{
if (!string.IsNullOrEmpty(ClientTableName))
{
// 如果启用了 ClientTableName 则清除浏览器持久化列状态
await InvokeVoidAsync("clearColumnStates", ClientTableName);
}
// 清除缓存的列状态
_tableColumnStateCache.Clear();
_resetColumns = true;
_invoke = true;
StateHasChanged();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| export function clearColumnStates(tableName) { | ||
| const columnStateKey = `bb-table-${tableName}`; | ||
| localStorage.removeItem(columnStateKey); |
Comment on lines
11
to
+38
| @@ -26,3 +28,12 @@ | |||
| </Table> | |||
| <ConsoleLogger @ref="Logger" /> | |||
| </DemoBlock> | |||
|
|
|||
| @code { | |||
| // private Table<Foo> Table { get; set; } | |||
|
|
|||
| // private async Task Reset() | |||
| // { | |||
| // await Table.ClearTableColumnClientStatus(); | |||
| // } | |||
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8005 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 765 766 +1
Lines 34118 34131 +13
Branches 4691 4692 +1
=========================================
+ Hits 34118 34131 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 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 #8004
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add support for clearing persisted table column client status and propagating that reset through the table component.
New Features:
Enhancements:
Tests: