Skip to content

chore: Change usage of ChooseOne (no final condition) #4158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion site/src/components/Conditionals/ChooseOne.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,36 @@ export const FirstIsTrue: Story = () => (
<ChooseOne>
<Cond condition>The first one shows.</Cond>
<Cond condition={false}>The second one does not show.</Cond>
<Cond>The default does not show.</Cond>
</ChooseOne>
)

export const SecondIsTrue: Story = () => (
<ChooseOne>
<Cond condition={false}>The first one does not show.</Cond>
<Cond condition>The second one shows.</Cond>
<Cond>The default does not show.</Cond>
</ChooseOne>
)

export const AllAreTrue: Story = () => (
<ChooseOne>
<Cond condition>Only the first one shows.</Cond>
<Cond condition>The second one does not show.</Cond>
<Cond>The default does not show.</Cond>
</ChooseOne>
)

export const NoneAreTrue: Story = () => (
<ChooseOne>
<Cond condition={false}>The first one does not show.</Cond>
<Cond condition={false}>The second shows because it is the fallback.</Cond>
<Cond condition={false}>The second one does not show.</Cond>
<Cond>The default shows.</Cond>
</ChooseOne>
)

export const OneCond: Story = () => (
<ChooseOne>
<Cond>An only child renders.</Cond>
</ChooseOne>
)
33 changes: 23 additions & 10 deletions site/src/components/Conditionals/ChooseOne.tsx
Original file line number Diff line number Diff line change
@@ -1,29 +1,42 @@
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<CondProps>): JSX.Element => {
export const Cond = ({ children }: PropsWithChildren<CondProps>): JSX.Element => {
return <>{children}</>
}

/**
* 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[]
const chosen = childArray.find((child) => child.props.condition)
return chosen ?? childArray[childArray.length - 1]
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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk if this is worth consideration but if given an empty array defaultCase would be undefined I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! How does this look?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!!

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
}
4 changes: 2 additions & 2 deletions site/src/pages/TemplatesPage/TemplatesPageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export const TemplatesPageView: FC<React.PropsWithChildren<TemplatesPageViewProp
defaultMessage={t("errors.getTemplatesError")}
/>
</Cond>
<Cond condition>
<Cond>
<TableContainer>
<Table>
<TableHead>
Expand Down Expand Up @@ -173,7 +173,7 @@ export const TemplatesPageView: FC<React.PropsWithChildren<TemplatesPageViewProp
</TableCell>
</TableRow>
</Cond>
<Cond condition>
<Cond>
{props.templates?.map((template) => {
const templatePageLink = `/templates/${template.name}`
const hasIcon = template.icon && template.icon !== ""
Expand Down