[Checkbox] Component and Hook #159

atomiks · 2024-03-06T08:32:39Z

I have followed (at least) the PR section of the contributing guide.

Closes #24.

This implementation closely follows #135. I reimplemented several of the utils files to get working, but they're mostly the same as in that PR, so they'll be merged without too many conflicts (aside from the getStyleHookProps utility function). Most of the logic here can actually be reused with that PR, but I assume that could be refactored in the future once both are merged to main.

danilo-leal

Just a very quick pass through! Excited to have this one out, particularly this fast — great work! 💥 One little thing, too, so we don't forget: removing the "Planned" tag from the Checkbox sidenav item on the pages.ts file!

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminate.js

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminateGroup.js

docs/data/base/components/checkbox/UnstyledCheckboxIntroduction/tailwind/index.tsx

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminate.js

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminateGroup.js

docs/data/base/components/checkbox/checkbox.md

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminate.js

packages/mui-base/src/Checkbox/Checkbox.test.tsx

michaldudak · 2024-03-20T14:31:47Z

packages/mui-base/src/Checkbox/CheckboxIndicator.test.tsx

+    skip: ['reactTestRenderer'],
+  }));
+
+  it('should not render indicator by default', () => {


I wonder if it's worth having an indicator present at all times - to render a different content when the checkbox is unchecked.

By default, it's useful to have it conditionally unmount so it doesn't render the children without needing to hide it with CSS. keepMounted lets you implement CSS animations

oliviertassinari · 2024-03-20T16:13:33Z

I pushed an empty commit, it seems to fix Circle CI. I don't know why.

atomiks · 2024-03-20T20:17:08Z

@oliviertassinari Seems like it started happening again once I pulled and pushed new commits 😞

What I can do is add the revert commit here so the tests can run for approval, then remove it & merge at the end. I hope it's just an artifact of these PRs being opened before the CI change for some reason, so new ones that get opened with fresh commits don't have the issue.

michaldudak · 2024-03-20T21:00:15Z

This could be related: https://discuss.circleci.com/t/certain-jobs-within-workflow-hang-forever-with-no-explanation-just-for-me/39304

@oliviertassinari I suspect that only the org members can trigger workflows using the context you created.

Adding @atomiks to the org is one thing, but this setting is not acceptable if we want to accept community contributions.

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminate.js

oliviertassinari · 2024-03-20T21:54:58Z

only the org members can trigger workflows using the context

The author of mui/mui-x#12484 isn't inside the MUI organization and yet has the same CircleCI context config, it runs. https://discuss.circleci.com/t/certain-jobs-within-workflow-hang-forever-with-no-explanation-just-for-me/39304 has an error message, we don't have one. There are no "restrictions" setup on this context. James is inside the MUI GiHub organization, no?

I raised the bug to Circle CI: https://support.circleci.com/hc/en-us/requests/147508. At the very least, the UI should explain what's wrong. I will report back here.

atomiks · 2024-03-21T19:14:55Z

@oliviertassinari I did file a support request earlier. Here's their response:

The error you are seeing is related to the org-global context you are trying to use in the pull request.
Since you are not a member of this organization, it will fail.

So it looks like @michaldudak is right. We should remove the requirement to be in the org for community contributions. I don't know why it worked for the MUI X PR though... my CircleCI account is a bit strange, as when I tried to join the MUI org, I got /vcs-connect-logged-out with an "identity taken" error. They're asking me to send a network log, which I can provide

Update

Support unlinked my account and I signed back in with GitHub to the MUI org, and now everything works again. It's possible this may be a problem for community contributors in this repo specifically, or maybe not, so we'll need to keep an eye on it.

oliviertassinari · 2024-03-22T16:48:27Z

@atomiks the answer I got from CircleCI support:

Hi Olivier, Thank you for your patience. Based on the logs and settings enabled, the user should not be seeing errors. I found the following in our internal logs Bad credentials . We've seen this issue occur due to stale tokens.
To resolve this, lets try having the user atomiks sign into CircleCI and then trigger a build. Please let me know how it goes. I am standing by for your response.

Update

Ok great, so we can continue to use CircleCI context 🎉

Fix test Fix doc wording Add tests for indeterminate prop Fix prop diff Update based on discussions Update demos Add conditional unmounting to CheckboxIndicator Avoid passing keepMounted prop to element Update docs/data/base/components/checkbox/UnstyledCheckboxIndeterminate.js Co-authored-by: Danilo Leal <[email protected]> Signed-off-by: atomiks <[email protected]> Update docs/data/base/components/checkbox/UnstyledCheckboxIndeterminateGroup.js Co-authored-by: Danilo Leal <[email protected]> Signed-off-by: atomiks <[email protected]> Update docs/data/base/components/checkbox/UnstyledCheckboxIndeterminateGroup.js Co-authored-by: Danilo Leal <[email protected]> Signed-off-by: atomiks <[email protected]> Update docs/data/base/components/checkbox/UnstyledCheckboxIntroduction/tailwind/index.tsx Co-authored-by: Danilo Leal <[email protected]> Signed-off-by: atomiks <[email protected]> Update docs/data/base/components/checkbox/UnstyledCheckboxIndeterminateGroup.js Co-authored-by: Danilo Leal <[email protected]> Signed-off-by: atomiks <[email protected]> Update docs/data/base/components/checkbox/checkbox.md Co-authored-by: Danilo Leal <[email protected]> Signed-off-by: atomiks <[email protected]> Use 24px size Fix formatting Remove planned from Checkbox Generate API docs and sync with Switch Simplify types Use undefined fallback due to defaults Remove unused file Remove unused colors Simplify API Resolve styling inconsistencies Remove unused inputRef prop Revert otherProps spread location Remove unused colors i18n Try deleting tests Add back CheckboxIndicator tests Bifurcate Checkbox tests Bifurcate Checkbox tests Bifurcate Checkbox tests Bifurcate Checkbox tests Bifurcate Checkbox tests Bifurcate Checkbox tests Bifurcate Checkbox tests Bifurcate Checkbox tests Try deleting Button test Skip tests rerun Reuse variable Update Rerun docs generation Use describeConformance Update tests Update docs ci

oliviertassinari · 2024-07-29T13:07:39Z

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminateGroup.tsx

+            {isIndeterminate ? <HorizontalRuleIcon /> : <CheckIcon />}
+          </Indicator>
+        </Checkbox>
+        <Label htmlFor={id} onMouseDown={(e) => e.preventDefault()}>


We avoid using shorthands like e in the codebase, especially for the docs as it makes it not very clear to entry-level developers what the variable is about.

Suggested change

<Label htmlFor={id} onMouseDown={(e) => e.preventDefault()}>

<Label htmlFor={id} onMouseDown={(event) => event.preventDefault()}>

Fixed in 25eb16b to match with the rest of the codebase.

atomiks marked this pull request as draft March 6, 2024 08:32

atomiks force-pushed the feat/Checkbox branch from 57dd93d to 54d6b03 Compare March 7, 2024 07:46

atomiks added the component: checkbox This is the name of the generic UI component, not the React module! label Mar 7, 2024

atomiks force-pushed the feat/Checkbox branch 2 times, most recently from 8a2a511 to b049801 Compare March 7, 2024 08:19

colmtuite requested a review from a team March 11, 2024 09:28

atomiks marked this pull request as ready for review March 11, 2024 09:46

danilo-leal reviewed Mar 11, 2024

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 12, 2024

colmtuite requested review from michaldudak and colmtuite and removed request for a team March 12, 2024 12:23

colmtuite approved these changes Mar 12, 2024

View reviewed changes

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 14, 2024

danilo-leal reviewed Mar 14, 2024

View reviewed changes

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminate.js Show resolved Hide resolved

atomiks force-pushed the feat/Checkbox branch 3 times, most recently from 37bfe43 to 97147b4 Compare March 20, 2024 09:54

atomiks closed this Mar 20, 2024

atomiks reopened this Mar 20, 2024

atomiks mentioned this pull request Mar 20, 2024

[Test][Checkbox] Component and Hook #206

Closed

atomiks force-pushed the feat/Checkbox branch 2 times, most recently from 5670d18 to ec78eaf Compare March 20, 2024 12:01

michaldudak reviewed Mar 20, 2024

View reviewed changes

atomiks force-pushed the feat/Checkbox branch 5 times, most recently from d11f0a8 to ab9b989 Compare March 20, 2024 16:58

atomiks force-pushed the feat/Checkbox branch from ab9b989 to 540600f Compare March 20, 2024 17:01

danilo-leal reviewed Mar 20, 2024

View reviewed changes

docs/data/base/components/checkbox/UnstyledCheckboxIndeterminate.js Show resolved Hide resolved

atomiks force-pushed the feat/Checkbox branch from 9b1b91a to f977c12 Compare March 21, 2024 23:07

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 25, 2024

atomiks and others added 6 commits April 2, 2024 19:01

run ci

f6cc263

Address feedback

9883900

Generate docs, sync with Switch

dc98cff

ci

ef6dc01

Delete index.js

644be22

atomiks force-pushed the feat/Checkbox branch from 74c52c9 to 644be22 Compare April 2, 2024 08:03

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 2, 2024

atomiks added 4 commits April 2, 2024 19:05

Use mergeReactProps util

50bac84

Update types

27f7f4d

Add id reference example

bdca632

Docs

c2c53af

atomiks merged commit 6f74390 into mui:master Apr 2, 2024
16 checks passed

atomiks deleted the feat/Checkbox branch April 2, 2024 08:24

oliviertassinari reviewed Jul 29, 2024

View reviewed changes

oliviertassinari added the new feature New feature or request label Jul 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Checkbox] Component and Hook #159

[Checkbox] Component and Hook #159

atomiks commented Mar 6, 2024 •

edited

Loading

danilo-leal left a comment

michaldudak Mar 20, 2024

atomiks Mar 20, 2024

oliviertassinari commented Mar 20, 2024

atomiks commented Mar 20, 2024 •

edited

Loading

michaldudak commented Mar 20, 2024 •

edited

Loading

oliviertassinari commented Mar 20, 2024 •

edited

Loading

atomiks commented Mar 21, 2024 •

edited

Loading

oliviertassinari commented Mar 22, 2024 •

edited

Loading

oliviertassinari Jul 29, 2024

	<Label htmlFor={id} onMouseDown={(e) => e.preventDefault()}>
	<Label htmlFor={id} onMouseDown={(event) => event.preventDefault()}>

[Checkbox] Component and Hook #159

[Checkbox] Component and Hook #159

Conversation

atomiks commented Mar 6, 2024 • edited Loading

danilo-leal left a comment

Choose a reason for hiding this comment

michaldudak Mar 20, 2024

Choose a reason for hiding this comment

atomiks Mar 20, 2024

Choose a reason for hiding this comment

oliviertassinari commented Mar 20, 2024

atomiks commented Mar 20, 2024 • edited Loading

michaldudak commented Mar 20, 2024 • edited Loading

oliviertassinari commented Mar 20, 2024 • edited Loading

atomiks commented Mar 21, 2024 • edited Loading

Update

oliviertassinari commented Mar 22, 2024 • edited Loading

oliviertassinari Jul 29, 2024

Choose a reason for hiding this comment

atomiks commented Mar 6, 2024 •

edited

Loading

atomiks commented Mar 20, 2024 •

edited

Loading

michaldudak commented Mar 20, 2024 •

edited

Loading

oliviertassinari commented Mar 20, 2024 •

edited

Loading

atomiks commented Mar 21, 2024 •

edited

Loading

oliviertassinari commented Mar 22, 2024 •

edited

Loading