-
Notifications
You must be signed in to change notification settings - Fork 331
fix(grid): [grid] fix style error when dynamic group header is set fixed #2789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis pull request involves a minor version update for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Walkthrough此PR修复了在动态插入的分组表头设置左冻结后,left样式设置不正确的问题。通过调整表头和单元格的样式设置,确保在有纵向滚动条时,表头右冻结列的定位正确。 Changes
|
| columns.forEach((column) => { | ||
| columns.forEach((column, index) => { | ||
| // 叶子节点则返回 | ||
| if (index === columns.length - 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在循环中使用index来判断是否为叶子节点可能会导致错误,因为这假设了叶子节点总是最后一个元素。建议使用更可靠的方法来识别叶子节点,以避免潜在的逻辑错误。
WalkthroughThis PR fixes the problem of incorrect left style setting after setting the left freeze in the dynamically inserted group header. By adjusting the style settings of the table header and cells, ensure that the frozen column on the right side of the table header is positioned correctly when there is a vertical scroll bar. Changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vue/src/grid/src/table/src/utils/autoCellWidth.ts (1)
179-189: Consider preserving existing styles.While the changes effectively prevent duplicate style assignments and optimize performance with early returns, the style object initialization could be safer.
Consider this approach to preserve any existing styles:
- column.style = column.style || {} + if (!finishColumns.has(column.id)) { + column.style = { ...column.style } // Preserve existing styles column.style[direction] = leafDirectionPos + finishColumns.add(column.id) + } - if (!finishColumns.has(column.id)) { - column.style[direction] = leafDirectionPos - finishColumns.add(column.id) - }This ensures we:
- Only modify styles once per column (using
finishColumns)- Preserve any existing styles by creating a new object
- Set the direction property only when needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue/src/grid/package.json(1 hunks)packages/vue/src/grid/src/table/src/utils/autoCellWidth.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/vue/src/grid/package.json
🔇 Additional comments (2)
packages/vue/src/grid/src/table/src/utils/autoCellWidth.ts (2)
163-163: LGTM! Good performance optimization.The introduction of
finishColumnsSet efficiently prevents duplicate style assignments for the same column.
Line range hint
163-189: Add tests to verify the fixes.The changes look good and effectively address the style issues. However, to ensure the fixes remain stable:
- Add unit tests for the
setGroupHeaderPositionfunction- Include test cases for:
- Single column groups (early return)
- Multiple column groups
- Duplicate style prevention
- Left/right direction handling
Would you like me to help generate the test cases?
PR
修复动态插入的分组表头设置左冻结后,left样式设置不正确的问题

修复前:
修复后:

PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Version Update
@opentiny/vue-gridpackage version from3.21.0to3.21.1Performance Improvements