Skip to content

Commit

Permalink
fix: prevent banner from crashing with invalid variant (#5686)
Browse files Browse the repository at this point in the history
https://linear.app/unleash/issue/2-1760/prevent-banner-from-crashing-when-set-with-an-invalid-variant

This prevents the banner from crashing when set with an invalid variant.
Instead, it default its styles to the default variant, which is `info`.

Also adds tests to validate our assumptions.
  • Loading branch information
nunogois committed Dec 19, 2023
1 parent 8388700 commit c0bc2d9
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 5 deletions.
64 changes: 64 additions & 0 deletions frontend/src/component/banners/Banner/Banner.test.tsx
@@ -0,0 +1,64 @@
import { screen } from '@testing-library/react';
import { render } from 'utils/testRenderer';
import { Banner } from './Banner';

test('should render correctly when using basic options', () => {
render(
<Banner
banner={{
message: 'This is a simple banner message.',
variant: 'warning',
}}
/>,
);

expect(
screen.getByText('This is a simple banner message.'),
).toBeInTheDocument();
expect(screen.getByTestId('WarningAmberIcon')).toBeInTheDocument();
});

test('should render correctly when using advanced options', () => {
render(
<Banner
banner={{
message: 'This is a more advanced banner message.',
variant: 'success',
sticky: false,
icon: 'star',
link: 'dialog',
linkText: 'Click me',
dialogTitle: 'Dialog title',
dialog: 'Dialog content',
}}
/>,
);

expect(
screen.getByText('This is a more advanced banner message.'),
).toBeInTheDocument();

const link = screen.getByText('Click me');
expect(link).toBeInTheDocument();
link.click();

expect(screen.getByText('Dialog title')).toBeInTheDocument();
expect(screen.getByText('Dialog content')).toBeInTheDocument();
});

test('should default to info variant when an invalid variant is provided', () => {
render(
<Banner
banner={{
message: 'This defaulted to an info banner message.',
// @ts-expect-error
variant: 'invalid',
}}
/>,
);

expect(
screen.getByText('This defaulted to an info banner message.'),
).toBeInTheDocument();
expect(screen.getByTestId('InfoOutlinedIcon')).toBeInTheDocument();
});
17 changes: 12 additions & 5 deletions frontend/src/component/banners/Banner/Banner.tsx
Expand Up @@ -13,6 +13,8 @@ import ReactMarkdown from 'react-markdown';
import { BannerVariant, IBanner } from 'interfaces/banner';
import { Sticky } from 'component/common/Sticky/Sticky';

const DEFAULT_VARIANT = 'info';

const StyledBar = styled('aside', {
shouldForwardProp: (prop) =>
prop !== 'variant' && prop !== 'inline' && prop !== 'maxHeight',
Expand All @@ -36,9 +38,14 @@ const StyledBar = styled('aside', {
maxHeight: maxHeight,
overflow: 'auto',
}),
borderColor: theme.palette[variant].border,
background: theme.palette[variant].light,
color: theme.palette[variant].dark,
borderColor:
theme.palette[variant]?.border ??
theme.palette[DEFAULT_VARIANT].border,
background:
theme.palette[variant]?.light ??
theme.palette[DEFAULT_VARIANT].light,
color:
theme.palette[variant]?.dark ?? theme.palette[DEFAULT_VARIANT].dark,
fontSize: theme.fontSizes.smallBody,
}),
);
Expand All @@ -48,7 +55,7 @@ const StyledIcon = styled('div', {
})<{ variant: BannerVariant }>(({ theme, variant }) => ({
display: 'flex',
alignItems: 'center',
color: theme.palette[variant].main,
color: theme.palette[variant]?.main ?? theme.palette[DEFAULT_VARIANT].main,
}));

interface IBannerProps {
Expand All @@ -62,7 +69,7 @@ export const Banner = ({ banner, inline, maxHeight }: IBannerProps) => {

const {
message,
variant = 'info',
variant = DEFAULT_VARIANT,
sticky,
icon,
link,
Expand Down

0 comments on commit c0bc2d9

Please sign in to comment.