From 78bb57db06373eeb040de6754ed047fc537c66ba Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Thu, 28 Jan 2021 14:40:44 -0500 Subject: [PATCH 1/8] 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('', () => { it('renders without exploding', async () => { - const { getByText } = await renderInTestApp(); - expect(getByText('Mock title')).toBeInTheDocument(); + const { getByText } = await renderInTestApp( + , + ); + expect(getByText('Warning: Mock title')).toBeInTheDocument(); }); - it('renders message and children', async () => { + it('renders title', async () => { const { getByText } = await renderInTestApp( - children, + , ); + 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( + + Java stacktrace + , + ); + expect(getByText('Java stacktrace')).toBeInTheDocument(); + }); + + it('renders message', async () => { + const { getByText } = await renderInTestApp( + , + ); + expect(getByText('Warning')).toBeInTheDocument(); + expect(getByText('Some more info')).toBeInTheDocument(); + }); + + it('renders title, message, and children', async () => { + const { getByText } = await renderInTestApp( + + Java stacktrace + , + ); + 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(theme => ({ @@ -29,57 +37,104 @@ const ErrorOutlineStyled = () => { const classes = useErrorOutlineStyles(); return ; }; +const ExpandMoreIconStyled = () => { + const classes = useErrorOutlineStyles(); + return ; +}; const useStyles = makeStyles(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 ( -
-
+ + } + className={classes.summary} + > - - {title} - -
- {message && ( - - {message} + + {subTitle} + + {(message || children) && ( + + + {message && ( + + + {message} + + + )} + {children && ( + + {children} + + )} + + )} - {children} -
+ ); }; From 75731af05d65152de6686376a42a8120202cf3cb Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Thu, 28 Jan 2021 14:41:14 -0500 Subject: [PATCH 2/8] Update WarningPanel examples for accordion --- .../WarningPanel/WarningPanel.stories.tsx | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/core/src/components/WarningPanel/WarningPanel.stories.tsx b/packages/core/src/components/WarningPanel/WarningPanel.stories.tsx index 5098f0aa31..ef99a0fce4 100644 --- a/packages/core/src/components/WarningPanel/WarningPanel.stories.tsx +++ b/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 = () => ( - 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{' '} this guide. } @@ -37,9 +37,36 @@ export const Default = () => ( ); export const Children = () => ( - - + + + Supports custom children - for example these text elements. This can be + used to hide/expose stack traces for warnings, like this example: +
+ 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) + + ); + +export const FullExample = () => ( + + HTTP 500 Bad Gateway response from + https://usefulservice.mycompany.com/api/entity?44433 + +); + +export const TitleOnly = () => ; From d822468671cc08259a6de908193da9b5fe8f1b9a Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Thu, 28 Jan 2021 14:42:13 -0500 Subject: [PATCH 3/8] Add changeset --- .changeset/eight-carrots-talk.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/eight-carrots-talk.md diff --git a/.changeset/eight-carrots-talk.md b/.changeset/eight-carrots-talk.md new file mode 100644 index 0000000000..01e38b845d --- /dev/null +++ b/.changeset/eight-carrots-talk.md @@ -0,0 +1,5 @@ +--- +'@backstage/core': patch +--- + +Update `WarningPanel` component to use accordion-style expansion From 919820ea8a0af024459e6b3fd2712ac33c48b6a5 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Thu, 28 Jan 2021 14:53:42 -0500 Subject: [PATCH 4/8] Update warning text color for accordion --- packages/theme/src/themes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/theme/src/themes.ts b/packages/theme/src/themes.ts index 5b233ac303..fd13343ca8 100644 --- a/packages/theme/src/themes.ts +++ b/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, From c810082ae6e650cbcfeaaf01272a0f4f4f2f31f3 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Thu, 28 Jan 2021 14:54:46 -0500 Subject: [PATCH 5/8] Add theme changeset --- .changeset/fair-kids-laugh.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fair-kids-laugh.md diff --git a/.changeset/fair-kids-laugh.md b/.changeset/fair-kids-laugh.md new file mode 100644 index 0000000000..d478508ed9 --- /dev/null +++ b/.changeset/fair-kids-laugh.md @@ -0,0 +1,5 @@ +--- +'@backstage/theme': patch +--- + +Updates warning text color to align to updated `WarningPanel` styling From ff58e9765c214487d3751c1a9138d398d9d6bf1a Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Thu, 28 Jan 2021 15:12:03 -0500 Subject: [PATCH 6/8] Fix TypeScript compile error --- packages/core/src/components/WarningPanel/WarningPanel.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/components/WarningPanel/WarningPanel.tsx b/packages/core/src/components/WarningPanel/WarningPanel.tsx index e55a3c35f4..3da110c411 100644 --- a/packages/core/src/components/WarningPanel/WarningPanel.tsx +++ b/packages/core/src/components/WarningPanel/WarningPanel.tsx @@ -83,8 +83,7 @@ type Props = { children?: React.ReactNode; }; -const capitalize = s => { - if (typeof s !== 'string') return ''; +const capitalize = (s: string) => { return s.charAt(0).toUpperCase() + s.slice(1); }; From c27107c15ed23f242b1b4c4c93e87caaaea645b2 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Fri, 29 Jan 2021 10:16:41 -0500 Subject: [PATCH 7/8] Refactor to use screen obj --- .../WarningPanel/WarningPanel.test.tsx | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/packages/core/src/components/WarningPanel/WarningPanel.test.tsx b/packages/core/src/components/WarningPanel/WarningPanel.test.tsx index fd7a5f4349..38ba4bff9b 100644 --- a/packages/core/src/components/WarningPanel/WarningPanel.test.tsx +++ b/packages/core/src/components/WarningPanel/WarningPanel.test.tsx @@ -15,7 +15,7 @@ */ import React from 'react'; -import { fireEvent } from '@testing-library/react'; +import { fireEvent, screen } from '@testing-library/react'; import { renderInTestApp } from '@backstage/test-utils'; import { Typography } from '@material-ui/core'; @@ -27,49 +27,41 @@ const propsMessage = { message: 'Some more info' }; describe('', () => { it('renders without exploding', async () => { - const { getByText } = await renderInTestApp( - , - ); - expect(getByText('Warning: Mock title')).toBeInTheDocument(); + await renderInTestApp(); + expect(screen.getByText('Warning: Mock title')).toBeInTheDocument(); }); it('renders title', async () => { - const { getByText } = await renderInTestApp( - , - ); - const expandIcon = await getByText('Warning: Mock title'); + await renderInTestApp(); + const expandIcon = await screen.getByText('Warning: Mock title'); fireEvent.click(expandIcon); - expect(getByText('Warning: Mock title')).toBeInTheDocument(); - expect(getByText('Some more info')).toBeInTheDocument(); + expect(screen.getByText('Warning: Mock title')).toBeInTheDocument(); + expect(screen.getByText('Some more info')).toBeInTheDocument(); }); it('renders title and children', async () => { - const { getByText } = await renderInTestApp( + await renderInTestApp( Java stacktrace , ); - expect(getByText('Java stacktrace')).toBeInTheDocument(); + expect(screen.getByText('Java stacktrace')).toBeInTheDocument(); }); it('renders message', async () => { - const { getByText } = await renderInTestApp( - , - ); - expect(getByText('Warning')).toBeInTheDocument(); - expect(getByText('Some more info')).toBeInTheDocument(); + await renderInTestApp(); + expect(screen.getByText('Warning')).toBeInTheDocument(); + expect(screen.getByText('Some more info')).toBeInTheDocument(); }); it('renders title, message, and children', async () => { - const { getByText } = await renderInTestApp( + await renderInTestApp( Java stacktrace , ); - 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(); + expect(screen.getByText('Warning: Mock title')).toBeInTheDocument(); + expect(screen.getByText('Some more info')).toBeInTheDocument(); + expect(screen.getByText('Java stacktrace')).toBeInTheDocument(); }); }); From 0dbc8aacad13af965b8f745d9577ebb0e06311f2 Mon Sep 17 00:00:00 2001 From: Adam Harvey Date: Fri, 29 Jan 2021 10:16:55 -0500 Subject: [PATCH 8/8] Remove unused styling --- packages/core/src/components/WarningPanel/WarningPanel.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/core/src/components/WarningPanel/WarningPanel.tsx b/packages/core/src/components/WarningPanel/WarningPanel.tsx index 3da110c411..e82c49c49a 100644 --- a/packages/core/src/components/WarningPanel/WarningPanel.tsx +++ b/packages/core/src/components/WarningPanel/WarningPanel.tsx @@ -44,9 +44,6 @@ const ExpandMoreIconStyled = () => { const useStyles = makeStyles(theme => ({ panel: { - // display: 'flex', - // flexDirection: 'column', - // padding: theme.spacing(1.5), backgroundColor: theme.palette.warningBackground, color: theme.palette.warningText, verticalAlign: 'middle',