Skip to content

fix(list): preserve scroll position on expand/collapse#2678

Merged
gbirman merged 1 commit into
mainfrom
macro-cz7DcP86j5yRGJ4R6UqLq-expand/collapse-in-list-should-preserve-position
Apr 20, 2026
Merged

fix(list): preserve scroll position on expand/collapse#2678
gbirman merged 1 commit into
mainfrom
macro-cz7DcP86j5yRGJ4R6UqLq-expand/collapse-in-list-should-preserve-position

Conversation

@gbirman
Copy link
Copy Markdown
Contributor

@gbirman gbirman commented Apr 20, 2026

Summary

  • CollapsibleList.collapse was manually shifting scrollContainer.scrollTop by heightBefore - heightAfter, which fought virtua's built-in scroll anchoring (ACTION_ITEM_RESIZE + overflow-anchor: none) and caused the viewport to drift across expand/collapse cycles.
  • Removed the manual shift and the now-unused getScrollParent/toggleRef plumbing. Expand and collapse are now a single toggle handler; virtua handles the scroll anchoring.
  • Verified in Chrome DevTools against a search result with 27 content hits (~1080px height swing): entity stays pinned at the same viewport Y and scrollTop stays stable across 5 cycles, both at scrollTop=0 and mid-scroll.

Remove manual scrollTop adjustment in CollapsibleList's collapse path. The
virtualizer (virtua) already handles item-resize scroll anchoring via its
built-in ACTION_ITEM_RESIZE logic and overflow-anchor: none, so the manual
scrollTop adjustment was double-correcting and causing the viewport to
drift across expand/collapse cycles.
@macro-application
Copy link
Copy Markdown

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4c97bb0d-0bf4-442b-99c9-9f3376bb1811

📥 Commits

Reviewing files that changed from the base of the PR and between 278f710 and ae63298.

📒 Files selected for processing (1)
  • js/app/packages/entity/src/components/CollapsibleList.tsx

📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Streamlined the collapsible list component's toggle mechanism for improved simplicity.
    • Removed automatic scroll positioning behavior when expanding or collapsing lists.

Walkthrough

Simplified the ToggleButtonProps interface in CollapsibleList by replacing toggleRef, collapse, and setShowAll with a single toggle handler. Removed scroll-adjustment and scroll-parent detection logic, and simplified the internal click handler to invert the showAll state.

Changes

Cohort / File(s) Summary
CollapsibleList Toggle Refactor
js/app/packages/entity/src/components/CollapsibleList.tsx
Replaced toggleRef, collapse, and setShowAll props with a unified toggle(e) handler in ToggleButtonProps. Removed scroll-parent detection and manual scrollTop adjustment logic. Simplified internal click handler to stop propagation and invert showAll state.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commits format with 'fix:' prefix, is under 72 characters (54 chars), and accurately describes the main change.
Description check ✅ Passed The description clearly relates to the changeset, providing context about manual scroll adjustment removal and the shift to virtua's scroll anchoring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

@gbirman gbirman merged commit 2ea59ac into main Apr 20, 2026
23 checks passed
@gbirman gbirman deleted the macro-cz7DcP86j5yRGJ4R6UqLq-expand/collapse-in-list-should-preserve-position branch April 20, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant