feat(web-shell): add column reorder, resize, and freeze controls to markdown table - #6444
Conversation
Support resizing, reordering, and freezing table columns while preserving visible-order copy behavior and selection stability.
|
Thanks for the PR, @jifeng! Re-run — updating previous triage to reflect maintainer review feedback. Template looks good ✓ Problem: Feature addition — wide tables need column reorder/resize/freeze controls. Motivation is clear and the problem is real. Direction: Aligned with qwen-code's web-shell table UX. Natural extensions to existing sort/filter/hide-column feature set. Size: Not applicable — all changes in Approach: Overall approach is sound — Moving on to updated code review. 🔍 中文说明感谢 PR,@jifeng! Re-run — 更新之前的 triage 以反映 maintainer 的审查反馈。 模板完整 ✓ 问题:功能增强——多列宽表格需要列重排/调整列宽/冻结首列控件。动机清晰,问题真实存在。 方向:与 qwen-code 的 web-shell 表格 UX 一致。是现有排序/筛选/隐藏列功能的自然扩展。 规模:不适用——所有改动在 方案:整体方案合理—— 进入更新后的代码审查 🔍 — Qwen Code · qwen3.7-max |
Code Review (Re-run)Independent proposal: same as previous triage — The PR's overall architecture matches this, but the maintainer (@wenshao) identified two correctness bugs that I've now verified in the code: Critical #1: Cross-table drag accepted by
|
|
Re-run — previous approval was dismissed; maintainer @wenshao requested changes with two correctness bugs. Both Critical issues are verified and legitimate:
The overall direction and architecture are sound — column reorder/resize/freeze are the right features, and the HTML5 DnD + CSS sticky pattern is correct. The bugs are in the implementation details, not the design. Fixes should be straightforward:
Not approving — waiting for the author to address the maintainer's feedback. 中文说明Re-run — 之前的批准已被撤回;maintainer @wenshao 请求修改并提出两个正确性 bug。 两个 Critical 问题均已验证且属实:
整体方向和架构合理——列重排/调整列宽/冻结是正确的功能,HTML5 DnD + CSS sticky 模式也是对的。bug 在实现细节中,不在设计层面。修复应该是直接的:
暂不批准——等待作者处理 maintainer 的反馈。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
…e-column-controls
|
@qwen-code /triage |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
wenshao
left a comment
There was a problem hiding this comment.
Multi-agent review (8 agents: Correctness, Security, Code Quality, Performance, Test Coverage, Attacker/3AM/Maintainer)
No new Critical issues. The implementation is logically sound, tests are well-structured, and i18n is complete. Five actionable items below.
[Suggestion] Unthrottled resize triggers full re-render at ~60fps — EnhancedMarkdownTable.tsx
The mousemove handler in the resize useEffect calls setColumnWidths on every event (~60Hz) without any guard. Each state update triggers a full re-render of up to 500×50 = 25,000 <td> elements. The state updater also always creates a new object ({ ...current, [key]: nextWidth }), even when nextWidth equals the current value.
Two fixes:
- Bail out when the width hasn't changed:
setColumnWidths((current) => {
if (current[resizingColumn.columnIndex] === nextWidth) return current;
return { ...current, [resizingColumn.columnIndex]: nextWidth };
});- For further improvement, wrap in
requestAnimationFrameto cap at display refresh rate.
[Suggestion] Missing test for column width boundary conditions — EnhancedMarkdownTable.test.tsx
The resize test covers an in-range delta (+60px → 220px) but neither boundary clamp is exercised. The Math.min(MAX_COLUMN_WIDTH, Math.max(MIN_COLUMN_WIDTH, ...)) logic is untested — a regression that swaps or removes either clamp would go undetected.
Consider adding two cases:
- Drag far left (e.g.,
clientX: 0from start 100) → asserts80px - Drag far right (e.g.,
clientX: 900from start 100) → asserts640px
[Suggestion] State reset on table structure change not fully tested — EnhancedMarkdownTable.test.tsx
The source adds five new setState calls to the reset effect (setColumnWidths({}), setColumnOrder(...), setFreezeFirstColumn(false), setResizingColumn(null), setDraggingColumn(null)), but the existing reset test only asserts rowTexts(container) which reflects hidden-column reset. Column widths, column order, and freeze state are not verified.
A test that resizes, reorders, freezes, then re-renders with a different table structure and asserts default layout would close this gap.
[Nice to have] getSelectionBounds name is now misleading — EnhancedMarkdownTable.tsx
After removing minCol/maxCol, the function only returns { minRow, maxRow } but the name "bounds" implies a full 2D bounding box. Every caller now calls getSelectedColumnIndexes separately. Renaming to getSelectionRowBounds would clarify the reduced scope.
[Nice to have] draggingColumn state is redundant with DataTransfer — EnhancedMarkdownTable.tsx
The same information is already stored via setData(COLUMN_DRAG_MIME, String(columnIndex)) in startColumnDrag, and dropColumn already has a fallback through getDraggedColumnIndex(event.dataTransfer). Removing draggingColumn would eliminate two explicit reset paths (table-structure-change effect and onDragEnd), simplifying state management.
CI Failure AnalysisThe build failure in this PR is caused by a transient network issue on the GitHub Actions runner, not by any code change in this PR. Root CauseJob: The runner failed to download Job: Verdict✅ The PR code itself is fine. A simple re-run of the failed jobs should resolve this. 中文说明 (点击展开)CI 构建失败分析本 PR 的构建失败是由 GitHub Actions runner 上的临时网络问题导致的,与 PR 的代码变更无关。 根本原因Job: Runner 在从 GitHub Releases 下载 Job: 结论✅ PR 代码本身没有问题。重新运行失败的 job 即可解决。 |
What this PR does
Adds three column-level controls to the enhanced markdown table in web-shell: drag-to-reorder columns, drag-to-resize column widths, and a "Freeze first column" toggle that pins the leftmost visible data column (and the action column) with sticky positioning so they stay visible during horizontal scroll. Selection ranges and clipboard copy (TSV / quick copy / keyboard Ctrl+C) all follow the reordered column order, so copied data matches what the user sees on screen.
Why it's needed
The enhanced markdown table already supports sorting, filtering, and column hiding, but wide tables with many columns are hard to navigate — users lose context when scrolling horizontally, and the fixed column order forces them to mentally map data across distant columns. Column reordering lets users bring the columns they care about next to each other; column resizing lets them see long cell values without truncation; and freezing the first column preserves row identity while scrolling through wide datasets.
Reviewer Test Plan
How to verify
cd packages/web-shell && npx vitest run client/components/messages/EnhancedMarkdownTable.test.tsxEvidence (Before & After)
N/A — no tmux / CLI TUI changes; this is a web-shell React component. Manual browser verification is the primary evidence path.
Tested on
Environment (optional)
npm run dev(web-shell client), tested in Chrome on macOS.Risk & Scope
aria-labelbut no keyboard alternative yet.Linked Issues
demo
2026-07-07.20.45.14.mov
N/A
中文说明
本 PR 做了什么
为 web-shell 的增强 Markdown 表格新增三项列级交互控制:拖拽列头重新排序、拖拽列边缘调整列宽、以及"冻结首列"开关(使用 sticky 定位固定操作列和最左侧可见数据列,水平滚动时保持可见)。选区范围和剪贴板复制(TSV / 快捷复制 / Ctrl+C)均跟随重排后的列顺序,确保复制的数据与用户在屏幕上看到的一致。
为什么需要
增强表格已支持排序、筛选和隐藏列,但宽表格在水平滚动时容易丢失上下文,固定列顺序迫使用户在相距较远的列之间进行心理映射。列重排让用户把关心的列拉到一起;列宽调整让长单元格内容不被截断;冻结首列在浏览宽数据集时保留行身份信息。
验证方式
cd packages/web-shell && npx vitest run client/components/messages/EnhancedMarkdownTable.test.tsx风险与范围