From ad1a1429de42483a6e327a22b91eea362753e107 Mon Sep 17 00:00:00 2001
From: Andre Wanlin <67169551+awanlin@users.noreply.github.com>
Date: Fri, 14 Apr 2023 11:36:03 -0500
Subject: [PATCH] Improvements to the scaffolder/next buttons UX

Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com>
---
 .changeset/rotten-chefs-jog.md                | 10 +++
 .../src/next/components/Stepper/Stepper.tsx   |  3 +-
 .../src/next/OngoingTask/ContextMenu.tsx      | 22 ++++++-
 .../src/next/OngoingTask/OngoingTask.test.tsx | 37 ++++++++++-
 .../src/next/OngoingTask/OngoingTask.tsx      | 63 +++++++++++++++++--
 5 files changed, 125 insertions(+), 10 deletions(-)
 create mode 100644 .changeset/rotten-chefs-jog.md

diff --git a/.changeset/rotten-chefs-jog.md b/.changeset/rotten-chefs-jog.md
new file mode 100644
index 0000000000..120520c087
--- /dev/null
+++ b/.changeset/rotten-chefs-jog.md
@@ -0,0 +1,10 @@
+---
+'@backstage/plugin-scaffolder-react': patch
+'@backstage/plugin-scaffolder': patch
+---
+
+Improvements to the `scaffolder/next` buttons UX:
+
+- Added padding around the "Create" button in the `Stepper` component
+- Added a button bar that includes the "Cancel" and "Start Over" buttons to the `OngoingTask` component. The state of these buttons match their existing counter parts in the Context Menu
+- Added a "Show Button Bar"/"Hide Button Bar" item to the `ContextMenu` component
diff --git a/plugins/scaffolder-react/src/next/components/Stepper/Stepper.tsx b/plugins/scaffolder-react/src/next/components/Stepper/Stepper.tsx
index 946cc583a3..55b5703ecf 100644
--- a/plugins/scaffolder-react/src/next/components/Stepper/Stepper.tsx
+++ b/plugins/scaffolder-react/src/next/components/Stepper/Stepper.tsx
@@ -49,11 +49,11 @@ const useStyles = makeStyles(theme => ({
   backButton: {
     marginRight: theme.spacing(1),
   },
-
   footer: {
     display: 'flex',
     flexDirection: 'row',
     justifyContent: 'right',
+    padding: theme.spacing(2),
   },
   formWrapper: {
     padding: theme.spacing(2),
@@ -216,6 +216,7 @@ export const Stepper = (stepperProps: StepperProps) => {
               </Button>
               <Button
                 variant="contained"
+                color="primary"
                 onClick={() => {
                   props.onCreate(formState);
                   const name =
diff --git a/plugins/scaffolder/src/next/OngoingTask/ContextMenu.tsx b/plugins/scaffolder/src/next/OngoingTask/ContextMenu.tsx
index 21db9f5b08..166c26e854 100644
--- a/plugins/scaffolder/src/next/OngoingTask/ContextMenu.tsx
+++ b/plugins/scaffolder/src/next/OngoingTask/ContextMenu.tsx
@@ -28,6 +28,7 @@ import { useAsync } from '@react-hookz/web';
 import Cancel from '@material-ui/icons/Cancel';
 import Retry from '@material-ui/icons/Repeat';
 import Toc from '@material-ui/icons/Toc';
+import ControlPointIcon from '@material-ui/icons/ControlPoint';
 import MoreVert from '@material-ui/icons/MoreVert';
 import React, { useState } from 'react';
 import { useApi } from '@backstage/core-plugin-api';
@@ -36,8 +37,10 @@ import { scaffolderApiRef } from '@backstage/plugin-scaffolder-react';
 type ContextMenuProps = {
   cancelEnabled?: boolean;
   logsVisible?: boolean;
+  buttonBarVisible?: boolean;
   onStartOver?: () => void;
   onToggleLogs?: (state: boolean) => void;
+  onToggleButtonBar?: (state: boolean) => void;
   taskId?: string;
 };
 
@@ -48,8 +51,15 @@ const useStyles = makeStyles<BackstageTheme, { fontColor: string }>(() => ({
 }));
 
 export const ContextMenu = (props: ContextMenuProps) => {
-  const { cancelEnabled, logsVisible, onStartOver, onToggleLogs, taskId } =
-    props;
+  const {
+    cancelEnabled,
+    logsVisible,
+    buttonBarVisible,
+    onStartOver,
+    onToggleLogs,
+    onToggleButtonBar,
+    taskId,
+  } = props;
   const { getPageTheme } = useTheme<BackstageTheme>();
   const pageTheme = getPageTheme({ themeId: 'website' });
   const classes = useStyles({ fontColor: pageTheme.fontColor });
@@ -90,6 +100,14 @@ export const ContextMenu = (props: ContextMenuProps) => {
             </ListItemIcon>
             <ListItemText primary={logsVisible ? 'Hide Logs' : 'Show Logs'} />
           </MenuItem>
+          <MenuItem onClick={() => onToggleButtonBar?.(!buttonBarVisible)}>
+            <ListItemIcon>
+              <ControlPointIcon fontSize="small" />
+            </ListItemIcon>
+            <ListItemText
+              primary={buttonBarVisible ? 'Hide Button Bar' : 'Show Button Bar'}
+            />
+          </MenuItem>
           <MenuItem onClick={onStartOver}>
             <ListItemIcon>
               <Retry fontSize="small" />
diff --git a/plugins/scaffolder/src/next/OngoingTask/OngoingTask.test.tsx b/plugins/scaffolder/src/next/OngoingTask/OngoingTask.test.tsx
index c31322a9eb..16d9740c15 100644
--- a/plugins/scaffolder/src/next/OngoingTask/OngoingTask.test.tsx
+++ b/plugins/scaffolder/src/next/OngoingTask/OngoingTask.test.tsx
@@ -18,7 +18,7 @@ import { OngoingTask } from './OngoingTask';
 import React from 'react';
 import { renderInTestApp, TestApiProvider } from '@backstage/test-utils';
 import { scaffolderApiRef } from '@backstage/plugin-scaffolder-react';
-import { act, fireEvent, waitFor } from '@testing-library/react';
+import { act, fireEvent, waitFor, within } from '@testing-library/react';
 import { rootRouteRef } from '../../routes';
 
 jest.mock('react-router-dom', () => ({
@@ -61,7 +61,7 @@ describe('OngoingTask', () => {
       </TestApiProvider>,
       { mountedRoutes: { '/': rootRouteRef } },
     );
-    const { getByText, getByTestId } = rendered;
+    const { getByTestId } = rendered;
 
     await act(async () => {
       fireEvent.click(getByTestId('menu-button'));
@@ -69,7 +69,8 @@ describe('OngoingTask', () => {
     expect(getByTestId('cancel-task')).not.toHaveClass('Mui-disabled');
 
     await act(async () => {
-      fireEvent.click(getByText(cancelOptionLabel));
+      const element = getByTestId('cancel-task');
+      fireEvent.click(within(element).getByText(cancelOptionLabel));
     });
 
     expect(mockScaffolderApi.cancelTask).toHaveBeenCalled();
@@ -81,4 +82,34 @@ describe('OngoingTask', () => {
       expect(getByTestId('cancel-task')).toHaveClass('Mui-disabled');
     });
   });
+
+  it('should trigger cancel api on "Cancel" button click', async () => {
+    const cancelOptionLabel = 'Cancel';
+    const rendered = await renderInTestApp(
+      <TestApiProvider apis={[[scaffolderApiRef, mockScaffolderApi]]}>
+        <OngoingTask />
+      </TestApiProvider>,
+      { mountedRoutes: { '/': rootRouteRef } },
+    );
+    const { getByTestId } = rendered;
+
+    await act(async () => {
+      fireEvent.click(getByTestId('menu-button'));
+    });
+    expect(getByTestId('cancel-button')).not.toHaveClass('Mui-disabled');
+
+    await act(async () => {
+      const element = getByTestId('cancel-button');
+      fireEvent.click(within(element).getByText(cancelOptionLabel));
+    });
+
+    expect(mockScaffolderApi.cancelTask).toHaveBeenCalled();
+    await act(async () => {
+      fireEvent.click(getByTestId('menu-button'));
+    });
+
+    await waitFor(() => {
+      expect(getByTestId('cancel-button')).toHaveClass('Mui-disabled');
+    });
+  });
 });
diff --git a/plugins/scaffolder/src/next/OngoingTask/OngoingTask.tsx b/plugins/scaffolder/src/next/OngoingTask/OngoingTask.tsx
index f8026cf073..91c04897d9 100644
--- a/plugins/scaffolder/src/next/OngoingTask/OngoingTask.tsx
+++ b/plugins/scaffolder/src/next/OngoingTask/OngoingTask.tsx
@@ -16,13 +16,14 @@
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import { Content, ErrorPanel, Header, Page } from '@backstage/core-components';
 import { useNavigate, useParams } from 'react-router-dom';
-import { Box, makeStyles, Paper } from '@material-ui/core';
+import { Box, Button, makeStyles, Paper } from '@material-ui/core';
 import {
   ScaffolderTaskOutput,
+  scaffolderApiRef,
   useTaskEventStream,
 } from '@backstage/plugin-scaffolder-react';
 import { selectedTemplateRouteRef } from '../../routes';
-import { useRouteRef } from '@backstage/core-plugin-api';
+import { useApi, useRouteRef } from '@backstage/core-plugin-api';
 import qs from 'qs';
 import { ContextMenu } from './ContextMenu';
 import {
@@ -30,13 +31,22 @@ import {
   TaskLogStream,
   TaskSteps,
 } from '@backstage/plugin-scaffolder-react/alpha';
+import { useAsync } from '@react-hookz/web';
 
-const useStyles = makeStyles({
+const useStyles = makeStyles(theme => ({
   contentWrapper: {
     display: 'flex',
     flexDirection: 'column',
   },
-});
+  buttonBar: {
+    display: 'flex',
+    flexDirection: 'row',
+    justifyContent: 'right',
+  },
+  cancelButton: {
+    marginRight: theme.spacing(1),
+  },
+}));
 
 export const OngoingTask = (props: {
   TemplateOutputsComponent?: React.ComponentType<{
@@ -47,6 +57,7 @@ export const OngoingTask = (props: {
   const { taskId } = useParams();
   const templateRouteRef = useRouteRef(selectedTemplateRouteRef);
   const navigate = useNavigate();
+  const scaffolderApi = useApi(scaffolderApiRef);
   const taskStream = useTaskEventStream(taskId!);
   const classes = useStyles();
   const steps = useMemo(
@@ -59,6 +70,7 @@ export const OngoingTask = (props: {
   );
 
   const [logsVisible, setLogVisibleState] = useState(false);
+  const [buttonBarVisible, setButtonBarVisibleState] = useState(true);
 
   useEffect(() => {
     if (taskStream.error) {
@@ -66,6 +78,12 @@ export const OngoingTask = (props: {
     }
   }, [taskStream.error]);
 
+  useEffect(() => {
+    if (taskStream.completed && !taskStream.error) {
+      setButtonBarVisibleState(false);
+    }
+  }, [taskStream.error, taskStream.completed]);
+
   const activeStep = useMemo(() => {
     for (let i = steps.length - 1; i >= 0; i--) {
       if (steps[i].status !== 'open') {
@@ -100,6 +118,14 @@ export const OngoingTask = (props: {
     templateRouteRef,
   ]);
 
+  const [{ status: cancelStatus }, { execute: triggerCancel }] = useAsync(
+    async () => {
+      if (taskId) {
+        await scaffolderApi.cancelTask(taskId);
+      }
+    },
+  );
+
   const Outputs = props.TemplateOutputsComponent ?? DefaultTemplateOutputs;
 
   const templateName =
@@ -121,8 +147,10 @@ export const OngoingTask = (props: {
         <ContextMenu
           cancelEnabled={cancelEnabled}
           logsVisible={logsVisible}
+          buttonBarVisible={buttonBarVisible}
           onStartOver={startOver}
           onToggleLogs={setLogVisibleState}
+          onToggleButtonBar={setButtonBarVisibleState}
           taskId={taskId}
         />
       </Header>
@@ -147,6 +175,33 @@ export const OngoingTask = (props: {
 
         <Outputs output={taskStream.output} />
 
+        {buttonBarVisible ? (
+          <Box paddingBottom={2}>
+            <Paper>
+              <Box padding={2}>
+                <div className={classes.buttonBar}>
+                  <Button
+                    className={classes.cancelButton}
+                    disabled={!cancelEnabled || cancelStatus !== 'not-executed'}
+                    onClick={triggerCancel}
+                    data-testid="cancel-button"
+                  >
+                    Cancel
+                  </Button>
+                  <Button
+                    variant="contained"
+                    color="primary"
+                    disabled={cancelEnabled}
+                    onClick={startOver}
+                  >
+                    Start Over
+                  </Button>
+                </div>
+              </Box>
+            </Paper>
+          </Box>
+        ) : null}
+
         {logsVisible ? (
           <Box paddingBottom={2} height="100%">
             <Paper style={{ height: '100%' }}>