fix(Table): prevent column width test from restoring pre-drag width while resizing - #8357
Open
h2ls wants to merge 1 commit into
Open
fix(Table): prevent column width test from restoring pre-drag width while resizing#8357h2ls wants to merge 1 commit into
h2ls wants to merge 1 commit into
Conversation
…hile resizing (dotnetcore#8323) * fix: 拖动调整列宽期间暂停列宽测试，防止 ResizeObserver 恢复拖动前宽度导致 colgroup > col 宽度值闪烁 * fix: 拖动结束或双击自适应后将该列移出自动测量集合，用户显式设定的列宽不再被后续列宽测试覆盖 * fix: 未实际拖动的点击不再触发列宽回调，避免误触 resizer 导致全表列宽被冻结 * fix: 拖动过程中表格重置或销毁时丢弃过期回调，防止写入无效列宽状态 close dotnetcore#8323
|
Thanks for your PR, @h2ls. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Contributor
Reviewer's GuideUpdates the table’s client-side resize lifecycle so automatic width measurement is paused during drag, excluded for explicitly sized columns, and applied once afterward before persisting state. It also prevents no-op or stale resize callbacks, covers auto-fit, and resets resize state safely without changing public APIs or storage formats. Sequence diagram for the guarded table column resize lifecyclesequenceDiagram
participant User
participant ResizeHandler
participant Table
participant ResizeObserver
participant Server
User->>ResizeHandler: drag(col)
ResizeHandler->>Table: table.resizing = true
ResizeObserver->>Table: applyColumnMinWidth(table)
Table-->>ResizeObserver: return while resizing
ResizeHandler->>Table: update column width
User->>ResizeHandler: release drag
ResizeHandler->>Table: table.resizing = false
ResizeHandler->>Table: removeAutoColumn(table, col, colIndex)
ResizeHandler->>Table: applyColumnMinWidth(table)
ResizeHandler->>Table: getColumnStateObject(table)
ResizeHandler->>Server: resizeColumnCallback(field, state)
Flow diagram for valid and stale resize completionflowchart TD
A[Resize interaction ends] --> B{Actual movement?}
B -- No --> C[Skip resizeColumnCallback]
B -- Yes --> D{Header still attached?}
D -- No --> E[Discard stale callback]
D -- Yes --> F[removeAutoColumn]
F --> G[applyColumnMinWidth once]
G --> H[getColumnStateObject]
H --> I[saveColumnStateToLocalstorage]
I --> J[resizeColumnCallback]
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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor/Components/Table/Table.razor.js" line_range="506-509" />
<code_context>
originalX = e.clientX ?? e.touches[0].clientX
},
e => {
+ resized = true;
const eventX = e.clientX ?? e.changedTouches[0].clientX
const marginX = eventX - originalX
</code_context>
<issue_to_address>
**issue (bug_risk):** `resized` becomes true for every `mousemove`/`touchmove` event, even when `eventX - originalX` is zero, so moving the pointer without changing the column width still persists column state and invokes `resizeColumnCallback`, freezing a column that was not actually resized.
**Triggers:** When a pointer move event is delivered over the resizer without horizontal displacement, such as vertical pointer movement or duplicate events with the same clientX.
**Suggested fix:** Set `resized` only when the computed horizontal margin is nonzero and the resulting width differs from the starting width.
```suggestion
const eventX = e.clientX ?? e.changedTouches[0].clientX
const marginX = eventX - originalX
let calcColWidth = colWidth + marginX;
if (calcColWidth < 5) {
calcColWidth = 5;
}
resized = marginX !== 0 && calcColWidth !== colWidth;
table.tables.forEach(t => {
const group = [...t.children].find(i => i.nodeName === 'COLGROUP')
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the resizing logic is wrong, the table can save an incorrect column width to local storage and invoke the resize callback with that state, so the bad width can persist after the code is reverted. The impact is bounded to affected table users and can be repaired by resetting or clearing the saved column state; otherwise this is an ordinary UI behavior bug.
Blocking findings: src/BootstrapBlazor/Components/Table/Table.razor.js:509
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+506
to
509
| resized = true; | ||
| const eventX = e.clientX ?? e.changedTouches[0].clientX | ||
| const marginX = eventX - originalX | ||
| table.tables.forEach(t => { |
Contributor
There was a problem hiding this comment.
issue (bug_risk): resized becomes true for every mousemove/touchmove event, even when eventX - originalX is zero, so moving the pointer without changing the column width still persists column state and invokes resizeColumnCallback, freezing a column that was not actually resized.
Triggers: When a pointer move event is delivered over the resizer without horizontal displacement, such as vertical pointer movement or duplicate events with the same clientX.
Suggested fix: Set resized only when the computed horizontal margin is nonzero and the resulting width differs from the starting width.
Suggested change
| resized = true; | |
| const eventX = e.clientX ?? e.changedTouches[0].clientX | |
| const marginX = eventX - originalX | |
| table.tables.forEach(t => { | |
| const eventX = e.clientX ?? e.changedTouches[0].clientX | |
| const marginX = eventX - originalX | |
| let calcColWidth = colWidth + marginX; | |
| if (calcColWidth < 5) { | |
| calcColWidth = 5; | |
| } | |
| resized = marginX !== 0 && calcColWidth !== colWidth; | |
| table.tables.forEach(t => { | |
| const group = [...t.children].find(i => i.nodeName === 'COLGROUP') |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in the Table fixed-header resizing experience where the auto column-width measurement (ResizeObserver-driven applyColumnMinWidth) can interfere with manual drag resizing / auto-fit, causing colgroup > col widths to flicker and stale widths to be persisted.
Changes:
- Add a
table.resizingguard soapplyColumnMinWidthwon’t run during drag resizing. - On drag end / auto-fit, remove the affected column from
autoColumnsand syncoptions.columnStates.widthso later re-measures/resets don’t restore stale widths. - Avoid calling the resize callback when the user clicks a resizer but doesn’t actually move it.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+552
to
+556
| // 拖动的列由用户显式设定宽度，移出自动测量集合，防止列宽测试恢复拖动前宽度 | ||
| removeAutoColumn(table, col, colIndex); | ||
|
|
||
| const field = getColumnName(col); | ||
| table.invoke.invokeMethodAsync(table.options.resizeColumnCallback, field, state); | ||
| // 拖动结束后列宽测试只执行一次 | ||
| applyColumnMinWidth(table); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8357 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 771 771
Lines 34579 34579
=========================================
Hits 34579 34579
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
问题
fixes #8323
固定表头表格在列内容超宽时，拖动调整列宽会导致
colgroup > col上的宽度值闪烁（偶尔有、偶尔无），且列宽测试会把拖动前测量的宽度恢复。根因
#8126 引入的列宽自适应测量（
setColSize/applyColumnMinWidth）与手动拖拽调宽相互冲突：table.body上的ResizeObserver在拖动过程中因横向滚动条出现/消失反复触发applyColumnMinWidth，把拖动前测量的minWidth写回colgroup > col，与 mousemove 设置的拖动宽度互相覆盖，表现为 col 上的值偶尔有、偶尔无；autoColumns中，后续任何 body 尺寸变化都会再次把测量值写回。修复
仅修改
src/BootstrapBlazor/Components/Table/Table.razor.js：applyColumnMinWidth入口增加table.resizing守卫，统一覆盖 ResizeObserver 与拖动中查询重渲染（setColSize）两条触发路径 —— 对应期望行为"不应再将拖动调整前的各列宽恢复"autoColumns并同步options.columnStates宽度（防止reset()时setColSize将该列重新纳入测量），再补偿执行一次applyColumnMinWidth让其余自动列落定，最后才采集状态回传服务端 —— 对应期望行为"列宽测试应该只执行一次"resizeColumnCallback：避免误触 resizer（想点排序）导致全表列宽被冻结、自适应功能静默失效；双击自适应也因此只回调一次而非三次resizing标志不泄漏兼容性
applyColumnMinWidth有thead早退保护，行为与旧版完全一致ResizeColumnCallback→UpdateTableColumnState冻结列宽的既有契约不变；OnTableColumnClientStatusChanged仅不再收到零位移的空回调测试
客户端 JS 拖拽交互无法通过 bUnit 覆盖，使用与 issue 附件一致的独立复现工程手工验证（.NET 10 + Interactive Server）：BootstrapBlazorApp1.Server.zip（release 页面）
复现条件：
IsFixedHeader+AllowResizing+ 列未设置Width+ 列内超宽内容。colgroup > col上的值，来回反复拖动不松开鼠标时值偶尔有、偶尔无，且拖动前宽度被反复恢复Summary by Sourcery
Prevent fixed-header table column resizing from restoring stale widths or persisting transient measurements.
Bug Fixes:
Enhancements: