Merge pull request #4294 from adamdmharvey/warning-accordion

core: Update WarningPanel to use accordion UX

.changeset/eight-carrots-talk.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core': patch
+---
+
+Update `WarningPanel` component to use accordion-style expansion

.changeset/fair-kids-laugh.md

@@ -0,0 +1,5 @@
+---
+'@backstage/theme': patch
+---
+
+Updates warning text color to align to updated `WarningPanel` styling

packages/core/src/components/WarningPanel/WarningPanel.stories.tsx

@@ -16,7 +16,7 @@
 
 import React from 'react';
 import { WarningPanel } from './WarningPanel';
-import { Link, Button } from '@material-ui/core';
+import { Button, Link, Typography } from '@material-ui/core';
 
 export default {
   title: 'Feedback/Warning Panel',
@@ -25,11 +25,11 @@ export default {
 
 export const Default = () => (
   <WarningPanel
-    title="Example Warning Title"
+    title="Entity missing annotation"
     message={
       <>
-        This example entity is missing something. If this is unexpected, please
-        make sure you have set up everything correctly by following{' '}
+        This example entity is missing an annotation. If this is unexpected,
+        please make sure you have set up everything correctly by following{' '}
         <Link href="http://example.com">this guide</Link>.
       </>
     }
@@ -37,9 +37,36 @@ export const Default = () => (
 );
 
 export const Children = () => (
-  <WarningPanel title="Example Warning Title">
-    <Button variant="outlined" color="primary">
-      Supports custom children - for example this button
-    </Button>
+  <WarningPanel title="Could not contact backend system">
+    <Typography>
+      Supports custom children - for example these text elements. This can be
+      used to hide/expose stack traces for warnings, like this example:
+      <br />
+      SyntaxError: Error transforming
+      /home/user/github/backstage/packages/core/src/components/WarningPanel/WarningPanel.stories.tsx:
+      Unexpected token (42:16) at unexpected
+      (/home/user/github/backstage/node_modules/sucrase/dist/parser/traverser/util.js:83:15)
+      at tsParseMaybeAssignWithJSX
+      (/home/user/github/backstage/node_modules/sucrase/dist/parser/plugins/typescript.js:1399:22)
+      at tsParseMaybeAssign
+      (/home/user/github/backstage/node_modules/sucrase/dist/parser/plugins/typescript.js:1373:12)
+      at parseMaybeAssign
+      (/home/user/github/backstage/node_modules/sucrase/dist/parser/traverser/expression.js:118:43)
+      at parseExprListItem
+      (/home/user/github/backstage/node_modules/sucrase/dist/parser/traverser/expression.js:969:5)
+    </Typography>
+    <Button variant="contained">Learn More</Button>
   </WarningPanel>
 );
+
+export const FullExample = () => (
+  <WarningPanel
+    title="Could not contact backend system"
+    message="The backend system failed to respond. It is possible the service is down; please try again in a few minutes."
+  >
+    HTTP 500 Bad Gateway response from
+    https://usefulservice.mycompany.com/api/entity?44433
+  </WarningPanel>
+);
+
+export const TitleOnly = () => <WarningPanel title="Could not load data." />;

packages/core/src/components/WarningPanel/WarningPanel.test.tsx

@@ -15,23 +15,53 @@
  */
 
 import React from 'react';
+import { fireEvent, screen } 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();
+    await renderInTestApp(<WarningPanel {...propsTitleMessage} />);
+    expect(screen.getByText('Warning: Mock title')).toBeInTheDocument();
   });
 
-  it('renders message and children', async () => {
-    const { getByText } = await renderInTestApp(
-      <WarningPanel {...minProps}>children</WarningPanel>,
+  it('renders title', async () => {
+    await renderInTestApp(<WarningPanel {...propsTitleMessage} />);
+    const expandIcon = await screen.getByText('Warning: Mock title');
+    fireEvent.click(expandIcon);
+    expect(screen.getByText('Warning: Mock title')).toBeInTheDocument();
+    expect(screen.getByText('Some more info')).toBeInTheDocument();
+  });
+
+  it('renders title and children', async () => {
+    await renderInTestApp(
+      <WarningPanel {...propsTitle}>
+        <Typography>Java stacktrace</Typography>
+      </WarningPanel>,
     );
-    expect(getByText('Some more info')).toBeInTheDocument();
-    expect(getByText('children')).toBeInTheDocument();
+    expect(screen.getByText('Java stacktrace')).toBeInTheDocument();
+  });
+
+  it('renders message', async () => {
+    await renderInTestApp(<WarningPanel {...propsMessage} />);
+    expect(screen.getByText('Warning')).toBeInTheDocument();
+    expect(screen.getByText('Some more info')).toBeInTheDocument();
+  });
+
+  it('renders title, message, and children', async () => {
+    await renderInTestApp(
+      <WarningPanel {...propsTitleMessage}>
+        <Typography>Java stacktrace</Typography>
+      </WarningPanel>,
+    );
+    expect(screen.getByText('Warning: Mock title')).toBeInTheDocument();
+    expect(screen.getByText('Some more info')).toBeInTheDocument();
+    expect(screen.getByText('Java stacktrace')).toBeInTheDocument();
   });
 });

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,100 @@ 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: {
     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: string) => {
+  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>
   );
 };

packages/theme/src/themes.ts

@@ -58,7 +58,7 @@ export const lightTheme = createTheme({
     infoBackground: '#ebf5ff',
     errorText: '#CA001B',
     infoText: '#004e8a',
-    warningText: '#FEFEFE',
+    warningText: '#000000',
     linkHover: '#2196F3',
     link: '#0A6EBE',
     gold: yellow.A700,
@@ -120,7 +120,7 @@ export const darkTheme = createTheme({
     infoBackground: '#ebf5ff',
     errorText: '#CA001B',
     infoText: '#004e8a',
-    warningText: '#FEFEFE',
+    warningText: '#000000',
     linkHover: '#2196F3',
     link: '#0A6EBE',
     gold: yellow.A700,