Scroll horizontally to reveal deeply nested nodes (#220)#367
Open
TrevorBurnham wants to merge 2 commits into
Open
Scroll horizontally to reveal deeply nested nodes (#220)#367TrevorBurnham wants to merge 2 commits into
TrevorBurnham wants to merge 2 commits into
Conversation
scrollTo only called react-window's scrollToItem, which adjusts the vertical axis only. A node indented by level * indent could sit past the right edge when rows overflow horizontally, so it stayed off-screen. After the vertical scroll, nudge the list's horizontal scroll so the node's indentation start is in view. It's a no-op when the list doesn't overflow horizontally, so trees that fit their width are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes TreeApi.scrollTo() so that selecting/scrolling to deeply nested nodes also reveals them horizontally when indentation pushes content out of the viewport, addressing issue #220.
Changes:
- After
react-window’s verticalscrollToItem, adjusts the list outer element’sscrollLeftto bring the target node’s indentation start into view. - Adds Jest coverage for horizontal scrolling behavior across several scenarios.
- Adds a changeset entry documenting the user-facing fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
modules/react-arborist/src/interfaces/tree-api.ts |
Adds horizontal scrolling logic to scrollTo() via a helper that updates scrollLeft based on node indentation. |
modules/react-arborist/src/interfaces/tree-api.test.ts |
Adds tests validating horizontal scroll behavior for deep nodes and no-op conditions. |
.changes/367-scrollto-horizontal.md |
Records the fix in the project’s changeset system for release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+697
to
+704
| const left = node.level * this.indent; | ||
| const viewLeft = el.scrollLeft; | ||
| const viewRight = el.scrollLeft + el.clientWidth; | ||
| /* Only move when the node's indentation is outside the viewport, then align | ||
| its content start to the left edge so the label is revealed. */ | ||
| if (left < viewLeft || left > viewRight) { | ||
| el.scrollLeft = left; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Good catch on both. Fixed in e99a132: the right-edge check now uses left >= viewRight (the visible range is half-open, so a start exactly on the right edge is already clipped), and the target is clamped with Math.max(0, Math.min(left, scrollWidth - clientWidth)). Added tests for the exact-boundary and clamp cases.
…rr#367) The visible range is half-open [scrollLeft, scrollLeft + clientWidth), so a node whose start equals the right edge is already clipped; use >= and clamp the target scrollLeft to the scrollable range. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Summary
Fixes #220:
scrollTodoesn't bring deeply nested nodes into view horizontally.scrollToonly called react-window'sscrollToItem(index, align), which adjusts the vertical axis. A node is indented bylevel * indent(row-container.tsx), so when rows overflow the tree's width a deep node stays off-screen to the right — the tree scrolls down to its row but the label is never revealed. react-window doesn't know about react-arborist's horizontal indentation, so the library has to handle that axis itself.Change
After the vertical
scrollToItem,scrollTonow nudges the list's outer element so the node's indentation start (level * indent) is within the horizontal viewport, aligning it to the left edge when it's off-screen in either direction.scrollWidth <= clientWidth) — trees that fit their width are completely unaffected.Test plan
#220block intree-api.test.tscovering: scrolls right when past the right edge, scrolls left when past the left edge, no change when already in view, and no-op when the list doesn't overflow horizontally.yarn test— 53 passing.tsc --noEmit— clean.🤖 Generated with Claude Code