Created MUI-based checkbox component

[tombogle]

Solves #123

---

This change is 

[rolfheij-sil]

Nice work!

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @katherinejensen00 and @tombogle)

---

src/renderer/components/papi-components/checkbox.tsx line 1 at r1 (raw file):

import { Checkbox as MuiCheckBox } from '@mui/material';

We should be consistent with naming the objects, and either go for Checkbox or CheckBox. I think Checkbox is prefered.
So we should change MuiCheckBox to MuiCheckbox, and use papi-checkbox for our CSS classes

---

src/renderer/components/papi-components/checkbox.tsx line 17 at r1 (raw file):

   * If `true`, the component is checked by default.
   */
  defaultChecked?: boolean;

According to our style guide (https://github.com/paranext/paranext/wiki/Code-Style-Guide#typescript-specific-guidelines) this should be something like isDefaultChecked

Also, on line 46 this prop is assigned false as a default. Please add an @default comment for that here

---

src/renderer/components/papi-components/checkbox.tsx line 44 at r1 (raw file):

 */
function CheckBox({
  isChecked: checked,

It's good practice to keep the names of the local variable in sync with the prop names, so you can change this line to isChecked, and change the assignment on line 54 to `check={isChecked}

---

src/stories/checkbox.stories.ts line 9 at r1 (raw file):

  tags: ['autodocs'],
  argTypes: {
    isDisabled: { control: 'boolean' },

You can also add argtypes for isChecked, isIndeterminate and defaultChecked

[katherinejensen00]

I think this is looking pretty good so far. This morning we talked about adding a label and a label position so I will hold off approving it until then, but I don't see anything in the initial check in to change beyond what Rolf already said.

[katherinejensen00]

Oops. I put a comment in the wrong place. I think these changes look good and will be ready after Rolf's changes and the label changes we talked about in the standup this morning.

[rolfheij-sil]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @tombogle)

---

src/renderer/components/papi-components/checkbox.tsx line 61 at r2 (raw file):

}: CheckboxProps) {
  return (
    <FormLabel disabled={isDisabled} error={hasError}>

It would be even nicer if we could render FormLabel conditionally. So only render it when a labelText is set.
@tjcouch-sil Would that be possible here?

---

src/stories/checkbox.stories.ts line 9 at r1 (raw file):

Previously, rolfheij-sil wrote…

You can also add argtypes for isChecked, isIndeterminate and defaultChecked

I don't see these added yet. And you can also add labelText and className with control type text

---

src/stories/checkbox.stories.ts line 23 at r2 (raw file):

export const DefaultChecked: Story = {
  args: { defaultChecked: true, labelText: 'Initially checked' },

This one isn't initally checked when in my Storybook environment. Is it checked by default in your local environment?

[tombogle]

src/renderer/components/papi-components/checkbox.tsx line 61 at r2 (raw file):

Previously, rolfheij-sil wrote…

It would be even nicer if we could render FormLabel conditionally. So only render it when a labelText is set.
@tjcouch-sil Would that be possible here?

Yes, that's what I'm planning to try to do, though I thought I'd maybe ask to make sure that was the right approach. Anyway, wanted to check in my first crack at it, so I can easily fall back if I mess it up.

[tombogle]

src/stories/checkbox.stories.ts line 23 at r2 (raw file):

Previously, rolfheij-sil wrote…

This one isn't initally checked when in my Storybook environment. Is it checked by default in your local environment?

I broke it when I renamed defaultChecked to isDefaultChecked. It's going to take some time to get used to a world where you can break things and only find out at runtime.

[tombogle]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @rolfheij-sil and @tjcouch-sil)

---

src/stories/checkbox.stories.ts line 9 at r1 (raw file):

Previously, rolfheij-sil wrote…

I don't see these added yet. And you can also add labelText and className with control type text

Done.

[tombogle]

Still TODO:

1. Make label element conditional
2. Expose ability to position label
3. Figure out why the isChecked control in storybook doesn't have any effect.

[rolfheij-sil]

Reviewed 2 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tombogle)

[katherinejensen00]

This looks pretty good. I just had a quick question for you.

Reviewed 5 of 5 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tombogle)

---

src/renderer/components/papi-components/checkbox.component.tsx line 10 at r5 (raw file):

   * If `true`, the component is checked.
   */
  isChecked?: boolean;

Should this be optional? Should implementers have to choose a default state or do you think it is better to leave it optional?

[rolfheij-sil]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @katherinejensen00)

---

src/renderer/components/papi-components/checkbox.component.tsx line 10 at r5 (raw file):

Previously, katherinejensen00 wrote…

Should this be optional? Should implementers have to choose a default state or do you think it is better to leave it optional?

I think this prop is optional (there's a '?'). In most cases this prop won't be used, as using it makes the checkbox a controlled component.
If users want to set a defaultstate they can use the isDefaultChecked prop, which defaults to false

[katherinejensen00]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tombogle)

---

src/renderer/components/papi-components/checkbox.component.tsx line 10 at r5 (raw file):

Previously, rolfheij-sil wrote…

I think this prop is optional (there's a '?'). In most cases this prop won't be used, as using it makes the checkbox a controlled component.
If users want to set a defaultstate they can use the isDefaultChecked prop, which defaults to false

That makes sense. Thanks! Then I think leaving it as optional is a good idea. The code looks good to me then.