fix(web): persist file explorer expanded tree and scroll position across navigation#911
Open
swear01 wants to merge 2 commits into
Open
fix(web): persist file explorer expanded tree and scroll position across navigation#911swear01 wants to merge 2 commits into
swear01 wants to merge 2 commits into
Conversation
…oss navigation Expanded folder state and scroll position in the Directories tab were stored only in local React state, so navigating to a file and back would reset the tree to the root and scroll to top. Now both are saved to sessionStorage (keyed by sessionId) on every change and restored on remount, so the explorer resumes exactly where the user left off. Closes tiann#910 via [HAPI](https://hapi.run) Co-Authored-By: HAPI <noreply@hapi.run>
There was a problem hiding this comment.
Findings
- [Major] Reset persisted tree state when the session id changes —
useState(() => readExpanded(props.sessionId))only reads storage on the first mount, but this route can be reused while$sessionIdchanges (web/src/router.tsx:992). The next render keeps the previous session's expanded set and the new write effect stores it under the new session key, so opening another session's Files page can inherit/overwrite the wrong tree state. Evidence:web/src/components/SessionFiles/DirectoryTree.tsx:203.
Suggested fix:<DirectoryTree key={sessionId} api={api} sessionId={sessionId} rootLabel={rootLabel} onOpenFile={(path) => handleOpenFile(path)} />
Summary
- Review mode: initial
- One state scoping issue found in the new sessionStorage-backed directory expansion persistence.
Testing
- Not run (automation):
bun typecheckunavailable in this runner (bun: command not found).
HAPI Bot
| onOpenFile: (path: string) => void | ||
| }) { | ||
| const [expanded, setExpanded] = useState<Set<string>>(() => new Set([''])) | ||
| const [expanded, setExpanded] = useState<Set<string>>(() => readExpanded(props.sessionId)) |
There was a problem hiding this comment.
[Major] useState(() => readExpanded(props.sessionId)) only reads storage on the first mount. Because the files route is under the parameterized $sessionId route, navigating from one session's Files page to another can reuse this component with a new sessionId; the old expanded set is then written under the new session key by the effect below. Key the tree by sessionId (or otherwise reset state before writing for a new id) so each session hydrates its own persisted state.
Suggested fix:
<DirectoryTree
key={sessionId}
api={api}
sessionId={sessionId}
rootLabel={rootLabel}
onOpenFile={(path) => handleOpenFile(path)}
/>…ate across sessions
When navigating between sessions, React can reuse the same DirectoryTree
instance. The useState lazy initializer only runs on first mount, so the
tree would hydrate with the wrong session's expanded set and then
overwrite the new session's storage key.
Adding key={sessionId} forces a fresh mount per session.
via [HAPI](https://hapi.run)
Co-Authored-By: HAPI <noreply@hapi.run>
There was a problem hiding this comment.
Findings
No issues found.
Questions
None.
Summary
- Review mode: follow-up after new commits
- Previous HAPI Bot finding is addressed by remounting
DirectoryTreepersessionIdatweb/src/routes/sessions/files.tsx:467. - Residual risk: no automated coverage found for the new directory expansion and scroll restoration behavior; current validation appears manual.
Testing
- Not run (automation)
HAPI Bot
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
DirectoryTreeto unmount and reset to only the root nodesessionStoragekeyed bysessionIdand restored on remountChanges
DirectoryTree.tsx: reads initialexpandedfromsessionStorageon mount; writes on every toggle viauseEffectfiles.tsx: adds arefon the scroll container; restoresscrollTopfromsessionStorageon mount and saves it on unmountTest plan
sessionStorageentry) still opens with only the root expandedFixes #910
🤖 Generated with Claude Code