Skip to content
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

fix: fix checkbox group all selection #1761

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
4 changes: 2 additions & 2 deletions src/components/CheckboxGroup/CheckboxGroup.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ export const NestedCheckboxes: Story = {
<CheckboxGroup indent>
<CheckboxGroup.All label="Sports" values={allHobbies.sports} />
{allHobbies.sports.map((item) => (
<CheckboxGroup.Item key={item} value={item} label={_.capitalize(item)} />
<CheckboxGroup.Item key={item} value={item} label={_.capitalize(item)} disabled={item === 'swimming' }/>
))}
</CheckboxGroup>
<CheckboxGroup indent>
<CheckboxGroup.All label="Other" values={allHobbies.other} />
{allHobbies.other.map((item) => (
<CheckboxGroup.Item key={item} value={item} label={_.capitalize(item)} />
<CheckboxGroup.Item key={item} value={item} label={_.capitalize(item)} disabled/>
))}
</CheckboxGroup>
</CheckboxGroup>
Expand Down
22 changes: 20 additions & 2 deletions src/components/CheckboxGroup/CheckboxGroupContext.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const CheckboxGroupProvider = ({
const name = parentCtx.name || nameProp;
const value = parentCtx.value || valueProp;

const [disabledValues, setDisabledValues] = React.useState([]);

const context = React.useMemo(() => {
const getIsItemChecked = (checkboxValue) => {
if (getIsChecked) return getIsChecked(checkboxValue, value);
Expand All @@ -28,11 +30,11 @@ const CheckboxGroupProvider = ({
};

const getIsAllChecked = (values) => {
if (_.isEmpty(values)) return false;
const result = _(values)
.map((item) => getIsItemChecked(item))
.uniq()
.value();

return result.length === 1 ? result[0] : 'partial';
};

Expand All @@ -54,6 +56,18 @@ const CheckboxGroupProvider = ({
}
};

const registerDisabledValue = (disabledValue) => {
if (!_.includes(disabledValues, disabledValue)) {
setDisabledValues((prevValues) => [...prevValues, disabledValue]);
}
};

const unregisterDisabledValue = (disabledValue) => {
if (_.includes(disabledValues, disabledValue)) {
setDisabledValues((prevValues) => _.filter(prevValues, (v) => v !== disabledValue));
}
};

return {
variant,
value,
Expand All @@ -64,8 +78,12 @@ const CheckboxGroupProvider = ({
getIsAllChecked,
onItemChange,
onAllChange,

registerDisabledValue,
unregisterDisabledValue,
disabledValues,
};
}, [getIsChecked, value, name, onChange, variant]);
}, [getIsChecked, value, name, onChange, variant, disabledValues]);

return <CheckboxGroupContext.Provider value={context}>{children}</CheckboxGroupContext.Provider>;
};
Expand Down
22 changes: 17 additions & 5 deletions src/components/CheckboxGroup/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,23 @@ import CheckboxGroupProvider, { useCheckboxGroup } from './CheckboxGroupContext'
import '../RadioGroup/style.css';
import './styles.css';

const CheckboxGroupItem = ({ value, ...rest }) => {
const CheckboxGroupItem = ({ value, disabled, ...rest }) => {
const groupCtx = useCheckboxGroup();
invariant(!_.isEmpty(groupCtx), 'CheckboxGroup.Item: must be used as children of CheckboxGroup');
invariant(!rest.name, 'CheckboxGroup.Item: name will be overridden by CheckboxGroup name');
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 } = groupCtx;
const { onItemChange, getIsItemChecked, name, variant, registerDisabledValue, unregisterDisabledValue } = groupCtx;

React.useEffect(() => {
if (disabled) {
registerDisabledValue(value);
}
return () => {
unregisterDisabledValue(value);
};
}, [disabled, registerDisabledValue, unregisterDisabledValue, value]);

return (
<Checkbox
Expand All @@ -26,6 +35,7 @@ const CheckboxGroupItem = ({ value, ...rest }) => {
variant={variant}
checked={getIsItemChecked(value)}
onChange={() => onItemChange(value)}
disabled={disabled}
/>
);
};
Expand All @@ -36,17 +46,19 @@ const CheckboxGroupAll = ({ className, label = 'All', values, ...rest }) => {
const groupCtx = useCheckboxGroup();
invariant(!_.isEmpty(groupCtx), 'CheckboxGroup.All: must be used as children of CheckboxGroup');

const { onAllChange, getIsAllChecked, name, variant } = groupCtx;
const { onAllChange, getIsAllChecked, name, variant, disabledValues } = groupCtx;
const enabledValues = _.filter(values, (value) => !_.includes(disabledValues, value));

return (
<Checkbox
{...rest}
className={classnames(className, 'is-all')}
name={name}
label={label}
checked={getIsAllChecked(values)}
onChange={onAllChange(values)}
checked={getIsAllChecked(enabledValues)}
onChange={onAllChange(enabledValues)}
variant={variant}
disabled={_.isEqual(values, disabledValues)}
/>
);
};
Expand Down
196 changes: 196 additions & 0 deletions src/components/CheckboxGroup/index.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -339,3 +339,199 @@ it('should work when the values are updated', async () => {
// eslint-disable-next-line jest-dom/prefer-checked
expect(items[0]).toHaveAttribute('aria-checked', 'mixed');
});

it('should disable checkbox group all if all the items are disabled', () => {
const onChange = jest.fn();
render(
<CheckboxGroup name="movies" value={[]} onChange={onChange}>
<CheckboxGroup.All label="All" dts="target" values={['terminator', 'predator', 'soundofmusic']} />
<CheckboxGroup.Item label="The Terminator" value="terminator" disabled />
<CheckboxGroup.Item label="Predator" value="predator" disabled />
<CheckboxGroup.Item label="The Sound of Music" value="soundofmusic" disabled />
</CheckboxGroup>
);
const checkbox = within(screen.getByDts('target')).getByTestId('checkbox-input');
expect(checkbox).toBeDisabled();
});

it('should not select disabled checkbox item', async () => {
const onChange = jest.fn();
render(
<CheckboxGroup name="movies" value={[]} onChange={onChange}>
<CheckboxGroup.All label="All" dts="target" values={['terminator', 'predator', 'soundofmusic']} />
<CheckboxGroup.Item label="The Terminator" value="terminator" disabled />
<CheckboxGroup.Item label="Predator" value="predator" />
<CheckboxGroup.Item label="The Sound of Music" value="soundofmusic" />
</CheckboxGroup>
);

const checkbox = within(screen.getByDts('target')).getByTestId('checkbox-input');
await user.click(checkbox);
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledWith(['predator', 'soundofmusic'], 'movies');
});

it('should work when re-render after disabled item is changed to non disabled', async () => {
const Component = () => {
const [value, setValue] = React.useState([]);
const allValues = ['terminator', 'predator', 'soundofmusic'];
const [checkboxDisabled, setCheckboxDisabled] = React.useState(true);

const handleButtonClick = () => {
setCheckboxDisabled(false);
};

return (
<div>
<button data-testid="button" onClick={handleButtonClick}>
enable terminator
</button>
<CheckboxGroup name="movies" value={value} onChange={setValue}>
<CheckboxGroup.All label="All" values={allValues} />
{allValues.map((item) => (
<CheckboxGroup.Item
key={item}
label={item}
value={item}
disabled={item === 'terminator' ? checkboxDisabled : false}
/>
))}
</CheckboxGroup>
</div>
);
};

render(<Component />);

await user.click(screen.getByTestId('button'));
const items = screen.queryAllByTestId('checkbox');
await user.click(within(items[0]).getByTestId('checkbox-input')); // click checkbox all
expect(items[1]).toBeChecked(); // terminator is no longer disabled, so it should be checked
expect(items[2]).toBeChecked(); // predator is checked
expect(items[3]).toBeChecked(); // soundofmusic is checked
});

it('should work when non disabled values are updated', async () => {
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} disabled={item === 'terminator' ? true : false} />
))}
<button
data-testid="button"
onClick={() => {
setAllValues((prev) => [...prev, 'batman']);
}}
>
Change All
</button>
</CheckboxGroup>
);
};

render(<Component />);

await user.click(screen.getByTestId('button'));
const items = screen.queryAllByTestId('checkbox');
await user.click(within(items[0]).getByTestId('checkbox-input')); // click checkbox all
expect(items[1]).not.toBeChecked(); // terminator is disabled hence it should not be checked
expect(items[2]).toBeChecked(); // predator is checked
expect(items[3]).toBeChecked(); // soundofmusic is checked
expect(items[4]).toBeChecked(); // batman is checked
});

it('should work when disabled values are updated', async () => {
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} disabled={item === 'batman' ? true : false} />
))}
<button
data-testid="button"
onClick={() => {
setAllValues((prev) => [...prev, 'batman']);
}}
>
Change All
</button>
</CheckboxGroup>
);
};

render(<Component />);

await user.click(screen.getByTestId('button'));
const items = screen.queryAllByTestId('checkbox');
await user.click(within(items[0]).getByTestId('checkbox-input')); // click checkbox all
expect(items[1]).toBeChecked(); // terminator is checked
glenkurniawan-adslot marked this conversation as resolved.
Show resolved Hide resolved
expect(items[2]).toBeChecked(); // predator is checked
expect(items[3]).toBeChecked(); // soundofmusic is checked
expect(items[4]).not.toBeChecked(); // batman is disabled hence it should not be checked
});

it('should work when children is updated', async () => {
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} disabled={item !== 'batman' ? true : false} />
))}
<button
data-testid="button"
onClick={() => {
setAllValues((prev) => [...prev, 'batman']);
}}
>
Change All
</button>
</CheckboxGroup>
);
};

render(<Component />);
const items = screen.queryAllByTestId('checkbox');
const checkboxAll = within(items[0]).getByTestId('checkbox-input');
const checkboxItem1 = within(items[1]).getByTestId('checkbox-input');
const checkboxItem2 = within(items[2]).getByTestId('checkbox-input');
const checkboxItem3 = within(items[3]).getByTestId('checkbox-input');

