Skip to content

Provide explicit Cancel for Publish>Apps and make it modal while running (BL-16350) #8133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
StephenMcConnel wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Provide explicit Cancel for Publish>Apps and make it modal while running (BL-16350) #8133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e7d540
Provide explicit Cancel for Publish>Apps and make it modal while runn…
StephenMcConnel Jul 30, 2026
67d4795
Merge remote-tracking branch 'origin/master' into BL-16350-ProvideCan…
StephenMcConnel Jul 30, 2026
815a2c4
Honor a pending cancel before launching the next RAB/adb subprocess (…
StephenMcConnel Jul 30, 2026
bf4a6e0
Don't report a download timeout (or other library cancellation) as a …
StephenMcConnel Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 95 additions & 24 deletions src/BloomBrowserUI/publish/Apps/AppPublisherScreen.tsx
View file
Open in desktop

@StephenMcConnel StephenMcConnel Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Devin] Bug: Busy state is pushed to the parent screen from a useEffect, which the repo's React guidance lists as a "don't"

The Apps screen notifies its host that an action is running from inside a React.useEffect watching screenState.busyAction. src/BloomBrowserUI/AGENTS.md → .github/skills/react-useeffect/SKILL.md lists "Notify parent of changes" as a DON'T for effects (DO: call in an event handler). (src/BloomBrowserUI/publish/Apps/AppPublisherScreen.tsx)

Sent to the developer as a decision (see the preflight report): busyAction here is derived from several asynchronous sources (the websocket action-complete event, status-poll recovery, tab-activation restore), not a single user event, so an effect that mirrors it up is arguably the right tool — but the fuller refactor (pass the callback into the hook, notify at each state change, keep an unmount-cleanup) is available if strict compliance is preferred.

Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,23 @@ 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);
// Report the running/Cancel-showing state up to the publish-tab host so it can make the
// operation modal: while an action runs, the host blocks switching to another publish tool
// (and C# blocks the main workspace tabs). 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]);
const prepareTooltip = useL10n(
"Create the Reading App Builder project in this collection's Bloom App Data folder.",
"PublishTab.Apps.Prepare.TooltipBloomAppData",
Expand Down Expand Up @@ -355,17 +365,39 @@ const AppPublisherScreenContents: React.FunctionComponent<{
>
<Step expanded={true} completed={false}>
<StepLabel>
<AppActionButton
enabled={canRunPrepare}
l10nKey="PublishTab.Apps.PrepareButton"
onClick={() =>
screenState.runAction("prepare")
}
size="large"
tooltip={prepareTooltipToShow}
<div
css={css`
display: flex;
flex-wrap: wrap;
gap: 12px;
align-items: flex-start;
`}
>
{prepareButtonLabel}
</AppActionButton>
<AppActionButton
enabled={canRunPrepare}
l10nKey="PublishTab.Apps.PrepareButton"
onClick={() =>
screenState.runAction("prepare")
}
size="large"
tooltip={prepareTooltipToShow}
>
{prepareButtonLabel}
</AppActionButton>
{busyAction === "prepare" && (
<AppActionButton
enabled={true}
l10nKey="Common.Cancel"
onClick={() =>
screenState.cancelAction()
}
size="large"
variant="outlined"
>
Cancel
</AppActionButton>
)}
</div>
</StepLabel>
<StepContent>
<PrepareAppStepper
Expand Down Expand Up @@ -459,20 +491,42 @@ const AppPublisherScreenContents: React.FunctionComponent<{
</Step>
<Step expanded={true} completed={false}>
<StepLabel>
<AppActionButton
enabled={prepareIsReady && canRunBuild}
l10nKey="PublishTab.Apps.Build"
onClick={() =>
screenState.runAction("build")
}
size="large"
tooltip={buildTooltipToShow}
iconBeforeText={
<PrecisionManufacturingIcon />
}
<div
css={css`
display: flex;
flex-wrap: wrap;
gap: 12px;
align-items: flex-start;
`}
>
Build
</AppActionButton>
<AppActionButton
enabled={prepareIsReady && canRunBuild}
l10nKey="PublishTab.Apps.Build"
onClick={() =>
screenState.runAction("build")
}
size="large"
tooltip={buildTooltipToShow}
iconBeforeText={
<PrecisionManufacturingIcon />
}
>
Build
</AppActionButton>
{busyAction === "build" && (
<AppActionButton
enabled={true}
l10nKey="Common.Cancel"
onClick={() =>
screenState.cancelAction()
}
size="large"
variant="outlined"
>
Cancel
</AppActionButton>
)}
</div>
</StepLabel>
<StepContent>
<InlineProgressStatus
Expand Down Expand Up @@ -530,6 +584,19 @@ const AppPublisherScreenContents: React.FunctionComponent<{
>
Try on phone
</AppActionButton>
{busyAction === "install" && (
<AppActionButton
enabled={true}
l10nKey="Common.Cancel"
onClick={() =>
screenState.cancelAction()
}
size="large"
variant="outlined"
>
Cancel
</AppActionButton>
)}
<AppActionButton
enabled={canUseCurrentApk}
l10nKey="PublishTab.Apps.ShowApkInFileExplorer"
Expand Down Expand Up @@ -658,6 +725,7 @@ const AppPublisherScreenContents: React.FunctionComponent<{

export const AppPublisherScreen: React.FunctionComponent<{
isActive: boolean;
onBusyChange?: (busy: boolean) => void;
}> = (props) => {
const optionsPanel = (
<SettingsPanel>
Expand Down Expand Up @@ -696,7 +764,10 @@ 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} />
<AppPublisherScreenContents
isActive={props.isActive}
onBusyChange={props.onBusyChange}
/>
</PublishScreenTemplate>
</Typography>
);
Expand Down
6 changes: 6 additions & 0 deletions src/BloomBrowserUI/publish/Apps/useAppBuilderPublisherScreen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export interface IAppBuilderPublisherScreenState {
progressStageCode?: string;
sizeEstimates: IAppSizeEstimates;
runAction: (action: AppBuilderAction) => void;
cancelAction: () => void;
showApkInExplorerInShell: () => void;
markConfigurationChanged: () => void;
}
Expand Down Expand Up @@ -448,6 +449,10 @@ export function useAppBuilderPublisherScreen(
void postJson("fileIO/showInFolder", { folderPath: status.apkPath });
}

function cancelAction(): void {
post("publish/rab/cancel");
}

function markConfigurationChanged(): void {
setPendingBuildNeeded(true);
void refreshStatus();
Expand All @@ -467,6 +472,7 @@ export function useAppBuilderPublisherScreen(
progressStageCode,
sizeEstimates,
runAction,
cancelAction,
showApkInExplorerInShell,
markConfigurationChanged,
};
Expand Down
21 changes: 21 additions & 0 deletions src/BloomBrowserUI/publish/PublishTab/PublishTabPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ 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);
const appBuilderFeatureStatus = useGetFeatureStatus("AppBuilder");
const setup = () => {
setTabIndex(kWaitForUserToChooseTabIndex);
Expand Down Expand Up @@ -239,6 +242,13 @@ 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) {
return false;
}
post("publish/switchingPublishMode");
logPublishTabSelected(newIndex);
setTabIndex(newIndex);
Expand Down Expand Up @@ -300,12 +310,22 @@ export const PublishTabPane: React.FunctionComponent = () => {
.invisible_tab {
display: none;
}
// Doubled class for enough specificity to override the tab color
// rule above, so tools disabled during a modal Apps action read as
// greyed out (react-tabs already makes them non-clickable).
.react-tabs__tab--disabled.react-tabs__tab--disabled {
opacity: 0.4;
cursor: default;
}
`}
>
<TabList>
{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"}
className={
tab.hidden
? "invisible_tab"
Expand Down Expand Up @@ -365,6 +385,7 @@ export const PublishTabPane: React.FunctionComponent = () => {
isActive={
publishTabs[tabIndex]?.id === "apps"
}
onBusyChange={setAppsBusy}
/>
</RequiresSubscriptionOverlayWrapper>
</TabPanel>
Expand Down
Loading