Fix GUI state bugs and add React hook test coverage (A-13, B-15..B-19, M-6, B-27) - #170
Merged
Merged
Conversation
B-27 found zero React hook or component tests against 87 modules. A-13, B-16 and B-17 all lived in that gap, which is why they survived. Adds @testing-library/react + jsdom (opted into per-file, so the ~57 existing pure tests keep running in node) and lands each fix with a test that was verified to fail against the old code. A-13: workspace search jumped to a row via setActiveTabId followed by setTimeout(() => selectRow(i), 0). The timeout fired after commit but still held the pre-switch closure, so it patched the *previous* tab and clamped against that tab's row count. Reproduced live: parked on ARP, searched nginx (row index 1), the jump switched tabs correctly but selected index 0. Replaced with selectRowInTab(tabId, index), which derives the destination tab's rows and has no ordering dependency. B-16: the selection-reset effect keyed on sortKey/sortDir, and each tool kind has its own DEFAULT_SORT, so merely switching tabs looked like a re-sort and wiped the destination tab's selection. Reproduced live: scan -> ARP -> scan dropped a 3-row selection to 0 while scan -> scan -> scan kept all 3. Now compares the previous tab id so re-sorting the same tab still resets, which is the behaviour that had to be preserved. B-17: setActiveTabId was called from inside a setTabs updater. React requires updaters to be pure and StrictMode double-invokes them in dev; this worked only because the call happened to be idempotent. The replacement id is now computed outside the updater. B-18: the global keydown listener was torn down and re-attached on every render -- deps included a fresh object and two fresh closures, and a 3-second stats poll re-renders App continuously. The handler is now read through a ref and the listener attaches once. B-15: the progress-event subscription had no .catch, so a rejected listen() left every later operation silently reporting no progress. Every other Tauri call in the codebase was already guarded. B-19: "Copied" was reported even when the write failed -- .catch(() => undefined) swallowed the rejection and the optional chain made a missing navigator.clipboard a silent no-op. All four copy paths now route through one helper that reports failure, matching what export already did. The transfer.ts helpers become pure text builders. M-6: an unterminated quote made the whole tail one literal token, so typing "open on the way to "open" silently matched nothing. Reproduced live: open -> 4 rows, 'open' -> 4, 'open -> 0. The tokenizer now re-tokenizes without the dangling quote, so it behaves as if the quote had not been typed yet. A mid-word apostrophe (don't) still never opens a quote. useWorkspace crossed the file-size cap, so selection handling moves to useSelection.ts -- the split the gate's own note prescribed. 72 tests pass, up from 57.
jsdom 30 requires Node ^22.22.2 || ^24.15.0 || >=26, and CI runs Node 20, so it failed with `webidl.util.markAsUncloneable is not a function` from undici. It only passed locally by luck -- this host is on 24.14.1, also below jsdom 30's floor, and npm had warned EBADENGINE at install time. jsdom 26 declares >=18. Verified by running the suite under Node 20 directly rather than trusting the range: 72 tests pass.
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.
Why these survived
B-27: zero React hook or component tests against 87 modules. A-13, B-16 and B-17 all live in that gap. This adds
@testing-library/react+jsdomand lands each fix with a test verified to fail against the old code — I reverted B-16 and confirmed the test goes red before restoring it.jsdom is opted into per-file via
@vitest-environment jsdom, so the ~57 existing pure-module tests keep running in node.72 tests pass, up from 57.
Fixes
setActiveTabId+setTimeout(() => selectRow(i), 0). The timeout fired after commit but held the pre-switch closure, so it patched the previous tab and clamped against its row count. Replaced withselectRowInTab(tabId, index).sortKey/sortDir. Each tool kind has its ownDEFAULT_SORT, so merely switching tabs looked like a re-sort and wiped the destination tab's selection. Now compares the previous tab id — re-sorting the same tab still resets.setActiveTabIdcalled inside asetTabsupdater. React requires updaters to be pure; StrictMode double-invokes them. Worked only because the call was idempotent.Appcontinuously. Handler now read through a ref; listener attaches once..catch, so a rejectedlisten()left every later operation silently reporting no progress..catch(() => undefined)swallowed the rejection and the optional chain made a missingnavigator.clipboarda silent no-op. All four copy paths now route through one reporting helper."openen route to"open"silently matched nothing.Reproduced live before fixing
Driven through the running GUI at
?demo=screenshot:open→4 rows,'open'→4,'open→0nginx(row idx 1); jump switched tabs but selected idx 0Note on scope
useWorkspacecrossed the file-size cap, so selection handling moved touseSelection.ts— the split the gate's own note already prescribed ("split selection handling next").Accessibility (H-3, B-20/21/22, C-13) is deliberately not in this PR; it's the next one.