feat(Checkbox): indeterminate checkboxes#3123
Conversation
|
View your CI Pipeline Execution ↗ for commit 8a51bd3 ☁️ Nx Cloud last updated this comment at |
LinKCoding
left a comment
There was a problem hiding this comment.
This looks great, I left some thoughts/suggestions, mostly around SB stuff.
I looked over the stories and VO sounds great!
|
|
||
| expect(onChange).toHaveBeenCalled(); | ||
| }); | ||
| it('sets the input indeterminate state when the prop is passed', () => { |
There was a problem hiding this comment.
Would it help to also check that checked is also false (or !true? falsey?) here when indeterminate is true?
E.g. as a safeguard in case types get changed?
| import { BaseInputProps } from '../types'; | ||
| import { CheckboxCheckedUnion, CheckboxLabelUnion } from './types'; | ||
|
|
||
| /** Something will happen here */ |
| function syncedRefs(element: HTMLInputElement | null) { | ||
| intRef.current = element; | ||
| if (ref) { | ||
| if (typeof ref === 'object') { | ||
| ref.current = element; | ||
| } else { | ||
| ref(element); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Q: when would ref be passed in as a function?
There was a problem hiding this comment.
callback refs are sometimes used in animations and other things - here's some react official docs with fancy examples: Manipulating the DOM with Refs
| ); | ||
| }; | ||
|
|
||
| const NestedCheckboxExample: React.FC = () => { |
packages/styleguide/src/lib/Atoms/FormInputs/Checkbox/Checkbox.mdx
Outdated
Show resolved
Hide resolved
|
|
||
| <Canvas of={CheckboxStories.NestedCheckboxes} /> | ||
|
|
||
| ```tsx |
There was a problem hiding this comment.
see comment on exporting the example directly — but I don't think you need to add this
|
|
||
| <Canvas of={CheckboxStories.Indeterminate} /> | ||
|
|
||
| `indeterminate` is used for Checkboxes that are the header of a list of checkboxes. When clicked, all of its child checkboxes should be selected or unselected. Clicking on only some of the child checkboxes should make the header checkbox show the indeterminate state. |
There was a problem hiding this comment.
indeterminateis used for Checkboxes that are the header of a list of checkboxes.
I think calling the parent a "header" might be confusing — maybe sticking with "parent" and "nested" like in your example works. Something like:
indeterminate is used for a Checkbox that is the parent of a list of checkboxes.
or GPT has this suggestion:
indeterminate is used for a parent Checkbox that represents the combined state of a group of nested checkboxes.
There was a problem hiding this comment.
When clicked, all of its child checkboxes should be selected or unselected. Clicking on only some of the child checkboxes should make the header checkbox show the indeterminate state.
Maybe: "When the parent checkbox is checked or blank, all of its child checkboxes should be selected or unselected, respectively. The indeterminate state is shown when only some of the child checkboxes are checked."
packages/styleguide/src/lib/Atoms/FormInputs/Checkbox/Checkbox.mdx
Outdated
Show resolved
Hide resolved
….mdx Co-authored-by: Kenny Lin <kenny.lin.91@gmail.com>
….mdx Co-authored-by: Kenny Lin <kenny.lin.91@gmail.com>
|
🚀 Styleguide deploy preview ready! |
📬Published Alpha Packages:@codecademy/gamut@65.1.0-alpha.8a51bd.0 |

Overview
Adds an indeterminate state to
CheckboxI structured these based on guidance from Mark to use a native checkbox instead of using aria-checked.
PR Checklist
Testing Instructions
checked+indeterminatecannot be true at the same time)PR Links and Envs