expect(checkboxAll).toBeDisabled(); // checkbox all is disabled
expect(checkboxItem1).toBeDisabled(); // terminator is disabled
expect(checkboxItem2).toBeDisabled(); // predator is disabled
expect(checkboxItem3).toBeDisabled(); // soundofmusic is disabled

await user.click(screen.getByTestId('button'));
const updatedItems = screen.queryAllByTestId('checkbox');
const updatedCheckboxAll = within(updatedItems[0]).getByTestId('checkbox-input');
const updatedCheckboxItem1 = within(updatedItems[1]).getByTestId('checkbox-input');
const updatedCheckboxItem2 = within(updatedItems[2]).getByTestId('checkbox-input');
const updatedCheckboxItem3 = within(updatedItems[3]).getByTestId('checkbox-input');
const updatedCheckboxItem4 = within(updatedItems[4]).getByTestId('checkbox-input');

expect(updatedCheckboxAll).toBeEnabled(); // checkbox all is now enabled, since there is one enabled child
expect(updatedCheckboxItem1).toBeDisabled(); // terminator is disabled
expect(updatedCheckboxItem2).toBeDisabled(); // predator is disabled
expect(updatedCheckboxItem3).toBeDisabled(); // soundofmusic is disabled
expect(updatedCheckboxItem4).toBeEnabled(); // soundofmusic is disabled

await user.click(checkboxAll); // click checkbox all
expect(updatedItems[0]).toBeChecked(); // checkbox all is checked, since there is one enabled child
expect(updatedItems[1]).not.toBeChecked(); // terminator is disabled
expect(updatedItems[2]).not.toBeChecked(); // predator is disabled
expect(updatedItems[3]).not.toBeChecked(); // soundofmusic is disabled
expect(updatedItems[4]).toBeChecked(); // batman is checked
});