feat(Table): add FitColumnWidthIncludeHeader parameter #6871
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
Modified the OnAutoFitContentAsync callback to accept the calculated column width as a parameter in both C# and JavaScript. This allows custom logic to consider the current width when determining the final column width, improving flexibility for auto-fit scenarios. Co-Authored-By: ljmay168 <209402714+ljmay168@users.noreply.github.com>
Modified the UnitTest
修改 单元测试宽度计算 Signed-off-by: AApuci <44996451+AApuci@users.noreply.github.com>
# Conflicts: # src/BootstrapBlazor/Components/Table/Table.razor.cs
Reviewer's guide (collapsed on small PRs)Reviewer's GuideIntroduces a new FitColumnWidthIncludeHeader toggle on the Table component to optionally include header cell width during auto-fit calculations, with corresponding updates to the JS autoFitColumnWidth logic. Class diagram for updated Table component parametersclassDiagram
class Table {
+Func<string, float, Task<float>>? OnAutoFitColumnWidthCallback
+bool FitColumnWidthIncludeHeader
}
Flow diagram for autoFitColumnWidth logic with FitColumnWidthIncludeHeaderflowchart TD
A["autoFitColumnWidth called"] --> B["Calculate maxWidth from data cells"]
B --> C{fitColumnWidthIncludeHeader?}
C -- Yes --> D["Include header cell width in maxWidth"]
C -- No --> E["Skip header cell width"]
D --> F["Invoke autoFitColumnWidthCallback if set"]
E --> F
F --> G["Set column width"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new parameter FitColumnWidthIncludeHeader to the Table component that allows including header cell width when auto-fitting column widths. This enhancement provides more control over the column width calculation behavior.
- Added
FitColumnWidthIncludeHeaderboolean parameter to control header inclusion in width calculations - Updated JavaScript auto-fit logic to consider header cell width when the parameter is enabled
- Enhanced the table options configuration to pass the new parameter to the client-side code
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Table.razor.cs | Added new boolean parameter FitColumnWidthIncludeHeader and included it in table options |
| Table.razor.js | Updated autoFitColumnWidth function to include header cell width calculation when enabled |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Add a null check around col.closest('th') in the JavaScript autoFitColumnWidth logic to prevent runtime errors when a header cell isn’t found.
- Translate or supplement the Chinese XML doc comment on FitColumnWidthIncludeHeader into English to match the surrounding codebase style.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a null check around col.closest('th') in the JavaScript autoFitColumnWidth logic to prevent runtime errors when a header cell isn’t found.
- Translate or supplement the Chinese XML doc comment on FitColumnWidthIncludeHeader into English to match the surrounding codebase style.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/Table/Table.razor.js:745-749` </location>
<code_context>
maxWidth = Math.max(maxWidth, calcCellWidth(cell));
});
+ if (table.options.fitColumnWidthIncludeHeader) {
+ const th = col.closest('th');
+ const span = th.querySelector('.table-cell');
+ maxWidth = Math.max(maxWidth, calcCellWidth(span));
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Consider null-checking 'th' and 'span' before using them.
Without null checks, calling 'calcCellWidth(span)' may cause runtime errors if the DOM structure is not as expected.
```suggestion
if (table.options.fitColumnWidthIncludeHeader) {
const th = col.closest('th');
if (th) {
const span = th.querySelector('.table-cell');
if (span) {
maxWidth = Math.max(maxWidth, calcCellWidth(span));
}
}
}
```
</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 #6871 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 739 739
Lines 31763 31765 +2
Branches 4466 4466
=========================================
+ Hits 31763 31765 +2
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:
|
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.
Link issues
fixes #6864
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add FitColumnWidthIncludeHeader parameter to Table to optionally include header cells in auto-fit column width calculations.
New Features:
Enhancements: