feat(table): fixed column not allow drag#8059
Conversation
feat(table):当表格存在固定列时,只允许相同属性列之间相互交换列顺序
|
Thanks for your PR, @Tony-ST0754. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRestricts table column drag-and-drop so that fixed and non-fixed columns can only swap order within their own group, and assigns a default width to fixed columns without an explicit Width to avoid misalignment when reordering. Sequence diagram for restricted table column drag-and-dropsequenceDiagram
actor User
participant Table as Table
participant Th as TableHeaderTh
User->>Th: dragstart
Th->>Table: setDraggable_dragstart_handler
Table->>Table: dragItem = col
Table->>Table: dragItemIsFixed = col.classList.contains('fixed')
User->>Th: dragover
Th->>Table: setDraggable_dragover_handler
Table->>Table: isOverFixed = col.classList.contains('fixed')
alt [dragItemIsFixed != isOverFixed]
Table->>Th: e.dataTransfer.dropEffect = none
Table-->>User: disallow_drop
else [dragItemIsFixed == isOverFixed]
Table->>Th: e.dataTransfer.dropEffect = move
Table-->>User: allow_drop
end
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.
Hey - I've found 1 issue, and left some high level feedback:
- In
Table.razor.js, consider resettingdragItemIsFixedin thedragendhandler (similar todragItem = null) to avoid stale state if a drag is canceled or future logic depends on this flag being accurate outside an active drag. - The condition
dragItemIsFixed != isOverFixedin thedragoverhandler could be made more explicit (e.g., by early-returning whendragItemis null and using strict equality!==) to avoid unexpected behavior if the drag state or CSS classes change. - The default-width assignment
Width = Fixed ? Width ?? 100 : Width;inOnInitializedonly runs once; ifFixedorWidthare updated dynamically after initialization, the defaulting logic will not reapply—consider moving this to a place that runs when parameters change if dynamic column configuration is expected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Table.razor.js`, consider resetting `dragItemIsFixed` in the `dragend` handler (similar to `dragItem = null`) to avoid stale state if a drag is canceled or future logic depends on this flag being accurate outside an active drag.
- The condition `dragItemIsFixed != isOverFixed` in the `dragover` handler could be made more explicit (e.g., by early-returning when `dragItem` is null and using strict equality `!==`) to avoid unexpected behavior if the drag state or CSS classes change.
- The default-width assignment `Width = Fixed ? Width ?? 100 : Width;` in `OnInitialized` only runs once; if `Fixed` or `Width` are updated dynamically after initialization, the defaulting logic will not reapply—consider moving this to a place that runs when parameters change if dynamic column configuration is expected.
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor/Components/Table/Table.razor.js" line_range="853-857" />
<code_context>
})
EventHandler.on(col, 'dragover', e => {
e.preventDefault()
+ const isOverFixed = col.classList.contains('fixed') ? true : false;
+ if (dragItemIsFixed != isOverFixed) {
+ // 表格中只允相同属性列间相互交换顺序,即fixed列之间可以交换顺序,fixed列与非fixed列不允许交换顺序
+ e.dataTransfer.dropEffect = 'none'
+ return false;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Use strict comparison and reconsider `return false` in the dragover handler.
You can simplify this to use strict comparison and avoid implicit coercion:
```js
const isOverFixed = col.classList.contains('fixed');
if (dragItemIsFixed !== isOverFixed) {
e.dataTransfer.dropEffect = 'none';
return; // or just fall through
}
```
Also, `return false` in an `addEventListener` handler has no special behavior (unlike inline HTML handlers), and `e.preventDefault()` already controls the default action, so it’s clearer to drop the boolean return.
Suggested implementation:
```javascript
dragItem = col
dragItemIsFixed = col.classList.contains('fixed')
e.dataTransfer.effectAllowed = 'move'
})
```
```javascript
EventHandler.on(col, 'dragover', e => {
e.preventDefault()
const isOverFixed = col.classList.contains('fixed')
if (dragItemIsFixed !== isOverFixed) {
// 表格中只允相同属性列间相互交换顺序,即fixed列之间可以交换顺序,fixed列与非fixed列不允许交换顺序
e.dataTransfer.dropEffect = 'none'
return
}
```
</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 #8059 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 766 766
Lines 34155 34163 +8
Branches 4697 4699 +2
=========================================
+ Hits 34155 34163 +8
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:
|
revert: 将数据列Fixed属性为true时,Column的Width默认宽度设置为200,保持与组件中DefaultFixedColumnWidth属性一致
44f2dff to
a739ed4
Compare
feat(table):当表格存在固定列时,只允许相同属性列之间相互交换列顺序
Link issues
fixes #8058
问题描述 / Problem Description
Table组件中AllowDragColumn属性为true时,且组件中存在数据列属性为Fixed为true时,目前没有控制固定列与非固定列之间不能交换顺序,这样用户在操作时,如果错误的将固定列拖拽到非固定列后面或者将非固定列拖拽到固定列前面时,整个表格组件显示会出现错位复现示例及示例代码
BootstrapBlazorApp11.zip
20260528_115849.mp4
根本原因 / Root Cause
class中fixed,导致实际固定列与非固列定交换后出现差异或错位。class中fixed添加或移除,还要关注从固定列变为非固定列后,固定列的总数量,同时还得关注固定列的最后一列的HTML元素上class中除了包含fixed外,是否还包含fr这个css类,同时还要重新处理宽度,导致逻辑过于复杂;如果,还要保持固定列与非固定列之间的交换,还会涉及到表格持久化的问题,延伸出来要改动的就多了,逻辑更复杂了。class中有没有给上fr这个css类解决方案 / Solution
Width值时,导致,计算问题出现错位,因此,在组件中列Fixed属性为true时,但未设置Width值时,组件内部会给Width默认值,且默认值为200,保持与组件中DefaultFixedColumnWidth属性默认值一致Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Restrict table column drag-and-drop reordering when fixed columns are present and assign a default width to fixed columns without an explicit width.
New Features:
Bug Fixes:
Enhancements: