From c13f247a943d4eeb1aeb20218ecc24ae687fe10d Mon Sep 17 00:00:00 2001 From: Jack Westbrook Date: Fri, 4 Jun 2021 14:38:29 +0200 Subject: [PATCH] Card: Defend against conditional action buttons (#35204) * fix(card): defend against invalid elements types passed to BaseActions * test(card): add test to support conditional buttons being passed in * fix(playlistpage): remove disabled prop from LinkButton for editors * chore(playlistpage): remove title from edit button --- .../src/components/Card/Card.test.tsx | 18 ++++++++++++++++++ .../grafana-ui/src/components/Card/Card.tsx | 8 ++++---- public/app/features/playlist/PlaylistPage.tsx | 9 +-------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/grafana-ui/src/components/Card/Card.test.tsx b/packages/grafana-ui/src/components/Card/Card.test.tsx index 1ceca474bce..a42736a04f6 100644 --- a/packages/grafana-ui/src/components/Card/Card.test.tsx +++ b/packages/grafana-ui/src/components/Card/Card.test.tsx @@ -72,5 +72,23 @@ describe('Card', () => { expect(screen.getByRole('button', { name: 'Click Me' })).not.toBeDisabled(); expect(screen.getByRole('button', { name: 'Delete' })).not.toBeDisabled(); }); + + it('Children should be conditional', () => { + const shouldNotRender = false; + render( + + + + {shouldNotRender && } + + + {shouldNotRender && } + + + ); + + expect(screen.getByRole('button', { name: 'Click Me' })).not.toBeDisabled(); + expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument(); + }); }); }); diff --git a/packages/grafana-ui/src/components/Card/Card.tsx b/packages/grafana-ui/src/components/Card/Card.tsx index a147c8559cf..48011b59112 100644 --- a/packages/grafana-ui/src/components/Card/Card.tsx +++ b/packages/grafana-ui/src/components/Card/Card.tsx @@ -243,7 +243,7 @@ const Meta: FC = memo(({ children, styles, Meta.displayName = 'Meta'; interface ActionsProps extends ChildProps { - children: JSX.Element | JSX.Element[]; + children?: React.ReactNode; variant?: 'primary' | 'secondary'; } @@ -251,9 +251,9 @@ const BaseActions: FC = ({ children, styles, disabled, variant }) const css = variant === 'primary' ? styles?.actions : styles?.secondaryActions; return (
- {Array.isArray(children) - ? React.Children.map(children, (child) => cloneElement(child, { disabled, ...child.props })) - : cloneElement(children, { disabled, ...children.props })} + {React.Children.map(children, (child) => { + return React.isValidElement(child) ? cloneElement(child, { disabled, ...child.props }) : null; + })}
); }; diff --git a/public/app/features/playlist/PlaylistPage.tsx b/public/app/features/playlist/PlaylistPage.tsx index 0280ebff4ea..ccaae7c9cd8 100644 --- a/public/app/features/playlist/PlaylistPage.tsx +++ b/public/app/features/playlist/PlaylistPage.tsx @@ -51,14 +51,7 @@ export const PlaylistPage: FC = ({ navModel }) => { Start playlist {contextSrv.isEditor && ( - + Edit playlist )}