fix(Table): parameter maybe not work on TableColumns template#7966
Merged
Conversation
修正表格动态生成列时，如果页面中手动指定列属性 Ignore为true时，操作编辑等操作时，仍将手动指定Ignore为true的列显示
|
Thanks for your PR, @Tony-ST0754. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts table column generation to correctly honor columns marked Ignore=true during dynamic rendering, especially on subsequent renders (e.g., edit operations), by preserving manual column definitions and filtering out columns that should not be displayed. Flow diagram for updated GetTableColumns behavior with Ignore handlingflowchart TD
start[Start GetTableColumns] --> initCols[Initialize empty list cols]
initCols --> initColumns[Initialize list columns]
initColumns --> hasSource{source is not null?}
hasSource -->|yes| addSource[Add all source columns to columns]
hasSource -->|no| propsLoopStart
addSource --> propsLoopStart
propsLoopStart[Iterate over props] --> nextProp{More props?}
nextProp -->|no| afterProps
nextProp -->|yes| buildTc[Build tc from prop metadata]
buildTc --> findCol[Find col in columns with same field name as tc]
findCol --> colFound{col found?}
colFound -->|yes| copyValues[CopyValue from col to tc]
colFound -->|no| checkIgnore
copyValues --> checkIgnore[Check tc.GetIgnore]
checkIgnore --> isIgnored{tc.GetIgnore is true?}
isIgnored -->|yes| skipAdd[Do not add tc to cols]
isIgnored -->|no| addTc[Add tc to cols]
skipAdd --> propsLoopStart
addTc --> propsLoopStart
afterProps[After processing all props] --> checkColumnsCount{columns.Count > 1?}
checkColumnsCount -->|no| applyOrder
checkColumnsCount -->|yes| filterCols[Remove from cols any item not in columns]
filterCols --> applyOrder[Apply defaultOrderCallback if provided]
applyOrder --> returnCols[Return resulting cols]
returnCols --> endNode[End GetTableColumns]
File-Level Changes
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
cols.RemoveAll(x => !columns.Contains(x))logic relies on reference equality forITableColumn; if these are reconstructed objects between calls, you may want to compare by field name or another stable key instead ofContainson the interface instances. - Changing the condition from
columns.Count > 0tocolumns.Count > 1looks arbitrary and may skip the filter logic when exactly one column is passed; consider clarifying the intent or reverting to> 0if not strictly required. - Rather than leaving the old behavior commented out and adding a long inline comment block, consider refactoring the post-processing into a small helper with a clear name and a concise summary comment to keep this method easier to follow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `cols.RemoveAll(x => !columns.Contains(x))` logic relies on reference equality for `ITableColumn`; if these are reconstructed objects between calls, you may want to compare by field name or another stable key instead of `Contains` on the interface instances.
- Changing the condition from `columns.Count > 0` to `columns.Count > 1` looks arbitrary and may skip the filter logic when exactly one column is passed; consider clarifying the intent or reverting to `> 0` if not strictly required.
- Rather than leaving the old behavior commented out and adding a long inline comment block, consider refactoring the post-processing into a small helper with a clear name and a concise summary comment to keep this method easier to follow.
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor/Utils/Utility.cs" line_range="566" />
<code_context>
+ * 2、等用户操作编辑等动作时，再次渲染传进来的 source 列集合中，并没有手写的含 Ignore 属性的列，这直接导致了二次渲染时，将不应该显示的列给显示了
+ * 3、这里直接将即将返回的列集合，与二次渲染的列集合进行比较，将不存在于二次渲染列集合中的列给排除掉，以保证渲染表格时列集合正确
+ */
+ cols.RemoveAll(x => !columns.Contains(x));
}
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider using a more explicit equality strategy and/or a `HashSet` when filtering `cols` by `columns`.
`columns.Contains(x)` depends on the default equality for `ITableColumn`, which may be reference-based. If column identity is defined by a property (e.g., `Field`/`PropertyName`), this can filter incorrectly when distinct instances represent the same logical column. Also, repeated `Contains` on a list is O(n²) for large `cols`/`columns`. Consider building a `HashSet` from the column key for lookups, or providing an `IEqualityComparer<ITableColumn>` that matches the intended identity.
Suggested implementation:
```csharp
/*
* 1、动态生成列时，如果有手写 Ignore 属性的列，并没有添加到 cols 中，这样在编辑或其他操作时，在页面上手动 Ignore 属性的列会被删除
* 2、等用户操作编辑等动作时，再次渲染传进来的 source 列集合中，并没有手写的含 Ignore 属性的列，这直接导致了二次渲染时，将不应该显示的列给显示了
* 3、这里直接将即将返回的列集合，与二次渲染的列集合进行比较，将不存在于二次渲染列集合中的列给排除掉，以保证渲染表格时列集合正确
*
* 使用列标识（例如 PropertyName）构建 HashSet，避免依赖引用相等以及 O(n²) 的 List.Contains 调用。
*/
var columnKeySet = new HashSet<string>(
columns
.Where(c => c != null)
.Select(c => c.PropertyName)
.Where(name => !string.IsNullOrEmpty(name)),
StringComparer.Ordinal
);
cols.RemoveAll(x =>
x == null ||
string.IsNullOrEmpty(x.PropertyName) ||
!columnKeySet.Contains(x.PropertyName)
);
}
```
1. This edit assumes `ITableColumn` (or whatever the element type of `columns`/`cols` is) exposes a stable identity via a `PropertyName` property. If your column identity is represented differently (e.g., `Field`, `FieldName`, `ColumnName`, etc.), replace `PropertyName` with that property.
2. If there is an existing utility or comparer in your codebase that defines column equality (e.g., `ITableColumnComparer` or a specific `IEqualityComparer<ITableColumn>`), you can instead build the `HashSet` on that key or use `new HashSet<ITableColumn>(columns, yourComparer)` and update the `RemoveAll` predicate accordingly.
3. If `columns` and `cols` are strongly typed as a more specific type (e.g., `TableColumn`), consider changing the `HashSet<string>`/`PropertyName` access to whatever the canonical key is used elsewhere in `Utility.cs` or in the table component, to keep identity handling consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7966 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 765 765
Lines 34112 34124 +12
Branches 4683 4685 +2
=========================================
+ Hits 34112 34124 +12
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:
|
# Conflicts: # src/BootstrapBlazor/Components/Table/TableColumn.cs
ArgoZhang
changed the title
ArgoZhang
approved these changes
May 11, 2026
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
2 participants
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.
修正表格动态生成列时，如果页面中手动指定列属性 Ignore为true时，操作编辑等操作时，仍将手动指定Ignore为true的列显示
PS:尴尬，单元测试少了
ColumnVisibleItem这个类找不到，看了下拉下来的源码文件，少了ColumnVisibleItem.cs这个文件Link issues
fixes #7965
问题描述 / Problem Description
修正近期Bate版表格组件中，动态生成列时且页面有手动指定的列属性
Ignore=true时，页面初次加载能正确显示，但当用户操作编辑等操作时，原先手动指定列属性Ignore=true的列会被组件忽略并显示出来复现示例 / Steps To Reproduce
20260511_102042.mp4
根本原因 / Root Cause
解决方案 / Solution
20260511_102042.mp4
根本原因 / Root Cause