Make the publish upload modal on the publish-tool switcher too (BL-16654)

src/BloomBrowserUI/app/App.tsx

@@ -8,12 +8,11 @@ import {
     defaultWorkspaceTabState,
     getActiveWorkspaceTab,
     TopBar,
-    TabStates,
+    useWorkspaceTabInfo,
     WorkspaceTabId,
 } from "../react_components/TopBar/TopBar";
 import { PublishTabPane } from "../publish/PublishTab/PublishTabPane";
 import { kPanelBackground } from "../bloomMaterialUITheme";
-import { useWatchApiObject } from "../utils/bloomApi";
 import { EditTabPane } from "./EditTabPane";
 import { ToastHost } from "../toast/ToastHost";
 
@@ -27,12 +26,7 @@ export const App: React.FunctionComponent = () => {
     // Team collection toast click returns to collection tab in WorkspaceView.cs:606.
     // Publish flow can force jump to edit tab in LibraryPublishApi.cs:517.
     // Edit-book command switches to edit tab in WorkspaceView.cs:1164.
-    const state = useWatchApiObject<{ tabStates: TabStates }>(
-        "workspace/tabs",
-        defaultWorkspaceTabState,
-        "workspace",
-        "tabs",
-    );
+    const state = useWorkspaceTabInfo();
 
     const tabStates = state.tabStates ?? defaultWorkspaceTabState.tabStates;
     const activeTab = React.useMemo((): WorkspaceTabId => {

src/BloomBrowserUI/publish/Apps/AppPublisherScreen.tsx

@@ -87,29 +87,17 @@ const AppActionButton: React.FunctionComponent<{
 // Keep this component mostly declarative. The hook owns websocket/API state so the JSX can stay focused on the workflow.
 const AppPublisherScreenContents: React.FunctionComponent<{
     isActive: boolean;
-    onBusyChange?: (busy: boolean) => void;
 }> = (props) => {
     const screenState = useAppBuilderPublisherScreen(props.isActive);
     const [showSettingsDialog, setShowSettingsDialog] = React.useState(false);
     const [showChooseBooksDialog, setShowChooseBooksDialog] =
         React.useState(false);
     const [showUsbDebuggingHelpDialog, setShowUsbDebuggingHelpDialog] =
         React.useState(false);
-    // An effect (not an event handler) is warranted here, even though "notify the parent of a
-    // change" usually belongs in the handler that caused the change: busyAction has no single
-    // originating handler. It is set/cleared from several asynchronous sources inside
-    // useAppBuilderPublisherScreen — the "actionComplete" websocket event, the status-poll
-    // recovery that reconciles with the backend after a blank/reload, and the action-start call —
-    // so the only place that observes every transition is a render keyed on the resulting value.
-    // What we are doing is synchronizing an external system (the publish-tab host, which makes the
-    // operation modal by blocking the other publish tools while C# blocks the main workspace tabs)
-    // to that state, which is exactly what effects are for. The cleanup resets it to false so
-    // leaving or unmounting never leaves the publish tools stuck disabled.
-    React.useEffect(() => {
-        props.onBusyChange?.(!!screenState.busyAction);
-        return () => props.onBusyChange?.(false);
-        // eslint-disable-next-line react-hooks/exhaustive-deps
-    }, [screenState.busyAction]);
+    // Note: this screen no longer tells the publish-tab host when it is busy. Blocking the other
+    // publish tools during a prepare/build/install is now driven by the same C# navigation lock
+    // that greys out the main workspace tabs (RabPublishApi sets it; PublishTabPane reads it),
+    // so the two can't disagree, and the lock survives this component remounting mid-action.
     const prepareTooltip = useL10n(
         "Create the Reading App Builder project in this collection's Bloom App Data folder.",
         "PublishTab.Apps.Prepare.TooltipBloomAppData",
@@ -731,7 +719,6 @@ const AppPublisherScreenContents: React.FunctionComponent<{
 
 export const AppPublisherScreen: React.FunctionComponent<{
     isActive: boolean;
-    onBusyChange?: (busy: boolean) => void;
 }> = (props) => {
     const optionsPanel = (
         <SettingsPanel>
@@ -770,10 +757,7 @@ export const AppPublisherScreen: React.FunctionComponent<{
                 bannerDescriptionMarkdown="Create an app that you can install on your Android phone, share with others, and publish on the Google Play Store."
                 optionsPanelContents={optionsPanel}
             >
-                <AppPublisherScreenContents
-                    isActive={props.isActive}
-                    onBusyChange={props.onBusyChange}
-                />
+                <AppPublisherScreenContents isActive={props.isActive} />
             </PublishScreenTemplate>
         </Typography>
     );

src/BloomBrowserUI/publish/LibraryPublish/LibraryPublishScreen.tsx

@@ -1,4 +1,5 @@
 import { css } from "@emotion/react";
+import * as React from "react";
 import { Typography } from "@mui/material";
 import { Link } from "../../react_components/link";
 import {
@@ -18,10 +19,13 @@ import { PublishTopic } from "../commonPublish/PublishTopic";
 
 export const kWebSocketContext = "libraryPublish";
 
-export const LibraryPublishScreen = () => {
+export const LibraryPublishScreen: React.FunctionComponent<{
+    // Passed straight through to LibraryPublishSteps, which owns the upload state.
+    onUploadingChange?: (uploading: boolean) => void;
+}> = (props) => {
     const mainPanel = (
         <PublishPanel>
-            <LibraryPublishSteps />
+            <LibraryPublishSteps onUploadingChange={props.onUploadingChange} />
         </PublishPanel>
     );
 

src/BloomBrowserUI/publish/LibraryPublish/LibraryPublishSteps.tsx

@@ -62,7 +62,11 @@ interface IReadonlyBookInfo {
 const kWebSocketEventId_uploadSuccessful: string = "uploadSuccessful";
 const kWebSocketEventId_uploadCanceled: string = "uploadCanceled";
 
-export const LibraryPublishSteps: React.FunctionComponent = () => {
+export const LibraryPublishSteps: React.FunctionComponent<{
+    // Called with true once the user commits to an upload and false when it is over. See the
+    // effect below for why the publish-tab host may only OR this into its lock.
+    onUploadingChange?: (uploading: boolean) => void;
+}> = (props) => {
     const selectedBookContext = React.useContext(SelectedBookContext);
     const [bookshelfHasProblem, setBookshelfHasProblem] = useState(false);
     const {
@@ -218,28 +222,62 @@ export const LibraryPublishSteps: React.FunctionComponent = () => {
     const [conflictIndex, setConflictIndex] = useState<number>(0);
 
     const [isUploading, setIsUploading] = useState<boolean>(false);
+    // Tell the publish-tab host as soon as the user commits to an upload, so it can lock the
+    // publish-tool switcher for the whole operation and not just the part C# knows about. C#
+    // takes its lock inside UploadBookAsync, but by then we have already made two API round
+    // trips (the subscription check and the "existing copy on server" query), during which the
+    // screen shows Cancel and a progress log while the tools were still live (BL-16654).
+    // The host OR-s this with C#'s lock rather than replacing it. That direction matters: this
+    // flag is not trustworthy as an *unlock* signal — clicking Cancel clears it while C# keeps
+    // working, and so does any error line in the progress log — but as an extra *lock* term it
+    // can only ever lock more than C# would, never less, so the unreliability is harmless here
+    // and C# stays the authority on when things reopen.
+    //
+    // Why an effect rather than doing this in the handler that starts the upload: isUploading has
+    // no single originating handler. It is set in uploadOneBook and cleared from four unrelated
+    // places — the Cancel button, the uploadSuccessful websocket, an error line in the progress
+    // box, and the collision dialog's cancel — so a render keyed on the resulting value is the
+    // only place that observes every transition. What we are doing is synchronizing an external
+    // system (the publish-tab host) to this state, which is what effects are for.
+    useEffect(() => {
+        props.onUploadingChange?.(isUploading);
+        // Never leave the host locked if this screen goes away mid-upload.
+        return () => props.onUploadingChange?.(false);
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [isUploading]);
     function uploadOneBook() {
         setIsUploadComplete(false);
         setIsUploading(true);
-        get("libraryPublish/checkSubscriptionMatch", (result) => {
-            if (result.data.error) {
-                // The API already sent an error message
-                return;
-            }
-            get(
-                "libraryPublish/getUploadCollisionInfo?index=" + conflictIndex,
-                (result) => {
-                    if (result.data.error) {
-                        // The API already sent an error message
-                        return;
-                    }
-                    if (result.data.shouldShow) {
-                        setUploadCollisionInfo(result.data);
-                        showUploadCollisionDialog();
-                    } else post("libraryPublish/upload");
-                },
-            );
-        });
+        // If either pre-upload request dies at the transport level we get no reply and no progress
+        // message, so nothing else would ever clear isUploading. That used to leave only a stale
+        // Cancel button, but now it would also keep the other publish tools disabled, so clear it
+        // here. (An error *reported by* the API still arrives as a progress message and is handled
+        // by handleUploadError.)
+        get(
+            "libraryPublish/checkSubscriptionMatch",
+            (result) => {
+                if (result.data.error) {
+                    // The API already sent an error message
+                    return;
+                }
+                get(
+                    "libraryPublish/getUploadCollisionInfo?index=" +
+                        conflictIndex,
+                    (result) => {
+                        if (result.data.error) {
+                            // The API already sent an error message
+                            return;
+                        }
+                        if (result.data.shouldShow) {
+                            setUploadCollisionInfo(result.data);
+                            showUploadCollisionDialog();
+                        } else post("libraryPublish/upload");
+                    },
+                    handleUploadError,
+                );
+            },
+            handleUploadError,
+        );
     }
 
     const changeConflictIndex = (index: number) => {

src/BloomBrowserUI/publish/PublishTab/PublishTabPane.tsx

@@ -30,6 +30,7 @@ import {
 import { AboutDialogLauncher } from "../../react_components/aboutDialog";
 import { RegistrationDialogEventLauncher } from "../../react_components/registration/registrationDialogLauncher";
 import { RequiresSubscriptionOverlayWrapper } from "../../react_components/requiresSubscription";
+import { useWorkspaceTabInfo } from "../../react_components/TopBar/TopBar";
 
 export const CheckoutNeededScreen: React.FunctionComponent<{
     titleForDisplay: string;
@@ -108,9 +109,31 @@ export const PublishTabPane: React.FunctionComponent = () => {
     const [tabIndex, setTabIndex] = React.useState(
         kWaitForUserToChooseTabIndex,
     );
-    // True while the Apps tool has a Reading App Builder action running (its Cancel button is
-    // showing). While busy, switching to another publish tool is blocked so the operation is modal.
-    const [appsBusy, setAppsBusy] = React.useState(false);
+    // True while a long-running publish operation has made itself modal by locking navigation:
+    // a BloomLibrary upload, or one of the Apps tool's Reading App Builder actions. C# owns this
+    // flag (it is the same one that greys out the main workspace tabs), so the publish tools
+    // unlock at exactly the moment the operation really finishes or is cancelled — not when the
+    // browser guesses it has. See BL-16654.
+    const navigationLocked = useWorkspaceTabInfo().navigationLocked;
+    // The Web tool tells us directly when the user has committed to an upload, because C# does
+    // not take its lock until a couple of API round trips later, leaving a window where the
+    // screen already shows Cancel but the tools were still clickable (BL-16654).
+    const [uploadUnderway, setUploadUnderway] = React.useState(false);
+    // OR, never AND. C#'s flag is the authority on when an operation has really finished, and
+    // uploadUnderway is deliberately only ever an *additional* reason to lock: it is unreliable
+    // as an unlock signal (it clears the moment Cancel is pressed, and on any error line in the
+    // progress log) but adding it can only lock more than C# alone would, never less.
+    //
+    // Both are then gated on a tool actually showing. The lock exists to stop the user walking
+    // away from an operation in progress, and none can be in progress while the sentinel "no tool
+    // chosen yet" panel is up. That gate matters because the C# flag is shared with other
+    // subsystems — e.g. the Copyright and License dialog, reachable from this tab's own "Missing
+    // Copyright" link, posts editView/setModalState, which locks. Without it, a lock still set
+    // while tabIndex is the sentinel would grey out every tool at once and leave the user no way
+    // to choose one at all.
+    const publishToolsLocked =
+        (navigationLocked || uploadUnderway) &&
+        tabIndex !== kWaitForUserToChooseTabIndex;
     const appBuilderFeatureStatus = useGetFeatureStatus("AppBuilder");
     const setup = () => {
         setTabIndex(kWaitForUserToChooseTabIndex);
@@ -242,11 +265,11 @@ export const PublishTabPane: React.FunctionComponent = () => {
                             labelBackgroundColor={kPanelBackground}
                             selectedIndex={tabIndex}
                             onSelect={(newIndex) => {
-                                // While a Reading App Builder action is running (its Cancel button
-                                // is showing), the Apps operation is modal: veto switching to another
-                                // publish tool until it finishes or is cancelled. The main workspace
-                                // tabs are locked from C# (RabPublishApi) to match.
-                                if (appsBusy) {
+                                // While an upload or an Apps action is running (its Cancel button is
+                                // showing), the operation is modal: veto switching to another publish
+                                // tool until it finishes or is cancelled. The main workspace tabs are
+                                // locked from C# by the same flag.
+                                if (publishToolsLocked) {
                                     return false;
                                 }
                                 post("publish/switchingPublishMode");
@@ -311,7 +334,7 @@ export const PublishTabPane: React.FunctionComponent = () => {
                                     display: none;
                                 }
                                 // Doubled class for enough specificity to override the tab color
-                                // rule above, so tools disabled during a modal Apps action read as
+                                // rule above, so tools disabled during a modal operation read as
                                 // greyed out (react-tabs already makes them non-clickable).
                                 .react-tabs__tab--disabled.react-tabs__tab--disabled {
                                     opacity: 0.4;
@@ -331,9 +354,14 @@ export const PublishTabPane: React.FunctionComponent = () => {
                                 {publishTabs.map((tab, index) => (
                                     <Tab
                                         key={index}
-                                        // Grey out the other publish tools while an Apps action is
-                                        // running, so it's clear the operation is modal.
-                                        disabled={appsBusy && tab.id !== "apps"}
+                                        // Grey out the other publish tools while a modal operation
+                                        // is running, so it's clear why they don't respond. The tool
+                                        // the operation belongs to stays looking normal, the same way
+                                        // C# leaves the active workspace tab looking active.
+                                        disabled={
+                                            publishToolsLocked &&
+                                            index !== tabIndex
+                                        }
                                         className={
                                             tab.hidden
                                                 ? "invisible_tab"
@@ -365,7 +393,9 @@ export const PublishTabPane: React.FunctionComponent = () => {
                             </TabPanel>
                             <TabPanel>
                                 {publishTabInfo.canUpload ? (
-                                    <LibraryPublishScreen />
+                                    <LibraryPublishScreen
+                                        onUploadingChange={setUploadUnderway}
+                                    />
                                 ) : (
                                     <WarningBox
                                         css={css`
@@ -393,7 +423,6 @@ export const PublishTabPane: React.FunctionComponent = () => {
                                         isActive={
                                             publishTabs[tabIndex]?.id === "apps"
                                         }
-                                        onBusyChange={setAppsBusy}
                                     />
                                 </RequiresSubscriptionOverlayWrapper>
                             </TabPanel>

src/BloomBrowserUI/react_components/TopBar/TopBar.tsx

@@ -22,6 +22,16 @@ export type WorkspaceTabState = "active" | "enabled" | "disabled" | "hidden";
 
 export type TabStates = Record<WorkspaceTabId, WorkspaceTabState>;
 
+// What C# (WorkspaceView.GetTabInfo) tells us about workspace navigation.
+export interface IWorkspaceTabInfo {
+    tabStates: TabStates;
+    // True while some operation has made itself modal by locking navigation: a BloomLibrary
+    // upload, a Reading App Builder action, or an Edit-tab modal dialog. Screens with their own
+    // navigation (notably the Publish tab's switcher between publish tools) use this to lock in
+    // step with the main tabs.
+    navigationLocked: boolean;
+}
+
 interface ITabDefinition {
     id: WorkspaceTabId;
     l10nId: string;
@@ -57,21 +67,28 @@ export function getActiveWorkspaceTab(tabStates: TabStates): WorkspaceTabId {
     );
 }
 
-export const defaultWorkspaceTabState: { tabStates: TabStates } = {
+export const defaultWorkspaceTabState: IWorkspaceTabInfo = {
     tabStates: {
         collection: "active",
         edit: "hidden",
         publish: "hidden",
     },
+    navigationLocked: false,
 };
 
-export const TopBar: React.FunctionComponent = () => {
-    const state = useWatchApiObject<{ tabStates: TabStates }>(
+// Subscribes to what C# says about workspace navigation, kept in one place because several
+// screens in different browser controls need the same answer.
+export function useWorkspaceTabInfo(): IWorkspaceTabInfo {
+    return useWatchApiObject<IWorkspaceTabInfo>(
         "workspace/tabs",
         defaultWorkspaceTabState,
         "workspace",
         "tabs",
     );
+}
+
+export const TopBar: React.FunctionComponent = () => {
+    const state = useWorkspaceTabInfo();
     const topBarRef = React.useRef<HTMLDivElement>(null);
 
     const tabStates = state.tabStates ?? defaultWorkspaceTabState.tabStates;