Repository navigation
Spin the Files refresh icon only on a manual refresh - #397
Open
coolestnick wants to merge 1 commit into
Open
coolestnick wants to merge 1 commit into
coolestnick wants to merge 1 commit into
Conversation
The Files rail reloads every 5 seconds while a task runs, and that poll reused the same path as the refresh button, so the icon spun and the button flickered disabled every 5 seconds on its own. Track the manual refresh separately: load() now spins the icon only when the viewer clicks, and the background poll and org-change reloads refresh quietly. Move the markup into ArtifactsRailView, which takes the rail's state as props, and add render tests for the loading, empty, unavailable and list states plus the rule that the icon spins only on a manual click.
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.
Closes #348
Problem
The Files panel next to a thread has no tests, and its 5-second background refresh reused the manual refresh path. So while a task runs, the refresh icon spins and the button flickers disabled every 5 seconds on its own, even when the viewer never touched it.
Fix
load()now takes{ manual }and sets therefreshingstate only when the viewer clicks. The initial load, the 5-second poll and org-change reloads all refresh quietly, so the icon stays still unless clicked.ArtifactsRailView, a presentational component that takes the rail's state as props (artifacts,available,refreshing,onRefresh). The spinner is now driven purely by therefreshingprop.ArtifactsRailkeeps its public{ threadId, live }signature, so its only consumer (session-view.tsx) is unchanged.Tests (new
artifacts-rail.test.tsx)refreshingprop is set.The interaction test drives the rail through local
fetch/EventSourcestubs restored in afinally, not bun'smock.module:mock.moduleis process-wide and would leak a never-resolving fetch into every component test file loaded afterward, hanging the suite.Screenshots
Each frame below reports the refresh icon's real spin state (read from the DOM) as the rail polls every 5 seconds. No clicks happen except where noted.
Before (on
main): the icon auto-spins every 5sAfter (this PR): the icon stays still, and spins only on a manual click
Checks
bun test components/chat: 855 pass / 0 failbun run test(frontend): 1774 pass / 0 failbun run typecheck(root, all packages): passbun run lint: 0 errorsbun run build: compiled successfullybun test scripts/agent-docs.test.ts: 5 passcomponentssuite runs clean in ~3s (no mock-leak hang)Merge order: this PR overlaps PR #395
PR #395 (reword the thread side panels) edits the same file's copy. This PR intentionally leaves that copy untouched so it stays a single-purpose change. Please merge this PR first. It touches only the refresh behavior and structure; #395 then rebases on top and changes the strings ("Session files", the "Refresh session files" aria-label, the subtitle) over the new
ArtifactsRailView. Merging in the other order would force this behavioral fix to rebase around the copy change for no benefit.