feat(Table): add ShowSearchInColumnList parameter #7346
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
|
Thanks for your PR, @momijijin. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Contributor
Reviewer's GuideAdds an optional 'column list select extension' feature to the Table column chooser, including search, bulk select/clear/invert behavior, and localized warning toasts when all columns would be hidden, plus minimal styling updates and a new enabling parameter. Sequence diagram for column list bulk visibility toggle with warning toastsequenceDiagram
actor User
participant UI_Dropdown as UI_ColumnDropdown
participant Table as Table_TItem
User->>UI_Dropdown: Click column list button
UI_Dropdown->>User: Show column list with select extension
User->>UI_Dropdown: Uncheck master checkbox
UI_Dropdown->>Table: OnToggleAllColumnsVisibleState(UnChecked, empty)
alt state is UnChecked
loop for each column except first
Table->>Table: Set column.Visible = false
Table->>Table: OnToggleColumnVisible(column.Name, false)
end
Table->>Table: ShowToastAsync(ColumnGroupSelectButtonWarnToastTitle,
Table->>Table: ColumnGroupSelectButtonWarnToastContent, Warning)
end
Table-->>UI_Dropdown: InvokeAsync(StateHasChanged)
UI_Dropdown->>User: Update column list and show warning toast
Class diagram for updated Table_TItem column list select extensionclassDiagram
class Table_TItem {
+bool ShowColumnList
+bool ShowColumnListSelectExtension
+string ColumnGroupSelectButtonWarnToastTitle
+string ColumnGroupSelectButtonWarnToastContent
-string _visibleColumnsSearchKey
-List_ColumnVisibleItem _visibleColumns
-List_ColumnVisibleItem _visibleColumnsSearchResult
+CheckboxState get_VisibleColumnsCurrentSelectedResult()
+List_ColumnVisibleItem get_VisibleColumnsSearchResult()
+Task SearchVisibleColumns(string searchKey)
+Task InverseSelected()
+Task OnToggleAllColumnsVisibleState(CheckboxState state, string columnName)
+Task OnToggleColumnVisible(string columnName, bool visible)
+Task ShowToastAsync(string title, string content, ToastCategory category)
-void OnInitLocalization()
}
class ColumnVisibleItem {
+string Name
+string DisplayName
+bool Visible
}
class CheckboxState {
<<enumeration>>
Checked
UnChecked
Indeterminate
}
class ToastCategory {
<<enumeration>>
Info
Success
Warning
Error
}
class Checkbox {
+CheckboxState State
+Task OnStateChanged(CheckboxState state, string value)
}
class BootstrapInputGroup
class BootstrapInput {
+string Value
+bool IsClearable
+bool UseInputEvent
+Task OnValueChanged(string value)
}
class Button {
+string Icon
+string TooltipText
+string TooltipTrigger
+Color Color
+Task OnClick()
}
class Color {
<<enumeration>>
Primary
Secondary
Success
Danger
Warning
Info
Light
Dark
}
Table_TItem "1" o-- "*" ColumnVisibleItem : manages
Table_TItem --> CheckboxState : uses
Table_TItem --> ToastCategory : uses
Table_TItem --> Checkbox : binds_state
Table_TItem --> BootstrapInput : binds_search
Table_TItem --> Button : binds_inverse
Checkbox --> CheckboxState : exposes_state
File-Level Changes
Assessment against 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 there - I've reviewed your changes - here's some feedback:
- When preventing all columns from being hidden in
InverseSelected, you set_visibleColumns[0].Visible = truebut don't callOnToggleColumnVisiblefor that column, which may leave the table's internal state or consumers out of sync with the UI. - The search cache
_visibleColumnsSearchResultis only updated insideSearchVisibleColumns; if_visibleColumnschanges (e.g., columns added/removed or renamed) while a search is active, the filtered list can become stale—consider recomputing the search result when the underlying columns collection changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When preventing all columns from being hidden in `InverseSelected`, you set `_visibleColumns[0].Visible = true` but don't call `OnToggleColumnVisible` for that column, which may leave the table's internal state or consumers out of sync with the UI.
- The search cache `_visibleColumnsSearchResult` is only updated inside `SearchVisibleColumns`; if `_visibleColumns` changes (e.g., columns added/removed or renamed) while a search is active, the filtered list can become stale—consider recomputing the search result when the underlying columns collection changes.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Table/Table.razor.Checkbox.cs:225` </location>
<code_context>
+ await ShowToastAsync(
+ ColumnGroupSelectButtonWarnToastTitle,
+ ColumnGroupSelectButtonWarnToastContent, ToastCategory.Warning);
+ _visibleColumns[0].Visible = true;
+ }
+ await InvokeAsync(StateHasChanged);
</code_context>
<issue_to_address>
**issue (bug_risk):** Restoring visibility for the first column doesn’t notify `OnToggleColumnVisible`.
In `InverseSelected`, when all columns are unchecked you set `_visibleColumns[0].Visible = true` but don’t call `OnToggleColumnVisible`. This can leave any external state that relies on that callback out of sync for the first column. Consider invoking `await OnToggleColumnVisible(_visibleColumns[0].Name, true);` after updating `Visible`.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/Table/Table.razor.Checkbox.cs:177` </location>
<code_context>
}
}
+
+ /// <summary>
+ /// 当前所有列的是否全都显示/全都不显示/部分显示状态计算
+ /// </summary>
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new column-visibility logic by removing redundant state, clarifying conditionals, extracting shared helpers, and centralizing the "at least one visible" rule.
You can simplify the new logic and remove some state/branching without changing behavior.
### 1. Remove redundant `_visibleColumnsSearchResult` state
You don’t need both `_visibleColumnsSearchKey` and `_visibleColumnsSearchResult`. You can compute the filtered list from `_visibleColumns` + key directly:
```csharp
private string _visibleColumnsSearchKey = "";
private IEnumerable<ColumnVisibleItem> VisibleColumnsSearchResult
=> string.IsNullOrWhiteSpace(_visibleColumnsSearchKey)
? _visibleColumns
: _visibleColumns.Where(r =>
(r.DisplayName ?? r.Name).Contains(_visibleColumnsSearchKey));
private Task SearchVisibleColumns(string searchKey)
{
_visibleColumnsSearchKey = searchKey;
return InvokeAsync(StateHasChanged);
}
```
This removes one source of truth and avoids keeping a second list in sync.
### 2. Replace nested ternary with clearer logic
`VisibleColumnsCurrentSelectedResult` is currently a nested ternary, which is harder to read. A small helper or an explicit property body is simpler:
```csharp
private CheckboxState VisibleColumnsCurrentSelectedResult
{
get
{
if (_visibleColumns.All(r => r.Visible))
return CheckboxState.Checked;
if (_visibleColumns.All(r => !r.Visible))
return CheckboxState.UnChecked;
return CheckboxState.Indeterminate;
}
}
```
Same behavior, easier to extend and debug.
### 3. Extract shared “toggle all” behavior
`InverseSelected` and `OnToggleAllColumnsVisibleState` both iterate `_visibleColumns`, toggle visibility, and call `OnToggleColumnVisible`. You can centralize this pattern:
```csharp
private async Task SetColumnsVisible(IEnumerable<ColumnVisibleItem> columns, bool visible)
{
foreach (var column in columns)
{
column.Visible = visible;
await OnToggleColumnVisible(column.Name, visible);
}
}
```
Then use it:
```csharp
private async Task InverseSelected()
{
foreach (var column in _visibleColumns)
{
column.Visible = !column.Visible;
await OnToggleColumnVisible(column.Name, column.Visible);
}
EnsureAtLeastOneColumnVisible();
await InvokeAsync(StateHasChanged);
}
private async Task OnToggleAllColumnsVisibleState(CheckboxState state, string _)
{
if (state == CheckboxState.Checked)
{
await SetColumnsVisible(_visibleColumns, true);
}
else if (state == CheckboxState.UnChecked)
{
await ShowToastAsync(
ColumnGroupSelectButtonWarnToastTitle,
ColumnGroupSelectButtonWarnToastContent,
ToastCategory.Warning);
await SetColumnsVisible(_visibleColumns.Skip(1), false);
}
EnsureAtLeastOneColumnVisible();
await InvokeAsync(StateHasChanged);
}
```
### 4. Encapsulate “at least one visible” rule
Instead of inlining the “must keep one visible” logic in different places, encapsulate it:
```csharp
private void EnsureAtLeastOneColumnVisible()
{
if (_visibleColumns.All(c => !c.Visible) && _visibleColumns.Any())
{
_visibleColumns[0].Visible = true;
}
}
```
You can call `EnsureAtLeastOneColumnVisible()` after bulk operations or inversions, and keep the toast behavior where it is today. This keeps the constraint in one place and simplifies future changes to the rule.
</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 #7346 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 748 748
Lines 32764 32793 +29
Branches 4546 4551 +5
=========================================
+ Hits 32764 32793 +29
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:
|
ArgoZhang
changed the title
ArgoZhang
approved these changes
Dec 21, 2025
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 #7345
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add optional extended column selection capabilities to the table column list dropdown, including bulk visibility controls, search, and localization support.
New Features:
Enhancements: