Skip to content

fix(Table): update resetTableWidth method#7836

Merged
ArgoZhang merged 6 commits intomainfrom
fix-table-js
Apr 3, 2026
Merged

fix(Table): update resetTableWidth method#7836
ArgoZhang merged 6 commits intomainfrom
fix-table-js

Conversation

@ArgoZhang
Copy link
Copy Markdown
Member

@ArgoZhang ArgoZhang commented Apr 3, 2026

Link issues

fixes #7835

Summary By Copilot

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Merge the latest code from the main branch

Summary by Sourcery

Adjust table width recalculation to use rendered column widths and support fixed-header layouts when resetting and initializing table widths.

Bug Fixes:

  • Fix table width reset logic to avoid incorrect widths when columns lack explicit widths or when using fixed-header tables.

Enhancements:

  • Introduce shared utilities to compute total table width from colgroups based on rendered header/body cell widths, with configurable defaults.

Copilot AI review requested due to automatic review settings April 3, 2026 14:53
@bb-auto bb-auto bot added the bug Something isn't working label Apr 3, 2026
@bb-auto bb-auto bot added this to the v10.5.0 milestone Apr 3, 2026
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai bot commented Apr 3, 2026

Reviewer's Guide

Refactors table width recalculation to derive widths from rendered column groups and cells, adds helpers to handle fixed-header colgroups and column width fallback logic, and slightly re-indents a Checkbox render fragment in the Razor markup.

Flow diagram for getColumnRenderWidth fallback logic

flowchart TD
    A[getColumnRenderWidth table, col, index, defaultWidth] --> B[Parse width from col.style.width]
    B --> C{width is a valid positive number?}
    C -- Yes --> D[Return width]
    C -- No --> E[Get header = thead > tr:last-child > th at index]
    E --> F[width = header.offsetWidth or 0]
    F --> G{width > 0?}
    G -- Yes --> H[Return width]
    G -- No --> I[Find first tbody tr not having class is-detail]
    I --> J[Get cell = row.children.item index]
    J --> K[width = cell.offsetWidth or 0]
    K --> L{width > 0?}
    L -- Yes --> M[Return width]
    L -- No --> N[Return defaultWidth]
Loading

File-Level Changes

Change Details Files
Adjust table width reset logic to compute width from colgroups and rendered cells, respect scrollWidth, and clear explicit width when not needed.
  • Destructure scrollWidth from table options at the start of resetTableWidth.
  • Replace manual summation of col.style.width with a call to getTableWidthByColumnGroup using a 100px default width.
  • Before measuring, obtain the correct colgroup via getLastColgroup and clear its inline style to allow natural sizing.
  • Only set an explicit table.style.width when the computed width exceeds the parent width plus scrollWidth; otherwise clear the inline width.
  • Keep saving column widths after recalculating widths.
src/BootstrapBlazor/Components/Table/Table.razor.js
Introduce shared helpers to compute effective column widths based on colgroups, header cells, and body rows, and to select the appropriate colgroup for fixed-header tables.
  • Add getLastColgroup to choose the correct colgroup element depending on whether the parent has the table-fixed-header class.
  • Add getTableWidthByColumnGroup to sum effective column widths from all col elements in colgroups.
  • Add getColumnRenderWidth to resolve a column's width from its inline style, corresponding header cell, first non-detail body row cell, or a default width fallback.
  • Update setTableDefaultWidth to reuse getTableWidthByColumnGroup instead of duplicating column width summation logic.
src/BootstrapBlazor/Components/Table/Table.razor.js
Tidy Checkbox render fragment formatting in the table column visibility dropdown.
  • Re-indent Checkbox attributes for consistent vertical alignment in the RenderColumnListItem fragment.
src/BootstrapBlazor/Components/Table/Table.razor

Assessment against linked issues

Issue Objective Addressed Explanation
#7835 Update the Table component’s resetTableWidth logic so that after column resizing and toggling column visibility, the table width is recalculated correctly and fills (or matches) the parent container width.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@ArgoZhang ArgoZhang merged commit be6a36a into main Apr 3, 2026
6 checks passed
@ArgoZhang ArgoZhang deleted the fix-table-js branch April 3, 2026 14:54
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In getLastColgroup, the length - 2/length - 1 indexing assumes a minimum number of col elements; consider guarding against length < 2 to avoid returning undefined or accessing out-of-bounds when table markup changes.
  • getColumnRenderWidth repeatedly queries thead and tbody for each column; if this runs frequently, you may want to cache the header row and a representative body row outside the loop and pass them in to avoid repeated DOM queries.
  • In resetTableWidth, colgroup.style = null wipes all inline styles; if there are non-width-related styles that need to be preserved, consider clearing only colgroup.style.width instead of resetting the whole style object.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `getLastColgroup`, the `length - 2`/`length - 1` indexing assumes a minimum number of `col` elements; consider guarding against `length < 2` to avoid returning `undefined` or accessing out-of-bounds when table markup changes.
- `getColumnRenderWidth` repeatedly queries `thead` and `tbody` for each column; if this runs frequently, you may want to cache the header row and a representative body row outside the loop and pass them in to avoid repeated DOM queries.
- In `resetTableWidth`, `colgroup.style = null` wipes all inline styles; if there are non-width-related styles that need to be preserved, consider clearing only `colgroup.style.width` instead of resetting the whole style object.

## Individual Comments

### Comment 1
<location path="src/BootstrapBlazor/Components/Table/Table.razor.js" line_range="1067-1088" />
<code_context>
     }));
 }

+const getTableWidthByColumnGroup = (table, defaultWidth) => {
+    return [...table.querySelectorAll('colgroup col')]
+        .map((col, index) => getColumnRenderWidth(table, col, index, defaultWidth))
+        .reduce((accumulator, val) => accumulator + val, 0);
+}
+
+const getColumnRenderWidth = (table, col, index, defaultWidth) => {
+    let width = parseFloat(col.style.width);
+    if (!isNaN(width) && width > 0) {
+        return width;
+    }
+
+    const header = table.querySelectorAll('thead > tr:last-child > th').item(index);
+    width = header?.offsetWidth ?? 0;
+    if (width > 0) {
+        return width;
+    }
+
+    const row = [...table.querySelectorAll('tbody > tr')].find(i => !i.classList.contains('is-detail'));
+    width = row?.children.item(index)?.offsetWidth ?? 0;
+    return width > 0 ? width : defaultWidth;
+}
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid repeated DOM queries inside width calculations for better performance.

`getTableWidthByColumnGroup` / `getColumnRenderWidth` re-run `querySelectorAll` on `colgroup col`, `thead`, and `tbody` for every column, which can be costly on large tables or during frequent resizes. Consider querying these once (e.g. `const cols = ...`, `const headers = ...`, a cached non-detail body row) and passing them into `getColumnRenderWidth`, or at least moving the header/body queries outside the per-column loop.

```suggestion
const getTableWidthByColumnGroup = (table, defaultWidth) => {
    const cols = [...table.querySelectorAll('colgroup col')];
    const headers = table.querySelectorAll('thead > tr:last-child > th');
    const bodyRows = table.querySelectorAll('tbody > tr');
    const dataRow = [...bodyRows].find(row => !row.classList.contains('is-detail'));

    return cols
        .map((col, index) => getColumnRenderWidth(col, index, defaultWidth, headers, dataRow))
        .reduce((accumulator, val) => accumulator + val, 0);
}

const getColumnRenderWidth = (col, index, defaultWidth, headers, dataRow) => {
    let width = parseFloat(col.style.width);
    if (!isNaN(width) && width > 0) {
        return width;
    }

    const header = headers.item(index);
    width = header?.offsetWidth ?? 0;
    if (width > 0) {
        return width;
    }

    const cell = dataRow?.children.item(index);
    width = cell?.offsetWidth ?? 0;

    return width > 0 ? width : defaultWidth;
}
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +1067 to +1088
const getTableWidthByColumnGroup = (table, defaultWidth) => {
return [...table.querySelectorAll('colgroup col')]
.map((col, index) => getColumnRenderWidth(table, col, index, defaultWidth))
.reduce((accumulator, val) => accumulator + val, 0);
}

const getColumnRenderWidth = (table, col, index, defaultWidth) => {
let width = parseFloat(col.style.width);
if (!isNaN(width) && width > 0) {
return width;
}

const header = table.querySelectorAll('thead > tr:last-child > th').item(index);
width = header?.offsetWidth ?? 0;
if (width > 0) {
return width;
}

const row = [...table.querySelectorAll('tbody > tr')].find(i => !i.classList.contains('is-detail'));
width = row?.children.item(index)?.offsetWidth ?? 0;
return width > 0 ? width : defaultWidth;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (performance): Avoid repeated DOM queries inside width calculations for better performance.

getTableWidthByColumnGroup / getColumnRenderWidth re-run querySelectorAll on colgroup col, thead, and tbody for every column, which can be costly on large tables or during frequent resizes. Consider querying these once (e.g. const cols = ..., const headers = ..., a cached non-detail body row) and passing them into getColumnRenderWidth, or at least moving the header/body queries outside the per-column loop.

Suggested change
const getTableWidthByColumnGroup = (table, defaultWidth) => {
return [...table.querySelectorAll('colgroup col')]
.map((col, index) => getColumnRenderWidth(table, col, index, defaultWidth))
.reduce((accumulator, val) => accumulator + val, 0);
}
const getColumnRenderWidth = (table, col, index, defaultWidth) => {
let width = parseFloat(col.style.width);
if (!isNaN(width) && width > 0) {
return width;
}
const header = table.querySelectorAll('thead > tr:last-child > th').item(index);
width = header?.offsetWidth ?? 0;
if (width > 0) {
return width;
}
const row = [...table.querySelectorAll('tbody > tr')].find(i => !i.classList.contains('is-detail'));
width = row?.children.item(index)?.offsetWidth ?? 0;
return width > 0 ? width : defaultWidth;
}
const getTableWidthByColumnGroup = (table, defaultWidth) => {
const cols = [...table.querySelectorAll('colgroup col')];
const headers = table.querySelectorAll('thead > tr:last-child > th');
const bodyRows = table.querySelectorAll('tbody > tr');
const dataRow = [...bodyRows].find(row => !row.classList.contains('is-detail'));
return cols
.map((col, index) => getColumnRenderWidth(col, index, defaultWidth, headers, dataRow))
.reduce((accumulator, val) => accumulator + val, 0);
}
const getColumnRenderWidth = (col, index, defaultWidth, headers, dataRow) => {
let width = parseFloat(col.style.width);
if (!isNaN(width) && width > 0) {
return width;
}
const header = headers.item(index);
width = header?.offsetWidth ?? 0;
if (width > 0) {
return width;
}
const cell = dataRow?.children.item(index);
width = cell?.offsetWidth ?? 0;
return width > 0 ? width : defaultWidth;
}

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (dd73301) to head (b759996).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #7836   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          764       764           
  Lines        34147     34147           
  Branches      4701      4701           
=========================================
  Hits         34147     34147           
Flag Coverage Δ
BB 100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Table component’s client-side width reset logic to address incorrect table width after resizing columns and then toggling column visibility (issue #7835), aiming to ensure the table properly fills its parent container.

Changes:

  • Reworked resetTableWidth to conditionally clear/set the table width based on computed column render widths and available container width.
  • Added helpers to compute table width using colgroup + rendered header/body cell widths as fallbacks.
  • Bumped BootstrapBlazor package version to 10.5.1-beta02 (plus a small Razor formatting adjustment).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/BootstrapBlazor/Components/Table/Table.razor.js Updates/reset logic for table width and adds width-calculation helpers.
src/BootstrapBlazor/Components/Table/Table.razor Minor formatting change in the column visibility dropdown item.
src/BootstrapBlazor/BootstrapBlazor.csproj Version bump to 10.5.1-beta02.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

t.style.width = `${width}px`;
const colgroup = getLastColgroup(t, group);
if (colgroup) {
colgroup.style = null;
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

colgroup.style = null attempts to assign to the DOM style property (a read-only CSSStyleDeclaration in ES modules/strict mode) and can throw a TypeError. If the intent is to clear the last column's inline width, use colgroup.style.removeProperty('width'), set colgroup.style.width = '', or colgroup.removeAttribute('style') (depending on what needs to be cleared).

Suggested change
colgroup.style = null;
colgroup.style.width = '';

Copilot uses AI. Check for mistakes.
t.style.width = `${width}px`;
}
else {
t.style.width = null;
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

Using t.style.width = null is unreliable because the setter coerces null to a string and may not remove the style (potentially leaving a stale width). Use t.style.removeProperty('width') (or set t.style.width = '') to explicitly clear the inline width when the table should auto-size to its container.

Suggested change
t.style.width = null;
t.style.removeProperty('width');

Copilot uses AI. Check for mistakes.
Comment on lines +560 to +569
const getLastColgroup = (table, group) => {
const p = table.parentElement;
if (p) {
const length = group.children.length;
if (p.classList.contains("table-fixed-header")) {
return group.children[length - 2];
}
else {
return group.children[length - 1];
}
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

getLastColgroup returns a col element (a child of the COLGROUP), not a COLGROUP. The current name/variable (colgroup) is misleading and makes the logic harder to follow. Consider renaming to something like getLastColElement / getLastDataCol and renaming the local variable accordingly.

Copilot uses AI. Check for mistakes.
Comment on lines +1068 to +1085
return [...table.querySelectorAll('colgroup col')]
.map((col, index) => getColumnRenderWidth(table, col, index, defaultWidth))
.reduce((accumulator, val) => accumulator + val, 0);
}

const getColumnRenderWidth = (table, col, index, defaultWidth) => {
let width = parseFloat(col.style.width);
if (!isNaN(width) && width > 0) {
return width;
}

const header = table.querySelectorAll('thead > tr:last-child > th').item(index);
width = header?.offsetWidth ?? 0;
if (width > 0) {
return width;
}

const row = [...table.querySelectorAll('tbody > tr')].find(i => !i.classList.contains('is-detail'));
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

getTableWidthByColumnGroup/getColumnRenderWidth calls querySelectorAll for the header and searches tbody rows once per column, which scales poorly with many columns (O(n^2) DOM queries). Consider caching the header NodeList and the first non-detail row once per table-width calculation and passing them into getColumnRenderWidth to reduce repeated DOM work.

Suggested change
return [...table.querySelectorAll('colgroup col')]
.map((col, index) => getColumnRenderWidth(table, col, index, defaultWidth))
.reduce((accumulator, val) => accumulator + val, 0);
}
const getColumnRenderWidth = (table, col, index, defaultWidth) => {
let width = parseFloat(col.style.width);
if (!isNaN(width) && width > 0) {
return width;
}
const header = table.querySelectorAll('thead > tr:last-child > th').item(index);
width = header?.offsetWidth ?? 0;
if (width > 0) {
return width;
}
const row = [...table.querySelectorAll('tbody > tr')].find(i => !i.classList.contains('is-detail'));
const headers = table.querySelectorAll('thead > tr:last-child > th');
const rows = table.querySelectorAll('tbody > tr');
const row = [...rows].find(i => !i.classList.contains('is-detail'));
return [...table.querySelectorAll('colgroup col')]
.map((col, index) => getColumnRenderWidth(col, index, defaultWidth, headers, row))
.reduce((accumulator, val) => accumulator + val, 0);
}
const getColumnRenderWidth = (col, index, defaultWidth, headers, row) => {
let width = parseFloat(col.style.width);
if (!isNaN(width) && width > 0) {
return width;
}
const header = headers.item(index);
width = header?.offsetWidth ?? 0;
if (width > 0) {
return width;
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(Table): update resetTableWidth method

2 participants