fix: checkboxgroup all when children are updated

Adslot · Oct 19, 2022 · 35ba8af · 35ba8af
1 parent 43c0875
commit 35ba8af
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 39 deletions.
diff --git a/src/components/CheckboxGroup/CheckboxGroupContext.jsx b/src/components/CheckboxGroup/CheckboxGroupContext.jsx
@@ -16,13 +16,6 @@ const CheckboxGroupProvider = ({
 }) => {
   const parentCtx = useCheckboxGroup();
 
-  const valuesRef = React.useRef([]);
-  React.useLayoutEffect(() => {
-    return () => {
-      valuesRef.current = [];
-    };
-  });
-
   const onChange = parentCtx.onChange || onChangeProp;
   const name = parentCtx.name || nameProp;
   const value = parentCtx.value || valueProp;
@@ -34,9 +27,9 @@ const CheckboxGroupProvider = ({
       return value.includes(checkboxValue);
     };
 
-    const getIsAllChecked = () => {
-      const hasUnchecked = valuesRef.current.some((item) => !getIsItemChecked(item));
-      const hasChecked = valuesRef.current.some((item) => getIsItemChecked(item));
+    const getIsAllChecked = (values) => {
+      const hasUnchecked = values.some((item) => !getIsItemChecked(item));
+      const hasChecked = values.some((item) => getIsItemChecked(item));
 
       return hasUnchecked && hasChecked ? 'partial' : !hasUnchecked && hasChecked;
     };
@@ -49,21 +42,16 @@ const CheckboxGroupProvider = ({
       onChange(newValues, name, checkboxValue);
     };
 
-    const onAllChange = () => {
-      const isAllChecked = getIsAllChecked();
+    const onAllChange = (values) => () => {
+      const isAllChecked = getIsAllChecked(values);
       if (isAllChecked === true) {
-        const newValues = value.filter((item) => !valuesRef.current.includes(item));
+        const newValues = value.filter((item) => !values.includes(item));
         onChange(newValues, name);
       } else {
-        onChange(_(value).concat(valuesRef.current).uniq().value(), name);
+        onChange(_(value).concat(values).uniq().value(), name);
       }
     };
 
-    const recordValue = (checkboxValue) => {
-      valuesRef.current.push(checkboxValue);
-      parentCtx.recordValue?.(checkboxValue);
-    };
-
     return {
       variant,
       value,
@@ -74,10 +62,8 @@ const CheckboxGroupProvider = ({
       getIsAllChecked,
       onItemChange,
       onAllChange,
-      valuesRef,
-      recordValue,
     };
-  }, [getIsChecked, value, name, onChange, variant, parentCtx]);
+  }, [getIsChecked, value, name, onChange, variant]);
 
   return <CheckboxGroupContext.Provider value={context}>{children}</CheckboxGroupContext.Provider>;
 };

diff --git a/src/components/CheckboxGroup/index.d.ts b/src/components/CheckboxGroup/index.d.ts
@@ -39,6 +39,10 @@ declare const CheckboxGroupItem: React.FC<CheckboxGroupItemProps>;
 export interface CheckboxGroupAllProps {
   label?: React.ReactNode;
   className?: string;
+  /**
+   * a array of values that the All option represent
+   */
+  values: any[];
 }
 
 declare const CheckboxGroupAll: React.FC<CheckboxGroupAllProps>;

diff --git a/src/components/CheckboxGroup/index.jsx b/src/components/CheckboxGroup/index.jsx
@@ -15,11 +15,7 @@ const CheckboxGroupItem = ({ value, ...rest }) => {
   invariant(!rest.variant, 'CheckboxGroup.Item: variant will be overridden by CheckboxGroup variant');
   invariant(!rest.onChange, 'CheckboxGroup.Item: onChange will be overridden by CheckboxGroup onChange');
 
-  const { onItemChange, getIsItemChecked, name, variant, recordValue } = groupCtx;
-
-  React.useLayoutEffect(() => {
-    recordValue(value);
-  });
+  const { onItemChange, getIsItemChecked, name, variant } = groupCtx;
 
   return (
     <Checkbox
@@ -35,7 +31,7 @@ const CheckboxGroupItem = ({ value, ...rest }) => {
 
 CheckboxGroupItem.propTypes = { ...shareCheckboxPropTypes };
 
-const CheckboxGroupAll = ({ className, label = 'All', ...rest }) => {
+const CheckboxGroupAll = ({ className, label = 'All', values, ...rest }) => {
   const groupCtx = useCheckboxGroup();
   invariant(!_.isEmpty(groupCtx), 'CheckboxGroup.All: must be used as children of CheckboxGroup');
 
@@ -47,8 +43,8 @@ const CheckboxGroupAll = ({ className, label = 'All', ...rest }) => {
       className={classnames(className, 'is-all')}
       name={name}
       label={label}
-      checked={getIsAllChecked()}
-      onChange={onAllChange}
+      checked={getIsAllChecked(values)}
+      onChange={onAllChange(values)}
       variant={variant}
     />
   );
@@ -57,6 +53,10 @@ const CheckboxGroupAll = ({ className, label = 'All', ...rest }) => {
 CheckboxGroupAll.propTypes = {
   label: PropTypes.node,
   className: PropTypes.string,
+  /**
+   * a array of values that the All option represent
+   */
+  values: PropTypes.array.isRequired,
 };
 
 const CheckboxGroup = ({

diff --git a/src/components/CheckboxGroup/index.spec.jsx b/src/components/CheckboxGroup/index.spec.jsx
@@ -194,7 +194,7 @@ describe('<CheckboxGroup />', () => {
     const onChange = jest.fn();
     const { container } = render(
       <CheckboxGroup name="movies" value={[]} onChange={onChange}>
-        <CheckboxGroup.All label="All" dts="target" />
+        <CheckboxGroup.All label="All" dts="target" values={['terminator', 'predator', 'soundofmusic']} />
         <CheckboxGroup.Item label="The Terminator" value="terminator" />
         <CheckboxGroup.Item label="Predator" value="predator" />
         <CheckboxGroup.Item label="The Sound of Music" value="soundofmusic" />
@@ -211,7 +211,7 @@ describe('<CheckboxGroup />', () => {
     const onChange = jest.fn();
     const { container } = render(
       <CheckboxGroup name="movies" value={['terminator', 'predator', 'soundofmusic']} onChange={onChange}>
-        <CheckboxGroup.All label="All" dts="target" />
+        <CheckboxGroup.All label="All" dts="target" values={['terminator', 'predator', 'soundofmusic']} />
         <CheckboxGroup.Item label="The Terminator" value="terminator" />
         <CheckboxGroup.Item label="Predator" value="predator" />
         <CheckboxGroup.Item label="The Sound of Music" value="soundofmusic" />
@@ -228,7 +228,7 @@ describe('<CheckboxGroup />', () => {
     const onChange = jest.fn();
     const { container } = render(
       <CheckboxGroup name="movies" value={['terminator']} onChange={onChange}>
-        <CheckboxGroup.All label="All" dts="target" />
+        <CheckboxGroup.All label="All" dts="target" values={['terminator', 'predator', 'soundofmusic']} />
         <CheckboxGroup.Item label="The Terminator" value="terminator" />
         <CheckboxGroup.Item label="Predator" value="predator" />
         <CheckboxGroup.Item label="The Sound of Music" value="soundofmusic" />
@@ -271,7 +271,7 @@ describe('<CheckboxGroup />', () => {
         <CheckboxGroup.Item label="The Sound of Music" value="soundofmusic" />
 
         <CheckboxGroup>
-          <CheckboxGroup.All label="All" dts="target" />
+          <CheckboxGroup.All label="All" dts="target" values={['Lung Cancer Late', 'Lung Cancer Early']} />
           <CheckboxGroup.Item label="Lung Cancer Late" value="Lung Cancer Late" />
           <CheckboxGroup.Item label="Lung Cancer Early" value="Lung Cancer Early" />
         </CheckboxGroup>
@@ -283,4 +283,57 @@ describe('<CheckboxGroup />', () => {
     expect(onChange).toHaveBeenCalledTimes(1);
     expect(onChange).toHaveBeenCalledWith(['Lung Cancer Late', 'Lung Cancer Early'], 'movies');
   });
+
+  it('should work when there is an initial value', () => {
+    const onChange = jest.fn();
+    const { container } = render(
+      <CheckboxGroup name="movies" value={['terminator']} onChange={onChange}>
+        <CheckboxGroup.All label="All" dts="target" values={['terminator', 'predator', 'soundofmusic']} />
+        <CheckboxGroup.Item label="The Terminator" value="terminator" />
+        <CheckboxGroup.Item label="Predator" value="predator" />
+        <CheckboxGroup.Item label="The Sound of Music" value="soundofmusic" />
+      </CheckboxGroup>
+    );
+
+    const checkbox = getByTestIdGlobal(getByDts(container, 'target'), 'checkbox-input');
+    expect(checkbox.checked).toBe(true);
+  });
+
+  it('should work when the values are updated', () => {
+    const Component = () => {
+      const [value, setValue] = React.useState([]);
+      const [allValues, setAllValues] = React.useState(['terminator', 'predator', 'soundofmusic']);
+
+      return (
+        <CheckboxGroup name="movies" value={value} onChange={setValue}>
+          <CheckboxGroup.All label="All" values={allValues} />
+          {allValues.map((item) => (
+            <CheckboxGroup.Item key={item} label={item} value={item} />
+          ))}
+          <button
+            data-testid="button"
+            onClick={() => {
+              setAllValues((prev) => [...prev, 'batman']);
+            }}
+          >
+            Change All
+          </button>
+        </CheckboxGroup>
+      );
+    };
+
+    const { queryAllByTestId, getByTestId } = render(<Component />);
+
+    const items = queryAllByTestId('checkbox');
+    expect(items[0]).toHaveAttribute('aria-checked', 'false');
+
+    fireEvent.click(getByTestIdGlobal(items[1], 'checkbox-input'));
+    fireEvent.click(getByTestIdGlobal(items[2], 'checkbox-input'));
+    fireEvent.click(getByTestIdGlobal(items[3], 'checkbox-input'));
+    expect(items[0]).toHaveAttribute('aria-checked', 'true');
+
+    fireEvent.click(getByTestId('button'));
+
+    expect(items[0]).toHaveAttribute('aria-checked', 'mixed');
+  });
 });
diff --git a/www/containers/props.json b/www/containers/props.json
@@ -1396,6 +1396,13 @@
           },
           "required": false,
           "description": ""
+        },
+        "values": {
+          "type": {
+            "name": "array"
+          },
+          "required": true,
+          "description": "a array of values that the All option represent"
         }
       }
     },
@@ -1425,7 +1432,7 @@
           ],
           "params": [
             {
-              "name": "{ className, label = 'All', ...rest }",
+              "name": "{ className, label = 'All', values, ...rest }",
               "type": null
             }
           ],

diff --git a/www/examples/CheckboxGroup.mdx b/www/examples/CheckboxGroup.mdx
@@ -135,15 +135,15 @@ const Example = () => {
         className="checkbox-advanced"
       >
         <CheckboxGroup indent>
-          <CheckboxGroup.All label="Sports" />
+          <CheckboxGroup.All label="Sports" values={allHobbies.sports} />
           {allHobbies.sports.map((item) => (
-            <CheckboxGroup.Item value={item} label={_.capitalize(item)} />
+            <CheckboxGroup.Item key={item} value={item} label={_.capitalize(item)} />
           ))}
         </CheckboxGroup>
         <CheckboxGroup indent>
-          <CheckboxGroup.All label="Other" />
+          <CheckboxGroup.All label="Other" values={allHobbies.other} />
           {allHobbies.other.map((item) => (
-            <CheckboxGroup.Item value={item} label={_.capitalize(item)} />
+            <CheckboxGroup.Item key={item} value={item} label={_.capitalize(item)} />
           ))}
         </CheckboxGroup>
       </CheckboxGroup>