From 78bb57db06373eeb040de6754ed047fc537c66ba Mon Sep 17 00:00:00 2001
From: Adam Harvey <adam.harvey@dxc.com>
Date: Thu, 28 Jan 2021 14:40:44 -0500
Subject: [PATCH] Update to accordion UX

---
 .../WarningPanel/WarningPanel.test.tsx        |  50 +++++++-
 .../components/WarningPanel/WarningPanel.tsx  | 109 +++++++++++++-----
 2 files changed, 126 insertions(+), 33 deletions(-)

diff --git a/packages/core/src/components/WarningPanel/WarningPanel.test.tsx b/packages/core/src/components/WarningPanel/WarningPanel.test.tsx
index 07a25d34c8..fd7a5f4349 100644
--- a/packages/core/src/components/WarningPanel/WarningPanel.test.tsx
+++ b/packages/core/src/components/WarningPanel/WarningPanel.test.tsx
@@ -15,23 +15,61 @@
  */
 
 import React from 'react';
+import { fireEvent } from '@testing-library/react';
 import { renderInTestApp } from '@backstage/test-utils';
+import { Typography } from '@material-ui/core';
 
 import { WarningPanel } from './WarningPanel';
 
-const minProps = { title: 'Mock title', message: 'Some more info' };
+const propsTitle = { title: 'Mock title' };
+const propsTitleMessage = { title: 'Mock title', message: 'Some more info' };
+const propsMessage = { message: 'Some more info' };
 
 describe('<WarningPanel />', () => {
   it('renders without exploding', async () => {
-    const { getByText } = await renderInTestApp(<WarningPanel {...minProps} />);
-    expect(getByText('Mock title')).toBeInTheDocument();
+    const { getByText } = await renderInTestApp(
+      <WarningPanel {...propsTitleMessage} />,
+    );
+    expect(getByText('Warning: Mock title')).toBeInTheDocument();
   });
 
-  it('renders message and children', async () => {
+  it('renders title', async () => {
     const { getByText } = await renderInTestApp(
-      <WarningPanel {...minProps}>children</WarningPanel>,
+      <WarningPanel {...propsTitleMessage} />,
     );
+    const expandIcon = await getByText('Warning: Mock title');
+    fireEvent.click(expandIcon);
+    expect(getByText('Warning: Mock title')).toBeInTheDocument();
     expect(getByText('Some more info')).toBeInTheDocument();
-    expect(getByText('children')).toBeInTheDocument();
+  });
+
+  it('renders title and children', async () => {
+    const { getByText } = await renderInTestApp(
+      <WarningPanel {...propsTitle}>
+        <Typography>Java stacktrace</Typography>
+      </WarningPanel>,
+    );
+    expect(getByText('Java stacktrace')).toBeInTheDocument();
+  });
+
+  it('renders message', async () => {
+    const { getByText } = await renderInTestApp(
+      <WarningPanel {...propsMessage} />,
+    );
+    expect(getByText('Warning')).toBeInTheDocument();
+    expect(getByText('Some more info')).toBeInTheDocument();
+  });
+
+  it('renders title, message, and children', async () => {
+    const { getByText } = await renderInTestApp(
+      <WarningPanel {...propsTitleMessage}>
+        <Typography>Java stacktrace</Typography>
+      </WarningPanel>,
+    );
+    expect(getByText('Warning: Mock title')).toBeInTheDocument();
+    expect(getByText('Some more info')).toBeInTheDocument();
+    expect(getByText('Java stacktrace')).toBeInTheDocument();
+    // expect(getByText(/Some more info/)).toBeTruthy();
+    // expect(getByText(/Java stacktrace/)).toBeTruthy();
   });
 });
diff --git a/packages/core/src/components/WarningPanel/WarningPanel.tsx b/packages/core/src/components/WarningPanel/WarningPanel.tsx
index ae4d2bff02..e55a3c35f4 100644
--- a/packages/core/src/components/WarningPanel/WarningPanel.tsx
+++ b/packages/core/src/components/WarningPanel/WarningPanel.tsx
@@ -15,8 +15,16 @@
  */
 
 import { BackstageTheme } from '@backstage/theme';
-import { makeStyles, Typography } from '@material-ui/core';
+import {
+  Accordion,
+  AccordionSummary,
+  AccordionDetails,
+  Grid,
+  makeStyles,
+  Typography,
+} from '@material-ui/core';
 import ErrorOutline from '@material-ui/icons/ErrorOutline';
+import ExpandMoreIcon from '@material-ui/icons/ExpandMore';
 import React from 'react';
 
 const useErrorOutlineStyles = makeStyles<BackstageTheme>(theme => ({
@@ -29,57 +37,104 @@ const ErrorOutlineStyled = () => {
   const classes = useErrorOutlineStyles();
   return <ErrorOutline classes={classes} />;
 };
+const ExpandMoreIconStyled = () => {
+  const classes = useErrorOutlineStyles();
+  return <ExpandMoreIcon classes={classes} />;
+};
 
 const useStyles = makeStyles<BackstageTheme>(theme => ({
-  message: {
-    display: 'flex',
-    flexDirection: 'column',
-    padding: theme.spacing(1.5),
+  panel: {
+    // display: 'flex',
+    // flexDirection: 'column',
+    // padding: theme.spacing(1.5),
     backgroundColor: theme.palette.warningBackground,
     color: theme.palette.warningText,
     verticalAlign: 'middle',
   },
-  header: {
+  summary: {
     display: 'flex',
     flexDirection: 'row',
-    marginBottom: theme.spacing(1),
   },
-  headerText: {
+  summaryText: {
     color: theme.palette.warningText,
+    fontWeight: 'bold',
   },
-  messageText: {
+  message: {
+    width: '100%',
+    display: 'block',
     color: theme.palette.warningText,
+    backgroundColor: theme.palette.warningBackground,
+  },
+  details: {
+    width: '100%',
+    display: 'block',
+    color: theme.palette.textContrast,
+    backgroundColor: theme.palette.background.default,
+    border: `1px solid ${theme.palette.border}`,
+    padding: theme.spacing(2.0),
+    fontFamily: 'sans-serif',
   },
 }));
 
-/**
- * WarningPanel. Show a user friendly error message to a user similar to ErrorPanel except that the warning panel
- * only shows the warning message to the user
- */
-
 type Props = {
-  message?: React.ReactNode;
   title?: string;
+  severity?: 'warning' | 'error' | 'info';
+  message?: React.ReactNode;
   children?: React.ReactNode;
 };
 
+const capitalize = s => {
+  if (typeof s !== 'string') return '';
+  return s.charAt(0).toUpperCase() + s.slice(1);
+};
+
+/**
+ * WarningPanel. Show a user friendly error message to a user similar to ErrorPanel except that the warning panel
+ * only shows the warning message to the user.
+ *
+ * @param {string} [severity=warning] Ability to change the severity of the alert. Not fully implemented. (error, warning, info)
+ * @param {string} [title] A title for the warning. If not supplied, "Warning" will be used.
+ * @param {Object} [message] Optional more detailed user-friendly message elaborating on the cause of the error.
+ * @param {Object} [children] Objects to provide context, such as a stack trace or detailed error reporting.
+ *  Will be available inside an unfolded accordion.
+ */
 export const WarningPanel = (props: Props) => {
   const classes = useStyles(props);
-  const { title, message, children } = props;
+  const { severity, title, message, children } = props;
+
+  // If no severity or title provided, the heading will read simply "Warning"
+  const subTitle =
+    (severity ? capitalize(severity) : 'Warning') + (title ? `: ${title}` : '');
+
   return (
-    <div className={classes.message}>
-      <div className={classes.header}>
+    <Accordion className={classes.panel}>
+      <AccordionSummary
+        expandIcon={<ExpandMoreIconStyled />}
+        className={classes.summary}
+      >
         <ErrorOutlineStyled />
-        <Typography className={classes.headerText} variant="subtitle1">
-          {title}
-        </Typography>
-      </div>
-      {message && (
-        <Typography className={classes.messageText} variant="body2">
-          {message}
+        <Typography className={classes.summaryText} variant="subtitle1">
+          {subTitle}
         </Typography>
+      </AccordionSummary>
+      {(message || children) && (
+        <AccordionDetails>
+          <Grid container>
+            {message && (
+              <Grid item xs={12}>
+                <Typography className={classes.message} variant="body1">
+                  {message}
+                </Typography>
+              </Grid>
+            )}
+            {children && (
+              <Grid item xs={12} className={classes.details}>
+                {children}
+              </Grid>
+            )}
+          </Grid>
+        </AccordionDetails>
       )}
-      {children}
-    </div>
+    </Accordion>
   );
 };