feat(Table): support ResetColumnList method #7665
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 GuideAdds client- and server-side support for resetting the Table column list popover/dropdown mode at runtime, including a new JavaScript entry point, state tracking in the Table component, styling for popover dropdown menus, and sample/demo wiring with localization updates. Sequence diagram for resetting Table column list popover/dropdown mode at runtimesequenceDiagram
actor User
participant TablesColumnList as TablesColumnList
participant Table as TableComponent
participant JSInterop
participant TableJs as TableJsModule
participant BsDropdown as BootstrapDropdown
participant PopoverModule as PopoverModule
User->>TablesColumnList: Toggle IsPopoverToolbarDropdownButton switch
activate TablesColumnList
TablesColumnList-->>TablesColumnList: Update _isPopoverToolbarDropdownButton
TablesColumnList-->>Table: Re-render with IsPopoverToolbarDropdownButton
deactivate TablesColumnList
Table->>Table: OnAfterRenderAsync(firstRender)
alt First render
Table-->>Table: _lastIsPopoverToolbarDropdownButtonValue = IsPopoverToolbarDropdownButton
Table->>JSInterop: InvokeVoidAsync("init", Id)
JSInterop->>TableJs: init(id, invoke, options)
TableJs->>TableJs: reset(id)
else Subsequent render
Table-->>Table: Compare _lastIsPopoverPopoverToolbarDropdownButtonValue
alt Mode changed
Table-->>Table: _lastIsPopoverToolbarDropdownButtonValue = IsPopoverToolbarDropdownButton
Table->>JSInterop: InvokeVoidAsync("resetColumnList", Id)
JSInterop->>TableJs: resetColumnList(id)
activate TableJs
TableJs-->>TableJs: const table = Data.get(id)
TableJs-->>TableJs: Find toolbar.dropdown-column
TableJs->>BsDropdown: getInstance(button)
alt Dropdown instance exists
BsDropdown-->>TableJs: instance
TableJs->>BsDropdown: dispose()
end
TableJs-->>TableJs: Find existing popover for dropdown
alt Popover exists
TableJs->>PopoverModule: dispose(p)
TableJs-->>TableJs: Remove p from table.popovers
end
TableJs-->>TableJs: If button data-bs-toggle is bb.dropdown
TableJs->>PopoverModule: init(dropdown, isDisabled)
PopoverModule-->>TableJs: popoverInstance
TableJs-->>TableJs: Push popoverInstance to table.popovers
deactivate TableJs
else Mode unchanged
Table-->>Table: No JS interop call
end
end
Updated class diagram for Table component and TablesColumnList sampleclassDiagram
class TableComponent {
+bool ShowColumnListControls
+bool IsPopoverToolbarDropdownButton
-bool _lastIsPopoverToolbarDropdownButtonValue
-string DropdownListClassString
+Task OnAfterRenderAsync(bool firstRender)
+Task ProcessFirstRender()
+Task InvokeVoidAsync(string identifier, string id)
}
class TablesColumnList {
-TableComponent TableColumnVisible
-bool _isPopoverToolbarDropdownButton
-bool _showColumnListControls
+Task OnInitializedAsync()
+Task OnQueryAsync()
+Task ResetVisibleColumns()
}
class TableJsModule {
+async init(string id, object invoke, object options)
+async reset(string id)
+void saveColumnList(string tableName, object columns)
+void resetColumnList(string id)
}
class PopoverModule {
+object init(object element, object options)
+void dispose(object popover)
}
TablesColumnList --> TableComponent : configures
TableComponent --> TableJsModule : uses_JSInterop
TableJsModule --> PopoverModule : manages_popovers
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:
- In
resetColumnList(Table.razor.js),buttonis not null-checked beforebutton.getAttribute('data-bs-toggle')is called, which can throw if the.dropdown-toggleelement is missing; consider returning early or guarding that usage whenbuttonis falsy. - In
TablesColumnList.razor, theShowColumnListControls="_showColumnListControls"attribute is treated as a string literal rather than a C# expression; update it toShowColumnListControls="@_showColumnListControls"so the property is correctly bound.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `resetColumnList` (Table.razor.js), `button` is not null-checked before `button.getAttribute('data-bs-toggle')` is called, which can throw if the `.dropdown-toggle` element is missing; consider returning early or guarding that usage when `button` is falsy.
- In `TablesColumnList.razor`, the `ShowColumnListControls="_showColumnListControls"` attribute is treated as a string literal rather than a C# expression; update it to `ShowColumnListControls="@_showColumnListControls"` so the property is correctly bound.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Table/Table.razor.js:1004` </location>
<code_context>
})
}
+export function resetColumnList(id) {
+ const table = Data.get(id);
+ if (table) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying resetColumnList by using early returns for null checks and extracting the dropdown/popover reset logic into a small helper function.
You can keep the new behavior while reducing nesting and making the control flow more linear by:
1. Guarding early for missing `table`, `toolbar`, `dropdown`, or `button`.
2. Extracting the dropdown/popover reset logic into a tiny helper so it’s not inlined inside the exported function.
For example:
```js
function resetDropdownPopover(table, dropdown, button) {
const dropdownToggle = bootstrap.Dropdown.getInstance(button);
if (dropdownToggle) {
dropdownToggle.dispose();
}
const existingPopover = table.popovers.find(p => p.el === dropdown);
if (existingPopover) {
table.popovers = table.popovers.filter(p => p !== existingPopover);
Popover.dispose(existingPopover);
}
if (button.getAttribute('data-bs-toggle') === 'bb.dropdown') {
table.popovers.push(Popover.init(dropdown, {
isDisabled: () => false
}));
}
}
export function resetColumnList(id) {
const table = Data.get(id);
if (!table || !table.toolbar) return;
const dropdown = table.toolbar.querySelector('.dropdown-column');
if (!dropdown) return;
const button = dropdown.querySelector('.dropdown-toggle');
if (!button) return;
resetDropdownPopover(table, dropdown, button);
}
```
This keeps all current behavior (dispose/re-init dropdown and popover, update `table.popovers`) but:
- Flattens the control flow with early returns.
- Encapsulates the dropdown/popover reset pattern in one helper, so it’s easier to reuse wherever the column dropdown is initially set up.
- Avoids assuming `button` always exists, preventing a possible runtime error.
</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 #7665 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 749 749
Lines 33183 33196 +13
Branches 4604 4605 +1
=========================================
+ Hits 33183 33196 +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:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements a resetColumnList method for the Table component to support dynamically switching between dropdown and popover display modes for the column list toolbar button. The implementation detects when the IsPopoverToolbarDropdownButton property changes and properly reinitializes the dropdown/popover component on the client side.
Changes:
- Added client-side
resetColumnListJavaScript function to properly dispose and reinitialize the column list dropdown/popover - Implemented change detection in C# to track
IsPopoverToolbarDropdownButtonproperty changes and trigger the reset - Added CSS styling for popover mode and updated localization resources for the new feature
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/BootstrapBlazor/wwwroot/modules/base-popover.js | Removed unnecessary semicolon and restored UTF-8 BOM encoding |
| src/BootstrapBlazor/Components/Table/Table.razor.scss | Added CSS variable and styling for dropdown-menu-popover mode |
| src/BootstrapBlazor/Components/Table/Table.razor.js | Made init and reset async, added new resetColumnList export function, minor code cleanup |
| src/BootstrapBlazor/Components/Table/Table.razor.cs | Added change tracking for IsPopoverToolbarDropdownButton and logic to call resetColumnList when it changes |
| src/BootstrapBlazor.Server/Locales/zh-CN.json | Added Chinese description for the new feature |
| src/BootstrapBlazor.Server/Locales/en-US.json | Added English description for the new feature |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesColumnList.razor.cs | Added state variables for demo controls |
| src/BootstrapBlazor.Server/Components/Samples/Table/TablesColumnList.razor | Updated demo to showcase the new feature with toggle controls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/BootstrapBlazor.Server/Components/Samples/Table/TablesColumnList.razor.cs
Show resolved
Hide resolved
src/BootstrapBlazor.Server/Components/Samples/Table/TablesColumnList.razor.cs
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 #7664
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add support for resetting and re-rendering the table column list UI when its display mode changes, and update the sample to demonstrate the new behavior.
New Features:
Enhancements:
Documentation: