Skip to content

fix(Table): toggle column not work after resize column width #7726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ArgoZhang merged 5 commits into from
Feb 25, 2026
Merged

fix(Table): toggle column not work after resize column width #7726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
252c2b3
refactor: 移除 return 关键字
ArgoZhang Feb 25, 2026
ed00a5a
refactor: 格式化代码
ArgoZhang Feb 25, 2026
d18a5e3
refactor: 增加列默认值
ArgoZhang Feb 25, 2026
401b2f5
chore: bump version 10.4.0-beta01
ArgoZhang Feb 25, 2026
cb5bb04
fix: 修复逻辑错误
ArgoZhang Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/BootstrapBlazor/BootstrapBlazor.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk.Razor">

<PropertyGroup>
<Version>10.3.3</Version>
<Version>10.4.0-beta01</Version>
</PropertyGroup>

<ItemGroup>
Expand Down
14 changes: 10 additions & 4 deletions src/BootstrapBlazor/Components/Table/Table.razor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export async function init(id, invoke, options) {

export function saveColumnList(tableName, columns) {
const key = `bb-table-column-visiable-${tableName}`
return localStorage.setItem(key, JSON.stringify(columns));
localStorage.setItem(key, JSON.stringify(columns));
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): saveColumnList no longer returns a value, which may break any code relying on its return

Removing the return changes the function’s signature and can break any callers that use saveColumnList as an expression (e.g., in a return or condition), even though localStorage.setItem returns undefined. If this change is intentional, either keep the return for backward compatibility or audit/update all call sites to treat this as a side-effect-only function.

}

export function reloadColumnList(tableName) {
Expand Down Expand Up @@ -432,7 +432,7 @@ const setExcelKeyboardListener = table => {
}

const setFocus = target => {
const handler = setTimeout(function () {
const handler = setTimeout(function() {
clearTimeout(handler);
if (target.focus) {
target.focus();
Expand Down Expand Up @@ -541,9 +541,15 @@ const resetTableWidth = table => {
if (group) {
let width = 0;
[...group.children].forEach(col => {
width += parseInt(col.style.width)
let colWidth = parseInt(col.style.width);
if (isNaN(colWidth)) {
colWidth = 100;
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded default width of 100 for columns with NaN width is inconsistent with the system's ColumnMinWidth setting which defaults to 64. Consider using table.options.columnMinWidth instead of the hardcoded 100 to maintain consistency with the rest of the codebase.

Suggested change
colWidth = 100;
const minWidth = table.options && typeof table.options.columnMinWidth === 'number'
? table.options.columnMinWidth
: 100;
colWidth = minWidth;

Copilot uses AI. Check for mistakes.
}
width += colWidth;
})
t.style.width = `${width}px`
t.style.width = `${width}px`;

saveColumnWidth(table);
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions src/BootstrapBlazor/Dynamic/DataTableDynamicContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,14 @@ private static bool GetShownColumns(ITableColumn col, IEnumerable<string>? invis
ret = false;
}

// 隐藏列优先 移除隐藏列
// 隐藏列存在时隐藏列
if (ret && hiddenColumns != null && hiddenColumns.Any(c => c.Equals(columnName, StringComparison.OrdinalIgnoreCase)))
{
col.Visible = false;
}

// 显示列不存在时 不显示
Comment on lines -99 to -105
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Hidden vs shown column precedence appears to have changed and may now contradict the comment

Previously, if a column was in both hiddenColumns and shownColumns, it stayed hidden because the hidden check executed first and the shown logic only hid columns not in the shown list. With the new logic, such a column becomes visible, so shown now overrides hidden, which conflicts with the original intent "隐藏列优先" (hidden columns take priority). Please either adjust the conditions/order so hidden still wins, or update the comment to reflect the new precedence explicitly.

if (ret && shownColumns != null && !shownColumns.Any(c => c.Equals(columnName, StringComparison.OrdinalIgnoreCase)))
// 显示列存在时显示列
if (ret && shownColumns != null && shownColumns.Any(c => c.Equals(columnName, StringComparison.OrdinalIgnoreCase)))
{
col.Visible = true;
}
Comment on lines +105 to 109
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing test GetShownColumns_Ok only verifies the count of columns but doesn't verify that the Visible property is set correctly on each column. Consider enhancing this test to explicitly check that columns in shownColumns have Visible=true and columns in hiddenColumns have Visible=false. This would provide better coverage for the logic fix in this PR.

Copilot uses AI. Check for mistakes.
Comment on lines +105 to 109
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix corrects a critical logic bug where the condition was inverted. However, there's a potential issue with the interaction between hiddenColumns and shownColumns. If a column is in both hiddenColumns and shownColumns, the current code will first set Visible = false (line 102), then immediately set Visible = true (line 108). This means shownColumns takes precedence over hiddenColumns. Consider whether this precedence order is intentional and whether it should be documented in the XML comments.

Copilot uses AI. Check for mistakes.
Expand Down