From 5510fc4771b3e90eaf51086f4b1f85da9ddbac7c Mon Sep 17 00:00:00 2001 From: Justin Anastos Date: Tue, 2 Mar 2021 13:53:16 -0500 Subject: [PATCH 1/2] Add failing tests for removing FormControl children --- src/FormControl/FormControl.spec.tsx | 47 +++++++++++++++++----------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/src/FormControl/FormControl.spec.tsx b/src/FormControl/FormControl.spec.tsx index 4fb8ec3d..f3a6fa6f 100644 --- a/src/FormControl/FormControl.spec.tsx +++ b/src/FormControl/FormControl.spec.tsx @@ -8,6 +8,9 @@ import { FormLabel } from "../FormLabel"; import { FormHelperText } from "../FormHelperText"; import { Input } from "../Input"; import { FormErrorMessage } from "../FormErrorMessage"; +import { FormEndAdornment } from "../FormEndAdornment"; +import { FormStartAdornment } from "../FormStartAdornment"; +import { FormDescription } from "../FormDescription"; test("when passed a label, renders it", () => { render( @@ -88,24 +91,6 @@ test("when passed ``, renders error and svg", () => { expect(container.querySelector("svg")).toBeInTheDocument(); }); -test("when passed `` that is removed, removes the error message", () => { - const Component: React.FC<{ errorText?: string }> = ({ errorText }) => ( - - - {errorText && {errorText}} - - ); - - const { container, rerender } = render(); - - expect(screen.getByText("error text")).toBeInTheDocument(); - expect(container.querySelector("svg")).toBeInTheDocument(); - - rerender(); - - expect(screen.queryByText("error text")).not.toBeInTheDocument(); -}); - test("when passed `` witout `showIcon` prop, renders no svg", () => { const { container } = render( @@ -143,3 +128,29 @@ test("when not passed `autoFocus` prop, should not have focus after mounting", ( expect(formField).not.toHaveFocus(); }); + +test.each([ + ["FormDescription", FormDescription], + ["FormEndAdornment", FormEndAdornment], + ["FormErrorMessage", FormErrorMessage], + ["FormHelperText", FormHelperText], + ["FormLabel", FormLabel], + ["FormStartAdornment", FormStartAdornment], +])("%s can be removed on subsequent renders", (_, Component) => { + const { rerender } = render( + + text + + , + ); + + expect(screen.getByText("text")).toBeInTheDocument(); + + rerender( + + + , + ); + + expect(screen.queryByText("text")).not.toBeInTheDocument(); +}); From af1894eb294b297df70ed3ed5722c97a8249b976 Mon Sep 17 00:00:00 2001 From: Justin Anastos Date: Tue, 2 Mar 2021 14:04:06 -0500 Subject: [PATCH 2/2] Fix form controls not being removeable --- src/FormDescription/index.tsx | 2 ++ src/FormEndAdornment/index.tsx | 11 +++++------ src/FormHelperText/index.tsx | 2 ++ src/FormLabel/index.tsx | 2 ++ src/FormStartAdornment/index.tsx | 11 +++++------ 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/FormDescription/index.tsx b/src/FormDescription/index.tsx index 8fcb3d1c..99988e33 100644 --- a/src/FormDescription/index.tsx +++ b/src/FormDescription/index.tsx @@ -52,6 +52,8 @@ export const FormDescription: React.FC = ({ React.useLayoutEffect(() => { setDescription?.(element); + + return () => setDescription?.(null); }, [element, setDescription]); return setDescription ? null : element; diff --git a/src/FormEndAdornment/index.tsx b/src/FormEndAdornment/index.tsx index 5d4496cd..0db230ca 100644 --- a/src/FormEndAdornment/index.tsx +++ b/src/FormEndAdornment/index.tsx @@ -10,13 +10,10 @@ import { useFormControlInternalContext } from "../shared/FormControlContext"; * This is intended to be rendered below ``. If this is rendered on * it's own; it will render `children` without any modification. */ -export function FormEndAdornment({ +export const FormEndAdornment: React.FC<{ className?: string }> = ({ children, className, -}: { - children: React.ReactNode; - className?: string; -}): React.ReactNode { +}) => { const { setEndAdornment } = useFormControlInternalContext(); const element = React.useMemo( @@ -40,7 +37,9 @@ export function FormEndAdornment({ React.useLayoutEffect(() => { setEndAdornment?.(element); + + return () => setEndAdornment?.(null); }, [element, setEndAdornment]); return setEndAdornment ? null : element; -} +}; diff --git a/src/FormHelperText/index.tsx b/src/FormHelperText/index.tsx index d19f7de4..dc48f0f1 100644 --- a/src/FormHelperText/index.tsx +++ b/src/FormHelperText/index.tsx @@ -59,6 +59,8 @@ export const FormHelperText: React.FC = ({ React.useLayoutEffect(() => { // This will cause a bug if you change the `error` prop setHelper?.(element); + + return () => setHelper?.(null); }, [element, setHelper]); // If `setHelper` exists then we're rendering this under the form control diff --git a/src/FormLabel/index.tsx b/src/FormLabel/index.tsx index a76e467b..c202724f 100644 --- a/src/FormLabel/index.tsx +++ b/src/FormLabel/index.tsx @@ -73,6 +73,8 @@ export const FormLabel: React.FC = ({ React.useLayoutEffect(() => { setLabel?.(element); + + return () => setLabel?.(null); }, [element, setLabel]); return setLabel ? null : element; diff --git a/src/FormStartAdornment/index.tsx b/src/FormStartAdornment/index.tsx index 826f2294..63633d61 100644 --- a/src/FormStartAdornment/index.tsx +++ b/src/FormStartAdornment/index.tsx @@ -10,13 +10,10 @@ import { useFormControlInternalContext } from "../shared/FormControlContext"; * This is intended to be rendered below ``. If this is rendered on * it's own; it will render `children` without any modification. */ -export function FormStartAdornment({ +export const FormStartAdornment: React.FC<{ className?: string }> = ({ children, className, -}: { - children: React.ReactNode; - className?: string; -}): React.ReactNode { +}) => { const { setStartAdornment } = useFormControlInternalContext(); const element = React.useMemo( @@ -39,7 +36,9 @@ export function FormStartAdornment({ React.useLayoutEffect(() => { setStartAdornment?.(element); + + return () => setStartAdornment?.(null); }, [element, setStartAdornment]); return setStartAdornment ? null : element; -} +};