Improvements to the scaffolder/next buttons UX

Signed-off-by: Andre Wanlin <67169551+awanlin@users.noreply.github.com>
2023-04-14 11:36:03 -05:00
parent 8379de0eb2
commit ad1a1429de
5 changed files with 125 additions and 10 deletions
@@ -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
@@ -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 =
@@ -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" />
@@ -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');
+    });
+  });
 });
@@ -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%' }}>