fix(tui): migrate to ControlledScrollView with onContentHeightChange - #577
Merged
Conversation
- getMaxScrollOffset() is not part of ink-scroll-view API - Replaced with documented getBottomOffset() method - This fixes broken scroll-up detection where isUserScrollingRef would never be set due to silent undefined fallback
- Remove unnecessary triggerRender() from updateMessage — bubbles update themselves via pub/sub without parent re-render - Replace remeasure() with remeasureItem(lastIdx) during scroll to avoid O(n) layout thrashing on every update - Fix ref dependency in scroll position check (lastMsgCountRef.current doesn't trigger re-renders) - Prune pub/sub topics for messages that fall off the render slice - Reset _messageIdCounter in _reset() for test isolation - Use string-based content hash instead of sum to avoid collisions
…nly when content changes - Only call triggerRender() in updateMessage when content actually changed - Metadata-only updates (tool calls, status, etc.) still use pub/sub only - Preserves scroll behavior while avoiding unnecessary parent re-renders
…r scroll-to-bottom
avoidwork
changed the title
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Migrate the message list from uncontrolled
ScrollViewto controlledControlledScrollViewfrom ink-scroll-view. The scroll position is now managed viascrollOffsetstate and updated through theonContentHeightChangecallback, providing more reliable scroll-to-bottom behavior during streaming.Also fixes
getMaxScrollOffset→getBottomOffsetAPI usage, optimizes re-render triggers to only fire on actual content changes, prunes pub/sub topics for messages that fall off the render slice, and improves_resetcleanup.Type of Change
Testing
Lint passes with
npm run lint. No code changes require test updates — the scroll behavior is UI-level and tested via the TUI integration tests.Coverage
Checklist
npm run lintpasses