ui: update EP step form destination UX (#3897)

* add notice confirmation if no destinations on ep step * update text field margins * update destinations padding, button, and wording * wording * update no actions text * add tooltip * fix test * Merge branch 'master' into stepform-ux Squashed commit of the following: commit ea1eb53 Author: Nathaniel Caza <mastercactapus@gmail.com> Date: Wed Jun 12 13:25:26 2024 -0500 dest/action: simplify error flow in forms (#3926) * simplify error flow in forms * add comment and return type commit 5420402 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue Jun 11 10:45:57 2024 -0500 build(deps-dev): bump @storybook/test-runner from 0.17.0 to 0.18.2 (#3911) Bumps [@storybook/test-runner](https://github.com/storybookjs/test-runner) from 0.17.0 to 0.18.2. - [Release notes](https://github.com/storybookjs/test-runner/releases) - [Changelog](https://github.com/storybookjs/test-runner/blob/v0.18.2/CHANGELOG.md) - [Commits](storybookjs/test-runner@v0.17.0...v0.18.2) --- updated-dependencies: - dependency-name: "@storybook/test-runner" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit b2231df Author: Nathaniel Caza <mastercactapus@gmail.com> Date: Tue Jun 11 10:04:01 2024 -0500 switch from global to per-rule continue option (#3924) commit 97c12af Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue Jun 11 10:03:02 2024 -0500 build(deps): bump golang.org/x/crypto from 0.23.0 to 0.24.0 (#3919) Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.23.0 to 0.24.0. - [Commits](golang/crypto@v0.23.0...v0.24.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 691e609 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue Jun 11 10:02:46 2024 -0500 build(deps-dev): bump @mui/material from 5.15.15 to 5.15.19 (#3921) Bumps [@mui/material](https://github.com/mui/material-ui/tree/HEAD/packages/mui-material) from 5.15.15 to 5.15.19. - [Release notes](https://github.com/mui/material-ui/releases) - [Changelog](https://github.com/mui/material-ui/blob/v5.15.19/CHANGELOG.md) - [Commits](https://github.com/mui/material-ui/commits/v5.15.19/packages/mui-material) --- updated-dependencies: - dependency-name: "@mui/material" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit df67326 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue Jun 11 10:02:29 2024 -0500 build(deps-dev): bump diff and @types/diff (#3922) Bumps [diff](https://github.com/kpdecker/jsdiff) and [@types/diff](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/diff). These dependencies needed to be updated together. Updates `diff` from 5.1.0 to 5.2.0 - [Changelog](https://github.com/kpdecker/jsdiff/blob/master/release-notes.md) - [Commits](kpdecker/jsdiff@v5.1.0...v5.2.0) Updates `@types/diff` from 5.0.8 to 5.2.1 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/diff) --- updated-dependencies: - dependency-name: diff dependency-type: direct:development update-type: version-update:semver-minor - dependency-name: "@types/diff" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 4af6332 Author: Nathaniel Cook <NathanielJCook@outlook.com> Date: Mon Jun 10 06:33:04 2024 -0700 ui: fix step number display for EP step delay text (#3916) * fix index on caption * add playwright test * add return type of void commit b292f68 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri Jun 7 10:20:16 2024 -0500 build(deps-dev): bump react-big-calendar and @types/react-big-calendar (#3900) Bumps [react-big-calendar](https://github.com/jquense/react-big-calendar) and [@types/react-big-calendar](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react-big-calendar). These dependencies needed to be updated together. Updates `react-big-calendar` from 1.8.5 to 1.12.2 - [Release notes](https://github.com/jquense/react-big-calendar/releases) - [Changelog](https://github.com/jquense/react-big-calendar/blob/master/CHANGELOG.md) - [Commits](jquense/react-big-calendar@v1.8.5...v1.12.2) Updates `@types/react-big-calendar` from 1.6.5 to 1.8.9 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react-big-calendar) --- updated-dependencies: - dependency-name: react-big-calendar dependency-type: direct:development update-type: version-update:semver-minor - dependency-name: "@types/react-big-calendar" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> commit 489dbf3 Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon Jun 3 08:26:20 2024 -0700 build(deps-dev): bump @typescript-eslint/eslint-plugin (#3913) Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 7.7.0 to 7.11.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v7.11.0/packages/eslint-plugin) --- updated-dependencies: - dependency-name: "@typescript-eslint/eslint-plugin" dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix mobile cypress test --------- Co-authored-by: AllenDing <Allen.Ding@target.com>
target · Jun 24, 2024 · 6f220c6 · 6f220c6
1 parent ea1eb53
commit 6f220c6
Show file tree

Hide file tree

Showing 11 changed files with 112 additions and 25 deletions.
diff --git a/web/src/app/dialogs/FormDialog.jsx b/web/src/app/dialogs/FormDialog.jsx
@@ -26,7 +26,7 @@ const useStyles = makeStyles((theme) => {
     },
     dialogContent: {
       height: '100%', // parents of form need height set to properly function in Safari
-      paddingTop: '8px !important', // workaround for https://github.com/mui/material-ui/issues/31185
+      paddingTop: '8px',
     },
     formContainer: {
       width: '100%',

diff --git a/web/src/app/escalation-policies/PolicyStepCreateDialog.tsx b/web/src/app/escalation-policies/PolicyStepCreateDialog.tsx
@@ -4,6 +4,7 @@ import { splitErrorsByPath } from '../util/errutil'
 import FormDialog from '../dialogs/FormDialog'
 import PolicyStepForm, { FormValue } from './PolicyStepForm'
 import { errorPaths } from '../users/UserContactMethodForm'
+import { getNotice } from './utils'
 
 const mutation = gql`
   mutation createEscalationPolicyStep(
@@ -28,6 +29,10 @@ export default function PolicyStepCreateDialog(props: {
   const [createStepStatus, createStep] = useMutation(mutation)
   const [err, setErr] = useState<CombinedError | null>(null)
 
+  const [hasSubmitted, setHasSubmitted] = useState(false)
+  const [hasConfirmed, setHasConfirmed] = useState(false)
+  const noActionsNoConf = value.actions.length === 0 && !hasConfirmed
+
   useEffect(() => {
     setErr(null)
   }, [value])
@@ -50,6 +55,11 @@ export default function PolicyStepCreateDialog(props: {
       maxWidth='sm'
       onClose={props.onClose}
       onSubmit={() => {
+        if (noActionsNoConf) {
+          setHasSubmitted(true)
+          return
+        }
+
         createStep(
           {
             input: {
@@ -73,6 +83,7 @@ export default function PolicyStepCreateDialog(props: {
           onChange={(value: FormValue) => setValue(value)}
         />
       }
+      notices={getNotice(hasSubmitted, hasConfirmed, setHasConfirmed)}
     />
   )
 }
diff --git a/web/src/app/escalation-policies/PolicyStepEditDialog.tsx b/web/src/app/escalation-policies/PolicyStepEditDialog.tsx
@@ -9,6 +9,7 @@ import {
   FieldValuePair,
   UpdateEscalationPolicyStepInput,
 } from '../../schema'
+import { getNotice } from './utils'
 
 interface PolicyStepEditDialogProps {
   escalationPolicyID: string
@@ -69,6 +70,10 @@ export default function PolicyStepEditDialog(
 
   const [editStepStatus, editStep] = useMutation(mutation)
 
+  const [hasSubmitted, setHasSubmitted] = useState(false)
+  const [hasConfirmed, setHasConfirmed] = useState(false)
+  const noActionsNoConf = value.actions.length === 0 && !hasConfirmed
+
   // Edit dialog has no errors to be handled by the form:
   // - actions field has it's own validation
   // - errors on existing actions are not handled specially, and just display in the dialog (i.e., duplicates)
@@ -83,8 +88,13 @@ export default function PolicyStepEditDialog(
       disablePortal={props.disablePortal}
       maxWidth='sm'
       onClose={props.onClose}
-      onSubmit={() =>
-        editStep(
+      onSubmit={() => {
+        if (noActionsNoConf) {
+          setHasSubmitted(true)
+          return
+        }
+
+        return editStep(
           {
             input: {
               id: props.stepID,
@@ -96,14 +106,15 @@ export default function PolicyStepEditDialog(
         ).then((result) => {
           if (!result.error) props.onClose()
         })
-      }
+      }}
       form={
         <PolicyStepForm
           disabled={editStepStatus.fetching}
           value={value}
           onChange={(value: FormValue) => setValue(value)}
         />
       }
+      notices={getNotice(hasSubmitted, hasConfirmed, setHasConfirmed)}
     />
   )
 }
diff --git a/web/src/app/escalation-policies/PolicyStepForm.stories.tsx b/web/src/app/escalation-policies/PolicyStepForm.stories.tsx
@@ -143,6 +143,6 @@ export const ManageActions: Story = {
     // Delete the chip
     await userEvent.click(await canvas.findByTestId('CancelIcon'))
 
-    await expect(await canvas.findByText('No actions')).toBeVisible()
+    await expect(await canvas.findByText('No destinations')).toBeVisible()
   },
 }
diff --git a/web/src/app/escalation-policies/PolicyStepForm.tsx b/web/src/app/escalation-policies/PolicyStepForm.tsx
@@ -18,6 +18,7 @@ import {
 import { splitErrorsByPath } from '../util/errutil'
 import DialogContentError from '../dialogs/components/DialogContentError'
 import makeStyles from '@mui/styles/makeStyles'
+import { Add } from '../icons'
 
 const useStyles = makeStyles(() => {
   return {
@@ -96,18 +97,21 @@ export default function PolicyStepForm(props: PolicyStepFormProps): ReactNode {
       errors={props.errors}
     >
       <Grid container spacing={2}>
-        <Grid item xs={12}>
-          {props.value.actions.map((a, idx) => (
-            <DestinationInputChip
-              key={idx}
-              value={a}
-              onDelete={props.disabled ? undefined : () => handleDelete(a)}
-            />
+        <Grid container spacing={1} item xs={12} sx={{ p: 1 }}>
+          {props.value.actions.map((a) => (
+            <Grid item key={JSON.stringify(a.values)}>
+              <DestinationInputChip
+                value={a}
+                onDelete={props.disabled ? undefined : () => handleDelete(a)}
+              />
+            </Grid>
           ))}
           {props.value.actions.length === 0 && (
-            <Typography variant='body2' color='textSecondary'>
-              No actions
-            </Typography>
+            <Grid item xs={12}>
+              <Typography variant='body2' color='textSecondary'>
+                No destinations
+              </Typography>
+            </Grid>
           )}
         </Grid>
         <Grid item xs={12}>
@@ -147,6 +151,8 @@ export default function PolicyStepForm(props: PolicyStepFormProps): ReactNode {
           <Button
             variant='contained'
             color='secondary'
+            fullWidth
+            startIcon={<Add />}
             onClick={() => {
               if (!props.onChange) return
               validationClient

diff --git a/web/src/app/escalation-policies/PolicyStepsCard.tsx b/web/src/app/escalation-policies/PolicyStepsCard.tsx
@@ -115,12 +115,10 @@ export default function PolicyStepsCard(
         <CreateFAB onClick={() => setCreateStep(true)} title='Create Step' />
       )}
       {createStep && (
-        <React.Fragment>
-          <PolicyStepCreateDialog
-            escalationPolicyID={props.escalationPolicyID}
-            onClose={resetCreateStep}
-          />
-        </React.Fragment>
+        <PolicyStepCreateDialog
+          escalationPolicyID={props.escalationPolicyID}
+          onClose={resetCreateStep}
+        />
       )}
       <Card>
         <CardHeader

diff --git a/web/src/app/escalation-policies/stepUtil.tsx b/web/src/app/escalation-policies/stepUtil.tsx
@@ -5,11 +5,24 @@ import Grid from '@mui/material/Grid'
 import Typography from '@mui/material/Typography'
 import { Destination } from '../../schema'
 import DestinationChip from '../util/DestinationChip'
+import { Warning } from '../icons'
 
 export function renderChipsDest(_a: Destination[]): ReactElement {
   const actions = sortBy(_a.slice(), ['type', 'displayInfo.text'])
   if (!actions || actions.length === 0) {
-    return <Chip label='No actions' />
+    return (
+      <Chip
+        label='No destinations'
+        icon={
+          <div style={{ padding: '4px' }}>
+            <Warning
+              placement='bottom'
+              message='With no destinations configured, nothing will happen when an alert reaches this step'
+            />
+          </div>
+        }
+      />
+    )
   }
 
   const items = actions.map((a, idx) => {

diff --git a/web/src/app/escalation-policies/utils.tsx b/web/src/app/escalation-policies/utils.tsx
@@ -0,0 +1,31 @@
+import React from 'react'
+import { Notice } from '../details/Notices'
+import { FormControlLabel, Checkbox } from '@mui/material'
+
+export function getNotice(
+  hasSubmitted: boolean,
+  hasConfirmed: boolean,
+  setHasConfirmed: (b: boolean) => void,
+): Notice[] {
+  if (!hasSubmitted) return []
+
+  return [
+    {
+      type: 'WARNING',
+      message: 'No destinations',
+      details:
+        'If you submit with no destinations, nothing will happen on this step',
+      action: (
+        <FormControlLabel
+          control={
+            <Checkbox
+              checked={hasConfirmed}
+              onChange={() => setHasConfirmed(!hasConfirmed)}
+            />
+          }
+          label='I acknowledge the impact of this'
+        />
+      ),
+    },
+  ]
+}
diff --git a/web/src/app/theme/themeConfig.tsx b/web/src/app/theme/themeConfig.tsx
@@ -116,6 +116,11 @@ function makeTheme(mode: MUIThemeMode, sourceColor: string): Theme {
           color: 'primary',
         },
       },
+      MuiTextField: {
+        defaultProps: {
+          margin: 'dense',
+        },
+      },
       MuiBreadcrumbs: {
         styleOverrides: {
           separator: {

diff --git a/web/src/app/wizard/utilTestData.js b/web/src/app/wizard/utilTestData.js
@@ -8,15 +8,15 @@ const keys = [
     value: 'generic',
   },
   {
-    label: 'Grafana Webhook URL',
+    label: 'Grafana',
     value: 'grafana',
   },
   {
     label: 'Site24x7 Webhook URL',
     value: 'site24x7',
   },
   {
-    label: 'Prometheus Alertmanager Webhook URL',
+    label: 'Prometheus Alertmanager',
     value: 'prometheusAlertmanager',
   },
   {

diff --git a/web/src/cypress/e2e/wizard.cy.ts b/web/src/cypress/e2e/wizard.cy.ts
@@ -9,7 +9,7 @@ const keys = [
     value: 'generic',
   },
   {
-    label: 'Grafana Webhook URL',
+    label: 'Grafana',
     value: 'grafana',
   },
   {
@@ -296,6 +296,15 @@ function testWizard(): void {
       cy.get('body').should('contain', 'cannot be more than 35 characters')
 
       cy.get('body').contains('button', 'Close').click()
+
+      // TODO: fix this
+      // Notes: scrollIntoView is needed to resolve a bug where the search-select input field dropdown button is
+      // not visible on mobile. This seems to come from an update to themeConfig.tsx (commit a4dbf570f786842fa0eb8db249fa9a7b80a6bd57)
+      // which makes the multi text field default margin to 'dense'. Somewhere down the line MUI changes the overflow
+      // property of the button's parent to be hidden resulting in cypress counting it as not visible...
+      cy.get(
+        `input[name="secondarySchedule.followTheSunRotation.timeZone"]`,
+      ).scrollIntoView()
       cy.get(
         `input[name="secondarySchedule.followTheSunRotation.timeZone"]`,
       ).selectByLabel('Asia/Kolkata')
@@ -306,6 +315,9 @@ function testWizard(): void {
       cy.get('body').should('contain', 'cannot be more than 42 characters')
 
       cy.get('body').contains('button', 'Close').click()
+      cy.get(
+        `input[name="secondarySchedule.followTheSunRotation.timeZone"]`,
+      ).scrollIntoView()
       cy.get(
         `input[name="secondarySchedule.followTheSunRotation.timeZone"]`,
       ).selectByLabel('America/Chicago')