components: Added indeterminate checkbox #2419
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 74f32e8:
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/components/src/Checkbox.tsx
Outdated
| @@ -76,7 +100,7 @@ export const Checkbox: ForwardRef<HTMLInputElement, CheckboxProps> = | |||
| }} | |||
| /> | |||
| <Box | |||
| as={CheckboxIcon} | |||
| as={(SVGProps) => CheckboxIcon({ isIndeterminate, ...SVGProps })} | |||
There was a problem hiding this comment.
This won't be a performance problem here, but we usually wouldn't want to create new components like this.
Could we use the :indeterminate pseudoclass for with general sibling combinator ~ to control what's displayed, just like we use :checked right now?
There was a problem hiding this comment.
I tried that first, but the attribute indeterminate on input cannot be set directly using HTML . And since none of the other components were performing side effects using hooks, I used this approach.
There was a problem hiding this comment.
All right, so I suppose we have a few choices.
- We can either leave it as it is,
- Stop doing
<Box as={CheckboxIcon}>and use<CheckboxIcon indeterminate={indeterminate}instead with the common props for all icons moved inside ofCheckboxIcon - Maybe even inline
CheckboxIconand see if we can gain something by removing this layer of abstraction - or use a hook like you mentioned
There was a problem hiding this comment.
I think we can go with the second option and use <CheckboxIcon indeterminate={indeterminate} as you said.
There was a problem hiding this comment.
Nice! I like this approach. Sorry for being so annoying here :)
There was a problem hiding this comment.
No worries,
Have made the changes.
|
Nice one @dev-cj! Feel free to just update the snapshot with |
|
Nice work! I'm merging to |
|
🚀 PR was released in |
hasparus
added
released
|
Hey, guys! I was looking for the indeterminate state and found this PR. It says merged and released, but it doesn't seem present in the current version. I found no mentions of reverting it in the changelog, nor in the git history. After digging a bit more it looks like these changes were deleted by Merge branch 'develop' into stable. Any idea if this was intentional? |
Checkbox path is sourced from Material Design (same as the other ones)
theme-ui2.mp4