feat(checkbox): add indeterminate state for partial selection#1112
feat(checkbox): add indeterminate state for partial selection#1112
Conversation
- Add `indeterminate` property to bds-checkbox component - Show minus icon (less) when in indeterminate state - Clicking when indeterminate transitions to checked state - Add styles for indeterminate and indeterminate-disabled states - Update bdsChange event to include indeterminate property - Add unit tests for indeterminate state behavior - Add e2e tests for indeterminate state - Update Storybook stories with AllStates and IndeterminateExample - Update MDX documentation with indeterminate state info Co-authored-by: TarsysFonseca <3694626+TarsysFonseca@users.noreply.github.com>
|
@copilot revise os seguintes pontos:
|
…mpatibility tests - Replace inline styles with bds-grid and bds-typo components - Add backwards compatibility tests to ensure checkbox works without indeterminate prop - Verify default checked/unchecked behavior is preserved Co-authored-by: TarsysFonseca <3694626+TarsysFonseca@users.noreply.github.com>
Addressed all the points in commit
|
There was a problem hiding this comment.
Pull request overview
This PR adds an indeterminate state to the bds-checkbox component to support partial selection scenarios, where a parent checkbox controls multiple child checkboxes. The implementation introduces a new indeterminate boolean property that displays a minus icon and transitions to checked state when clicked.
Key Changes:
- Added
indeterminateproperty to the checkbox component with proper handling in all interaction methods - Implemented new SCSS styles for indeterminate and indeterminate-disabled states
- Added comprehensive unit tests (36 total including 3 backwards compatibility tests) and e2e tests (10 total)
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/checkbox/checkbox.tsx | Added indeterminate property and updated interaction logic (onClick, toggle, handleKeyDown) to transition from indeterminate to checked state |
| src/components/checkbox/checkbox.scss | Added CSS styles for checkbox--indeterminate and checkbox--indeterminate-disabled states |
| src/components/checkbox/test/checkbox.spec.ts | Added 36 unit tests including indeterminate state tests and backwards compatibility tests |
| src/components/checkbox/test/checkbox.e2e.ts | Added 10 e2e tests covering indeterminate state interactions |
| src/components/checkbox/checkbox.stories.jsx | Added AllStates and IndeterminateExample stories with interactive parent-child demo |
| src/components/checkbox/checkbox.mdx | Updated documentation with indeterminate state examples and event details |
| src/components/checkbox/readme.md | Auto-generated property documentation for indeterminate property |
| src/components.d.ts | Auto-generated TypeScript definitions for the new property |
| @@ -101,13 +131,24 @@ export class Checkbox { | |||
|
|
|||
| private handleKeyDown(event) { | |||
| if (event.key == 'Enter') { | |||
There was a problem hiding this comment.
Use strict equality operator (===) instead of loose equality (==) for type-safe comparison. This is a JavaScript best practice and aligns with TypeScript conventions.
Change:
if (event.key == 'Enter') {To:
if (event.key === 'Enter') {| if (event.key == 'Enter') { | |
| if (event.key === 'Enter') { |
| useEffect(() => { | ||
| const parentCheckbox = document.getElementById('parent-checkbox'); | ||
| const childCheckboxes = [ | ||
| document.getElementById('child1'), | ||
| document.getElementById('child2'), | ||
| document.getElementById('child3') | ||
| ]; | ||
|
|
||
| const updateParentState = () => { | ||
| const checkedCount = childCheckboxes.filter(cb => cb.checked).length; | ||
|
|
||
| if (checkedCount === 0) { | ||
| parentCheckbox.checked = false; | ||
| parentCheckbox.indeterminate = false; | ||
| } else if (checkedCount === childCheckboxes.length) { | ||
| parentCheckbox.checked = true; | ||
| parentCheckbox.indeterminate = false; | ||
| } else { | ||
| parentCheckbox.indeterminate = true; | ||
| } | ||
| }; | ||
|
|
||
| parentCheckbox.addEventListener('bdsChange', (e) => { | ||
| // When parent is clicked (from indeterminate), it becomes checked | ||
| // and all children should be checked | ||
| const isChecked = e.detail.checked; | ||
| childCheckboxes.forEach(cb => { | ||
| cb.checked = isChecked; | ||
| }); | ||
| console.log('Parent changed:', e.detail); | ||
| }); | ||
|
|
||
| childCheckboxes.forEach(cb => { | ||
| cb.addEventListener('bdsChange', () => { | ||
| updateParentState(); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The useEffect hook is missing a cleanup function and dependency array. This can lead to memory leaks as event listeners are not removed when the component unmounts or re-renders.
Consider adding a cleanup function and dependency array:
useEffect(() => {
const parentCheckbox = document.getElementById('parent-checkbox');
const childCheckboxes = [
document.getElementById('child1'),
document.getElementById('child2'),
document.getElementById('child3')
];
const updateParentState = () => {
const checkedCount = childCheckboxes.filter(cb => cb.checked).length;
if (checkedCount === 0) {
parentCheckbox.checked = false;
parentCheckbox.indeterminate = false;
} else if (checkedCount === childCheckboxes.length) {
parentCheckbox.checked = true;
parentCheckbox.indeterminate = false;
} else {
parentCheckbox.indeterminate = true;
}
};
const parentHandler = (e) => {
const isChecked = e.detail.checked;
childCheckboxes.forEach(cb => {
cb.checked = isChecked;
});
console.log('Parent changed:', e.detail);
};
const childHandler = () => {
updateParentState();
};
parentCheckbox.addEventListener('bdsChange', parentHandler);
childCheckboxes.forEach(cb => {
cb.addEventListener('bdsChange', childHandler);
});
// Cleanup function
return () => {
parentCheckbox.removeEventListener('bdsChange', parentHandler);
childCheckboxes.forEach(cb => {
cb.removeEventListener('bdsChange', childHandler);
});
};
}, []); // Empty dependency array to run only once…termediate-checkbox-state
|


indeterminateproperty to bds-checkbox componentChanges
Component (
checkbox.tsx)indeterminateboolean property (mutable, reflected)Styles (
checkbox.scss).checkbox--indeterminateand.checkbox--indeterminate-disabledstatesStorybook (
checkbox.stories.jsx)bds-grid,bds-typo) instead of raw HTMLTests
Screenshots
All States
Indeterminate Example
Original prompt
bds-checkboxadd intermediate state to the checkbox #1111✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.