Skip to content

fix(web): stop the split tile busy timer outliving its download - #1663

Merged
snapotter-hq merged 1 commit into
snapotter-hq:mainfrom
drakeo338:claude/1662-fix
Sep 30, 2026
Merged

snapotter-hq merged 1 commit into
snapotter-hq:mainfrom
drakeo338:claude/1662-fix

Conversation

@drakeo338

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #1662.

handleDownloadTile started a 500 ms timer to clear the busy flag and never cancelled it. Clicking a second tile within 500 ms had its busy flag cleared early by the first tile's timer, and the timer still fired after the panel unmounted. The timer id now lives in a ref, is cleared before a new one starts, and is cleared on unmount.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • New translation or i18n update
  • Documentation update
  • Test improvement
  • Refactor (no functional change)

Checklist

  • I have read CONTRIBUTING.md
  • I have signed the CLA (the bot will prompt you if not)
  • My changes follow the project's code style (Biome passes)
  • I have added or updated tests for my changes
  • All existing tests pass locally (pnpm test)
  • TypeScript compiles without errors (pnpm typecheck)
  • My PR is focused on a single concern (under 400 lines of change)
  • I have not modified CI, release, or linter configuration files

Screenshots (if applicable)

No visual change.

Only the touched test file was run on this commit (19 passed); the full suite, Biome and typecheck were not run, and the CLA is still to be signed via the bot.

handleDownloadTile started a 500 ms setTimeout to clear the busy flag
and never cancelled it. A second tile clicked within 500 ms had its
flag cleared by the first tile's timer, and the timer still fired after
the panel unmounted.

Hold the timer id in a ref, clear the previous timer before starting a
new one, and clear it on unmount.

Fixes snapotter-hq#1662
@snapotter-hq
snapotter-hq merged commit 7a32ac1 into snapotter-hq:main Sep 30, 2026
21 checks passed
@snapotter-hq

Copy link
Copy Markdown
Owner

Review notes before merge.

Both new tests fail against the old split-settings.tsx and pass with the fix: the unmount test finds one timer still pending, and the second tile stops bouncing at 600 ms. The whole file passes on the PR head (19 of 19; 17 of 17 on the base), and required CI is green.

Nothing blocking. Three smaller things were looked at and left alone:

  • expect(vi.getTimerCount()).toBe(0) assumes nothing else in the rendered tree has a timer pending at unmount, so a provider that picks one up later would break it for reasons unrelated to split. Comparing the count before and after unmount() would be sturdier. It holds today, so I didn't change it.
  • handleDownloadTile returns quietly when a tile has no blobUrl, and clicking a revoked blob URL gives no feedback. Both predate this PR and aren't touched by it.
  • On its own, the unmount half only caused a setState on an unmounted component, which React ignores. The part users could see was the second tile's busy animation ending early, and the second test pins that.

@snapotter-hq

Copy link
Copy Markdown
Owner

Thanks @drakeo338. A second tile clicked within half a second now keeps its busy animation until its own timer runs out, and the split panel no longer leaves that timer behind when it unmounts. It merged as you sent it; both new tests fail without the fix, which is what sold me. It'll ship in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split tile download leaves a 500 ms timer running, failing the unit job after jsdom teardown

2 participants