From d6d06a1f84eecc7fd6434f1fc9cf4c882b27044d Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Thu, 22 Sep 2022 19:56:13 +0000 Subject: [PATCH 1/3] Change contract of Cond: no condition on default case --- .../Conditionals/ChooseOne.stories.tsx | 6 ++++- .../src/components/Conditionals/ChooseOne.tsx | 26 +++++++++++++------ .../pages/TemplatesPage/TemplatesPageView.tsx | 4 +-- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/site/src/components/Conditionals/ChooseOne.stories.tsx b/site/src/components/Conditionals/ChooseOne.stories.tsx index f2fc75fd43379..28b441e79bf28 100644 --- a/site/src/components/Conditionals/ChooseOne.stories.tsx +++ b/site/src/components/Conditionals/ChooseOne.stories.tsx @@ -11,6 +11,7 @@ export const FirstIsTrue: Story = () => ( The first one shows. The second one does not show. + The default does not show. ) @@ -18,6 +19,7 @@ export const SecondIsTrue: Story = () => ( The first one does not show. The second one shows. + The default does not show. ) @@ -25,12 +27,14 @@ export const AllAreTrue: Story = () => ( Only the first one shows. The second one does not show. + The default does not show. ) export const NoneAreTrue: Story = () => ( The first one does not show. - The second shows because it is the fallback. + The second one does not show. + The default shows. ) diff --git a/site/src/components/Conditionals/ChooseOne.tsx b/site/src/components/Conditionals/ChooseOne.tsx index 71b28d95db84a..a22cda19e133c 100644 --- a/site/src/components/Conditionals/ChooseOne.tsx +++ b/site/src/components/Conditionals/ChooseOne.tsx @@ -1,18 +1,17 @@ import { Children, PropsWithChildren } from "react" export interface CondProps { - condition: boolean + condition?: boolean } /** * Wrapper component that attaches a condition to a child component so that ChooseOne can - * determine which child to render. The last Cond in a ChooseOne is the fallback case; set - * its `condition` to `true` to avoid confusion. - * @param condition boolean expression indicating whether the child should be rendered + * determine which child to render. The last Cond in a ChooseOne is the fallback case and + * should not have a condition. + * @param condition boolean expression indicating whether the child should be rendered, or undefined * @returns child. Note that Cond alone does not enforce the condition; it should be used inside ChooseOne. */ -// eslint-disable-next-line @typescript-eslint/no-unused-vars -export const Cond = ({ children, condition }: PropsWithChildren): JSX.Element => { +export const Cond = ({ children }: PropsWithChildren): JSX.Element => { return <>{children} } @@ -21,9 +20,20 @@ export const Cond = ({ children, condition }: PropsWithChildren): JSX * with a condition under which it should be rendered. If no conditions are met, the final child * will be rendered. * @returns one of its children + * @throws an error if its last child has a condition prop, or any non-final children do not have a condition prop */ export const ChooseOne = ({ children }: PropsWithChildren): JSX.Element => { const childArray = Children.toArray(children) as JSX.Element[] - const chosen = childArray.find((child) => child.props.condition) - return chosen ?? childArray[childArray.length - 1] + const conditionedOptions = childArray.slice(0, childArray.length - 1) + const defaultCase = childArray[childArray.length - 1] + if (defaultCase.props.condition !== undefined) { + throw new Error( + "The last Cond in a ChooseOne was given a condition prop, but it is the default case.", + ) + } + if (conditionedOptions.some((cond) => cond.props.condition === undefined)) { + throw new Error("A non-final Cond in a ChooseOne does not have a condition prop.") + } + const chosen = conditionedOptions.find((child) => child.props.condition) + return chosen ?? defaultCase } diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index 64a3518a80871..af2d3e9de6fcd 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -139,7 +139,7 @@ export const TemplatesPageView: FC - + @@ -173,7 +173,7 @@ export const TemplatesPageView: FC - + {props.templates?.map((template) => { const templatePageLink = `/templates/${template.name}` const hasIcon = template.icon && template.icon !== "" From 0a435a6b82cb49dc0fe71b86e83df6f13e68f40d Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Tue, 27 Sep 2022 17:37:24 +0000 Subject: [PATCH 2/3] Handle no children case --- site/src/components/Conditionals/ChooseOne.stories.tsx | 10 +++++++++- site/src/components/Conditionals/ChooseOne.tsx | 7 +++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/site/src/components/Conditionals/ChooseOne.stories.tsx b/site/src/components/Conditionals/ChooseOne.stories.tsx index 28b441e79bf28..e7b2c265ade4c 100644 --- a/site/src/components/Conditionals/ChooseOne.stories.tsx +++ b/site/src/components/Conditionals/ChooseOne.stories.tsx @@ -12,7 +12,7 @@ export const FirstIsTrue: Story = () => ( The first one shows. The second one does not show. The default does not show. - + ) export const SecondIsTrue: Story = () => ( @@ -38,3 +38,11 @@ export const NoneAreTrue: Story = () => ( The default shows. ) + +export const OneCond: Story = () => ( + + + An only child renders. + + +) diff --git a/site/src/components/Conditionals/ChooseOne.tsx b/site/src/components/Conditionals/ChooseOne.tsx index a22cda19e133c..6bf9d0531fd1c 100644 --- a/site/src/components/Conditionals/ChooseOne.tsx +++ b/site/src/components/Conditionals/ChooseOne.tsx @@ -19,11 +19,14 @@ export const Cond = ({ children }: PropsWithChildren): JSX.Element => * Wrapper component for rendering exactly one of its children. Wrap each child in Cond to associate it * with a condition under which it should be rendered. If no conditions are met, the final child * will be rendered. - * @returns one of its children + * @returns one of its children, or null if there are no children * @throws an error if its last child has a condition prop, or any non-final children do not have a condition prop */ -export const ChooseOne = ({ children }: PropsWithChildren): JSX.Element => { +export const ChooseOne = ({ children }: PropsWithChildren): JSX.Element|null => { const childArray = Children.toArray(children) as JSX.Element[] + if (childArray.length === 0) { + return null + } const conditionedOptions = childArray.slice(0, childArray.length - 1) const defaultCase = childArray[childArray.length - 1] if (defaultCase.props.condition !== undefined) { From e84edd1e17cf0ac0f7807fdf58c7ec4c3740d269 Mon Sep 17 00:00:00 2001 From: Presley Pizzo Date: Tue, 27 Sep 2022 17:39:45 +0000 Subject: [PATCH 3/3] Format --- site/src/components/Conditionals/ChooseOne.stories.tsx | 6 ++---- site/src/components/Conditionals/ChooseOne.tsx | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/site/src/components/Conditionals/ChooseOne.stories.tsx b/site/src/components/Conditionals/ChooseOne.stories.tsx index e7b2c265ade4c..a6c1bf8483c3f 100644 --- a/site/src/components/Conditionals/ChooseOne.stories.tsx +++ b/site/src/components/Conditionals/ChooseOne.stories.tsx @@ -12,7 +12,7 @@ export const FirstIsTrue: Story = () => ( The first one shows. The second one does not show. The default does not show. - + ) export const SecondIsTrue: Story = () => ( @@ -41,8 +41,6 @@ export const NoneAreTrue: Story = () => ( export const OneCond: Story = () => ( - - An only child renders. - + An only child renders. ) diff --git a/site/src/components/Conditionals/ChooseOne.tsx b/site/src/components/Conditionals/ChooseOne.tsx index 6bf9d0531fd1c..4eca8130bb33d 100644 --- a/site/src/components/Conditionals/ChooseOne.tsx +++ b/site/src/components/Conditionals/ChooseOne.tsx @@ -22,7 +22,7 @@ export const Cond = ({ children }: PropsWithChildren): JSX.Element => * @returns one of its children, or null if there are no children * @throws an error if its last child has a condition prop, or any non-final children do not have a condition prop */ -export const ChooseOne = ({ children }: PropsWithChildren): JSX.Element|null => { +export const ChooseOne = ({ children }: PropsWithChildren): JSX.Element | null => { const childArray = Children.toArray(children) as JSX.Element[] if (childArray.length === 0) { return null