-
Notifications
You must be signed in to change notification settings - Fork 330
fix(grid): fix body height error after change pager size #3753
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
WalkthroughAdded two reactive refs for container scroll dimensions to the Vue table component and exposed them via setup return. Performed a minor whitespace cleanup in the grid body component. No changes to function signatures; only public reactive data additions in the table. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Consumer
participant Table as Table Component
participant State as Reactive State
Dev->>Table: Mount/use component
Note right of Table: Setup initializes<br/>containerScrollWidth = 0<br/>containerScrollHeight = 0
Table->>State: Create refs for scroll metrics
Table-->>Dev: Expose { containerScrollWidth, containerScrollHeight }
Note over Dev,Table: Consumers can read/react to scroll dimensions
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue/src/grid/src/body/src/body.tsx(0 hunks)packages/vue/src/grid/src/table/src/table.ts(2 hunks)
💤 Files with no reviewable changes (1)
- packages/vue/src/grid/src/body/src/body.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/vue/src/grid/src/table/src/table.ts (1)
packages/vue-common/src/index.ts (1)
hooks(453-453)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
| // 表格滚动宽度 | ||
| const containerScrollWidth = hooks.ref(0) | ||
| // 表格滚动高度 | ||
| const containerScrollHeight = hooks.ref(0) |
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.
Populate the new scroll refs or the pager-height bug will persist.
containerScrollWidth / containerScrollHeight are introduced here but never assigned anywhere else in the module, so they remain stuck at 0. Without wiring them into the code that measures the body wrapper (e.g., the resize handlers / GridBody scroll listeners), nothing downstream can react to the real scroll dimensions, meaning the table height issue after changing page size is still unfixed. Please make sure these refs are updated when the container mounts and whenever its scroll size changes.
🤖 Prompt for AI Agents
packages/vue/src/grid/src/table/src/table.ts around lines 662 to 665: the newly
added refs containerScrollWidth and containerScrollHeight are never updated,
leaving them at 0 and causing the pager-height bug; update these refs when the
container mounts and whenever its scroll size changes by wiring them into the
container mount logic and existing resize/scroll handlers (or add listeners on
GridBody): read element.scrollWidth and element.scrollHeight (using nextTick if
needed) and assign to the refs, ensure handlers run on mount and on
resize/scroll events, and unregister listeners on unmount.
PR
修复切换分页大小后,表格高度不正确问题
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
New Features
Style