Lock workspace tabs while a newly selected book loads (BL-15971) - #8185
Lock workspace tabs while a newly selected book loads (BL-15971)#8185andrew-polk wants to merge 2 commits into
Conversation
|
[Claude Fable 5] Consulted Devin on 2026-08-10 ~12:35 PT up to commit 63ac774. Result: clean — 0 bugs, 0 Investigate flags. 4 Informational observations (shared tab-lock re-enable semantics, lock scope vs. async edit-tab navigation, JS ordering of the blank-then-navigate calls, TopBar import side effect) — assessed, none actionable; details on the Devin review page. CI (pr-automation) green. |
Devin observed (informationally, PR #8185) that CollectionsTabBookPane importing useWorkspaceTabInfo from TopBar.tsx also runs TopBar's module-scope WireUpForWinforms registration in the collection bundle. Harmless today, but fragile; Andrew opted to extract the hook. The workspace-tab types, defaultWorkspaceTabState, getActiveWorkspaceTab, and the hook now live in workspaceTabInfo.ts, which has no side effects; TopBar.tsx and all other consumers import from it. Also rides along: a PAPERCUTS.md entry about the NoErrorForEmptyAudio test being fragile against temp paths containing "17" (an environmental failure observed during preflight, unrelated to this branch). Tests: pnpm typecheck passed; pnpm lint 0 errors; full Vitest suite 674 passed; isolated production bundle build (build/agent-vite.sh) succeeded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
[Claude Fable 5] Consulted Devin on 2026-08-10 ~14:55 PT up to commit f988286. Result: clean — 0 bugs, 0 Investigate flags. 6 Informational observations, all assessed, none actionable: four repeats from the previous review (the 'TopBar side effect' one is now stale — that import was moved to the new side-effect-free workspaceTabInfo.ts in this very commit, per the developer's decision), plus two new notes on ClearOutDisplay (a brief dev-server-reload-only window where the page URL points at about:blank, and a pageUnloading call that already fired on the other book-change path). Details on the Devin review page. CI (pr-automation) green. |
https://issues.bloomlibrary.org/youtrack/issue/BL-15971 Selecting a different book in the Collections tab did not lock the workspace UI while the new book was loaded (which can take several seconds for a heavy book, mostly in BringBookUpToDate). During that window the Edit tab and the "Edit this book" button stayed clickable, and entering the Edit tab could briefly show the previously edited book. Three changes: - CollectionApi: the "selected-book" and "selectAndEditBook" handlers now call WorkspaceView.SetTabsEnabled(false) while loading/selecting the book (re-enabled in finally), the same mechanism used by Edit-tab saves, BloomLibrary uploads, and RAB builds (BL-16654). - CollectionsTabBookPane: the "Edit this book"/"Make a book" button now honors the existing navigationLocked flag, so it greys out in step with the main tabs. - EditingView.OnVisibleChanged: when entering the Edit tab after the book changed, call ClearOutDisplay() (which also blanks the main content browser) instead of only clearing the page list, so the old book's page is no longer visible while the new book navigates. Tests: C# BloomTests filtered on Editing/Collection fixtures passed 74/74 via build/agent-dotnet.sh; front-end pnpm test (Vitest) passed 674/674 (5 skipped); pnpm typecheck passed; pnpm lint 0 errors. CSharpier and Prettier run on all changed files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Devin observed (informationally, PR #8185) that CollectionsTabBookPane importing useWorkspaceTabInfo from TopBar.tsx also runs TopBar's module-scope WireUpForWinforms registration in the collection bundle. Harmless today, but fragile; Andrew opted to extract the hook. The workspace-tab types, defaultWorkspaceTabState, getActiveWorkspaceTab, and the hook now live in workspaceTabInfo.ts, which has no side effects; TopBar.tsx and all other consumers import from it. Also rides along: a PAPERCUTS.md entry about the NoErrorForEmptyAudio test being fragile against temp paths containing "17" (an environmental failure observed during preflight, unrelated to this branch). Tests: pnpm typecheck passed; pnpm lint 0 errors; full Vitest suite 674 passed; isolated production bundle build (build/agent-vite.sh) succeeded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
[Claude Fable 5] Consulted Devin on 2026-08-10 ~15:20 PT up to commit 25d7f1a (the rebased head; tree identical to the previously reviewed f988286). Result: 0 bugs; 1 Investigate flag (does the tab-disable actually reach the browser while the UI thread is blocked?) — mirrored as an inline thread, verified not an issue (the hook consumes the websocket payload directly, no per-event API round-trip), and resolved: #8185 (comment). 7 Informational observations, same set as prior rounds, none actionable. CI green. |
|
Converting to draft as JT and I discuss if this is the right approach. |
Devin observed (informationally, PR #8185) that CollectionsTabBookPane importing useWorkspaceTabInfo from TopBar.tsx also runs TopBar's module-scope WireUpForWinforms registration in the collection bundle. Harmless today, but fragile; Andrew opted to extract the hook. The workspace-tab types, defaultWorkspaceTabState, getActiveWorkspaceTab, and the hook now live in workspaceTabInfo.ts, which has no side effects; TopBar.tsx and all other consumers import from it. Also rides along: a PAPERCUTS.md entry about the NoErrorForEmptyAudio test being fragile against temp paths containing "17" (an environmental failure observed during preflight, unrelated to this branch). Tests: pnpm typecheck passed; pnpm lint 0 errors; full Vitest suite 674 passed; isolated production bundle build (build/agent-vite.sh) succeeded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Selecting a different book in the Collections tab did not lock the workspace UI while the new book was loaded (which can take several seconds for a heavy book, mostly in
BringBookUpToDate). During that window the Edit tab and the "Edit this book" button stayed clickable, and entering the Edit tab could briefly show the previously edited book.Three changes:
selected-bookandselectAndEditBookhandlers now callWorkspaceView.SetTabsEnabled(false)while loading/selecting the book (re-enabled infinally) — the same mechanism used by Edit-tab saves, BloomLibrary uploads, and RAB builds (BL-16654).navigationLockedflag, so it greys out in step with the main tabs.ClearOutDisplay()(which also blanks the main content browser) instead of only clearing the page list, so the old book's page is no longer visible while the new book navigates.Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-15971
🤖 Generated with Claude Code
Devin review
This change is