Skip to content

fix(animation): carousel fails to scroll to bottom with fractional row heights - #5333

Open
dajiaohuang wants to merge 3 commits into
VisActor:developfrom
dajiaohuang:fix/carousel-animation-fractional-row-height
Open

dajiaohuang wants to merge 3 commits into
VisActor:developfrom
dajiaohuang:fix/carousel-animation-fractional-row-height

Conversation

@dajiaohuang

@dajiaohuang dajiaohuang commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Background and solution

Fixes #4812. The carousel can miss its restart boundary when scrolling with a non-integer row height. Keep the explicit last-row/last-column restart and round intermediate animation offsets.

Preserve fractional row and column animation targets: only integer targets receive a final position correction. Track the column correction timer, cancel it on replacement and release, and guard callbacks against a released table. Fractional row height and fractional row index are different; the latter must retain its half-row offset.

Validation

Six real-table API tests pass, including fractional row/column targets and column-timer replacement/release. All three new/expanded regressions fail on the preceding PR head. TypeScript checking passes. Editor declarations were generated locally to satisfy the test compiler; no dependency manifest changed. Full carousel visual acceptance and upstream CI remain pending.

Changelog

Patch entries cover carousel boundary handling and fractional scroll/timer lifecycle preservation.

…ctional row/col heights

- Run post-animation scroll correction for both integer and fractional scroll targets
- Add post-animation correction for scrollToCol (consistent with scrollToRow)
- Use Math.round instead of Math.floor in animation onUpdate to reduce pixel drift
- Add explicit boundary check in carousel plugin to reliably reset at end of scroll range
- Fixes TableCarouselAnimationPlugin failing to reach bottom when defaultRowHeight is a decimal value
@fangsmile

fangsmile commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

有 3 处阻塞性问题需要修正后再合并:

  1. scrollToRow 无条件注册结束纠正会破坏分数行目标。scrollToRow(1.5, { duration: 100 }) 动画先到 1.5,随后回调又执行 scrollToRow(Math.floor(1.5), false),最终跳回第 1 行。仓库现有 packages/vtable/__tests__/api/listTable-scrollToRow.test.ts 已明确断言分数行动画不应调用 setTimeout,因此本 PR 当前会确定性破坏既有测试和 API 契约。

  2. 新增的 scrollToCol 纠正存在相同回归。动画管理器在 animation.ts 中显式支持 colDecimal,但结束回调使用 Math.floor(col),例如 scrollToCol(1.5, animationOption) 会丢失半列偏移并跳回整数列。

  3. scrollToCol 的纠正定时器没有保存句柄,也未在 release() 中取消。若动画结束前释放表格,回调仍会调用 scrollToCell();此时 release() 已把 scenegraph 和 internalProps 置为 null,会访问已销毁实例并产生异步异常。应像 _scrollToRowCorrectTimer 一样管理列定时器,并在回调前检查 isReleased。

@dajiaohuang

Copy link
Copy Markdown
Contributor Author

已在 67fa198 修正三处问题:分数行和分数列保持原动画目标,仅整数目标做结束纠正;列纠正保存句柄,在新滚动和 release 时取消,并在回调检查 isReleased。真实 ListTable 的 6 个 API 测试及 TypeScript 检查通过;三个新增/扩展回归在前一版本全部失败。PR 正文已更正分数行高与分数行索引的区别,补齐 patch changelog。完整视觉验收及上游 CI 待确认。

@dajiaohuang

Copy link
Copy Markdown
Contributor Author

补充交付状态:67fa198 当前仍为本地提交,首次推送被仓库 pre-push 的全工作区测试阻止(部分工作区未安装 Jest)。正在补齐要求的工作区依赖;上条评论的修订与定向验证结果有效,但远端尚未收到该提交。

@visactor-review-bot visactor-review-bot Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest reviewed commit: affa603

No confirmed new issues found.

Comment thread packages/vtable/src/core/BaseTable.ts Outdated
this.animationManager.scrollTo({ col }, animationOption);
setTimeout(() => {
this.scrollToCell({ col: Math.floor(col) });
this.render();

@visactor-review-bot visactor-review-bot Bot Oct 14, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Avoid a second synchronous table render after column correction

When an animated scrollToCol reaches its correction timeout, scrollToCell({ col: Math.floor(col) }) already calls this.render(). The additional call here synchronously renders the scenegraph a second time with no intervening state change. For a large visible table using column carousel (e.g. colCount: 1 with short animation duration/delay), every column transition therefore incurs an unnecessary full canvas render, adding avoidable work and frame latency. Remove this call and let scrollToCell perform the single final render. This is separate from the fractional-target and timer-lifecycle regressions in the existing PR discussion; verified by tracing the frozen source, not by a full-project benchmark.


Confirmed fixed (affa603): For the original animated integer-column correction on a large visible table, the current timeout calls scrollToCell({ col }) once and does not call render separately; scrollToCell itself performs the final render. The extracted current scrollToCol method was run against a minimal mocked table, confirming one correction call and no extra render; its fractional-target branch schedules no correction. The new regression assertion also expects a single render. Thus the second synchronous render reported previously is absent on the original trigger path.

@github-actions github-actions Bot added the chore label Oct 15, 2026
@dajiaohuang

Copy link
Copy Markdown
Contributor Author

已正常推送 67fa198 和 affa603(当前远端 head:affa6032056153ecc4e39f40b29c7bd879cf8f2e):修复此前三项分数目标/列纠正定时器问题,并移除列纠正后重复 render,加入单次 render 回归断言。

此前 pre-push 停滞的启动原因已复现:宿主继承的 ELECTRON_RUN_AS_NODE=1 使 Electron 作为 Node 启动,jest-electron 主入口因 app 未定义退出,runner 仍等待 readiness;仅对本次测试/普通 push 子进程取消该环境变量后,真实 ProcReady/ProcInitialWinEnd 握手通过。未绕过或修改 hook/测试。

普通 pre-push rush test --only tag:package 已以 exit 0 完成(8 个有测试操作通过但有 warnings,3 个 NO OP);其中 VTable 80 suites/393 tests、vtable-sheet 76 suites/446 tests、vtable-plugins 10 suites/67 tests 通过,包含此次四个滚动 API 回归断言。whitespace 检查通过。完整独立 TypeScript 检查本次未重跑,先前 TS2742 限制未据此宣称解决;完整视觉验收及维护者 review 仍待确认。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 动画插件,行高为小数时,滚不到最底部

2 participants