fix(grid): fix scroll bar error after load data #3644
Conversation
WalkthroughUpdates grid table data handling and watchers: removing a notRefresh flag from loadTableData invocation to always clear scroll and recalculate on data changes; adding Vue 2-specific data watcher logic; and making parentHeight watcher call recalculate synchronously. Also imports isVue2 for compatibility. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant GridTable
participant Methods as methods.ts
App->>GridTable: Update data
alt Vue 2 and same array ref/length
GridTable->>GridTable: data watcher triggers
GridTable->>Methods: handleDataChange()
else Other cases
GridTable->>Methods: handleDataChange() (existing triggers)
end
Methods->>Methods: loadTableData(data)
Methods->>Methods: clearScroll()
Methods->>Methods: recalculate()
sequenceDiagram
participant App
participant GridTable
App->>GridTable: parentHeight change
GridTable->>GridTable: recalculate() (sync)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/vue/src/grid/src/table/src/table.ts (1)
543-545: Recalculating synchronously on parentHeight changes can thrash; coalesce with rAFCalling
recalculate()immediately on every parent height change may cause layout thrash during rapid resizes/animations. Consider coalescing into a single requestAnimationFrame tick leveraging the existingtaskspattern.Apply this diff:
- this.recalculate() + // Coalesce recalculations into a single rAF tick to avoid thrashing on rapid parent height changes + if (this.tasks.rafRecalc) cancelAnimationFrame(this.tasks.rafRecalc) + this.tasks.rafRecalc = requestAnimationFrame(() => { + this.tasks.rafRecalc = null + this.recalculate() + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/vue/src/grid/src/table/src/methods.ts(1 hunks)packages/vue/src/grid/src/table/src/table.ts(2 hunks)
⏰ 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)
🔇 Additional comments (1)
packages/vue/src/grid/src/table/src/table.ts (1)
33-35: Importing isVue2 is appropriate and used correctlyThe added
isVue2import is used by the newdatawatcher below. No concerns.
| if (Array.isArray(this.data)) { | ||
| !this._isUpdateData && this.loadTableData(this.data, true) | ||
| !this._isUpdateData && this.loadTableData(this.data) | ||
| this._isUpdateData = false | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Make data-change refresh reset scroll and reflow — aligns with the bugfix objective
Switching loadTableData(this.data, true) to loadTableData(this.data) will clear scroll and trigger recalculate(). This is consistent with fixing the abnormal scrollbar after data mutations. Note that clearScroll() already respects fetchOption.isReloadScroll, so consumers can still opt out of scroll reset.
Two things to verify:
- Virtualized scenarios (scrollX/YLoad) restore as expected via
attemptRestoreScrollwith large-to-small data changes. - No unexpected UX regressions for workflows relying on preserving scroll for in-place edits (covered by
isReloadScroll).
You can quickly scan for other internal callers that intend to preserve scroll by passing true to loadTableData:
🏁 Script executed:
#!/bin/bash
# Find all calls to loadTableData and show context.
rg -n -A 2 -B 2 $'loadTableData\\('Length of output: 3190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "----- packages/vue/src/grid/src/table/src/table.ts (1-160) -----"
sed -n '1,160p' packages/vue/src/grid/src/table/src/table.ts || true
echo
echo "----- packages/vue/src/grid/src/table/src/methods.ts (280-420) -----"
sed -n '280,420p' packages/vue/src/grid/src/table/src/methods.ts || true
echo
echo "----- packages/vue/src/grid/src/table/src/methods.ts (1990-2065) -----"
sed -n '1990,2065p' packages/vue/src/grid/src/table/src/methods.ts || true
echo
echo "----- occurrences of clearScroll, attemptRestoreScroll, recalculate, isReloadScroll -----"
rg -n "clearScroll\(|attemptRestoreScroll\(|recalculate\(|isReloadScroll" || trueLength of output: 16901
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "----- methods.ts: clearScroll (lines ~1800-1860) -----"
sed -n '1780,1860p' packages/vue/src/grid/src/table/src/methods.ts || true
echo
echo "----- methods.ts: loadTableData (lines ~280-340) -----"
sed -n '280,340p' packages/vue/src/grid/src/table/src/methods.ts || true
echo
echo "----- methods.ts: handleDataChange (lines ~2036-2060) -----"
sed -n '2028,2060p' packages/vue/src/grid/src/table/src/methods.ts || true
echo
echo "----- methods.ts: attemptRestoreScroll (lines ~2080-2120) -----"
sed -n '2080,2120p' packages/vue/src/grid/src/table/src/methods.ts || true
echo
echo "----- Search for _isUpdateData occurrences -----"
rg -n "_isUpdateData" || true
echo
echo "----- table.ts: loadStatic context (lines 88-104) -----"
sed -n '88,104p' packages/vue/src/grid/src/table/src/table.ts || trueLength of output: 8073
Make data-change refresh reset scroll and reflow — one remaining init caller to update
handleDataChange already calls loadTableData(this.data) (no notRefresh), so data-change will clear scroll and trigger recalc. However loadStatic still calls loadTableData(..., true) during initial prop load and should be updated if you want the same behavior on initial load.
Files to update / note:
- packages/vue/src/grid/src/table/src/methods.ts:2050-2053 — handleDataChange already uses this.loadTableData(this.data) (no change required).
- packages/vue/src/grid/src/table/src/table.ts:88-104 — loadStatic currently preserves scroll on init.
Recommended change (table.ts):
- if (data && data.length > 0) {
- _vm.loadTableData(data, true)
- _vm.updateStyle()
- }
- if (data && data.length > 0) {
- _vm.loadTableData(data)
- _vm.updateStyle()
- }
Notes to verify:
- Virtualized scenarios: attemptRestoreScroll clamps to max scroll and triggers scroll events — verify large→small data changes restore as expected.
- UX edge cases: clearScroll respects fetchOption.isReloadScroll, so workflows that must preserve scroll (in-place edits) remain opt-out-able.
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/table.ts around lines 88 to 104,
loadStatic currently calls _vm.loadTableData(data, true) which preserves scroll
on initial prop load; change it to call _vm.loadTableData(data) (no second arg)
so initial loads reset scroll and trigger reflow like handleDataChange, then
call _vm.updateStyle(); keep clearScroll/fetchOption.isReloadScroll behavior for
opt-out and verify virtualized scenarios clamp/restore scroll correctly.
| data(array1, array2) { | ||
| if (isVue2 && array1 === array2 && array1.length === array2.length) { | ||
| this.handleDataChange() | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good Vue 2 array-mutation watcher; add an Array guard to avoid unnecessary calls
The watcher correctly handles Vue 2’s same-reference array mutations (splice/$set). Add a guard to skip non-array data (since props.data also allows Object) to avoid needless handleDataChange() calls.
Apply this diff:
- data(array1, array2) {
- if (isVue2 && array1 === array2 && array1.length === array2.length) {
+ data(array1, array2) {
+ // Guard non-array props and avoid unnecessary calls when data is an object
+ if (isVue2 && Array.isArray(array1) && array1 === array2 && array1.length === array2.length) {
this.handleDataChange()
}
}To ensure the watcher captures Vue 2 in-place mutations across the codebase, you can grep for other places mutating props.data directly:
🏁 Script executed:
#!/bin/bash
# Look for common in-place mutations on `data` props
rg -n -A 2 -B 2 $'\\.data\\.(push|splice|pop|shift|unshift)\\('
rg -n -A 2 -B 2 $'\\$set\\(\\s*[^,]+\\.data'Length of output: 3166
Add Array guard to Vue 2 data watcher in table.ts
The watcher should only trigger for arrays — otherwise objects (with undefined length) can cause unnecessary handleDataChange() calls. Verified the repo contains in-place mutations of .data (so arrays are in use), so this guard is safe and needed.
Files to update / check:
- packages/vue/src/grid/src/table/src/table.ts — watcher at ~lines 548–552
- Examples showing in-place
.datamutations (for context): examples/sites/demos/pc/app/chart/question/demo4.vue, examples/sites/demos/pc/app/chart/question/demo4-composition-api.vue - renderless uses
.data.pushin: packages/renderless/src/fall-menu/index.ts, packages/renderless/src/chart-pie/index.ts - huicharts map uses
.data.pushin: packages/vue/src/huicharts/huicharts-map/src/map.ts
Apply this diff:
- data(array1, array2) {
- if (isVue2 && array1 === array2 && array1.length === array2.length) {
- this.handleDataChange()
- }
- }
+ data(array1, array2) {
+ // Guard non-array props and avoid unnecessary calls when data is an object
+ if (isVue2 && Array.isArray(array1) && array1 === array2 && array1.length === array2.length) {
+ this.handleDataChange()
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data(array1, array2) { | |
| if (isVue2 && array1 === array2 && array1.length === array2.length) { | |
| this.handleDataChange() | |
| } | |
| } | |
| data(array1, array2) { | |
| // Guard non-array props and avoid unnecessary calls when data is an object | |
| if (isVue2 && Array.isArray(array1) && array1 === array2 && array1.length === array2.length) { | |
| this.handleDataChange() | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/vue/src/grid/src/table/src/table.ts around lines 548 to 552, the Vue
2 data watcher currently calls this.handleDataChange() even when the watched
value is an object without length; update the guard so it only triggers for
arrays: ensure you check Array.isArray(array1) (and optionally
Array.isArray(array2)) before verifying equality and length (i.e., if (isVue2 &&
Array.isArray(array1) && array1 === array2 && array1.length === array2.length) {
this.handleDataChange() }), so only array in-place mutations invoke
handleDataChange.
2 participants
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
Bug Fixes