Add CheckablePanel component

[zinckiwi]

Summary

Adds a CheckablePanel component that wraps radio buttons or checkboxes and allows nested/child content to be more easily associated with the parent checkable.

In the course of working on this, also updated form/checkbox/* to TS.

It's been a while since I've done anything from scratch here so just yell if there's something I missed.

Checklist

• Checked in dark mode
• Checked in mobile
• Checked in IE11 and Firefox
• Props have proper autodocs
• Added documentation examples
• Added or updated jest tests
• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[zinckiwi]

Not sure what to do with the build failure (on warning):

/app/src/components/datagrid/data_grid_schema.tsx
370:6  warning  React Hook useMemo has missing dependencies: 'definedColumnSchemas' and 'inMemory'. Either include them or remove the dependency array  react-hooks/exhaustive-deps

Only showed up when I pulled in latest master so I'd figured it was something funky with my node_modules or something as yarn ran clean.

[cchaos]

@zinckiwi Sorry it's taking so long to get to your PR. There were quite a few large ones going in this week. But if you rebase with master that warning should be gone now. I'll be sure to start reviewing this first thing next week.

[cchaos]

@zinckiwi This is a really great start and thank you for converting the checkbox files to TS! You'll want to be sure to add a changelog entry for that work as well.

Design

My initial thought is the extra blue accents (border and shaded background) is a bit too much. I'm looking at this compared to the switches below and the radios above in the examples and I think just letting the checkbox and radio show the selected state.

Disabled state

This probably needs to be more noticeably disabled. For instance in the radios and switches we decrease the contrast of the label.

Code

Looks really good! Thank you for using SASS variables and correct BEM naming schemes.

My only concern with the layout is the use of EuiFlexGroup/Item. We find using this component within our own EUI components can become brittle and it's a bit safer to just write our own elements and flex styles.

I left a few other comments directly.

Docs

Question about the second example. I'd assume that for the nested example, the nested radio group should be disabled if the actual Option two is not checked? Otherwise users could select options but they never get applied since the parent isn't selected.

Accessibility

Looks pretty good. Mainly got one Axe error around the fact that there's no unifying label for the top level radio group. Either we can just add another part to the docs example or we may need to create a EuiCheckablePanelGroup to enforce a unifying label.

---

@zinckiwi Also feel free to hand this off to us if you don't have time for these updates. Or if you want to do some but not all that's perfectly fine too.

[chandlerprall]

Instead of casting here, let's add indeterminate: false to the defaultProps definition

[zinckiwi]

This still complains due to the entry in the Props type being optional -- leaving this as-is unless you know a way around that. I'd have thought it would parse defaultProps and figure out that by runtime it can't be undefined, but apparently not?

[chandlerprall]

Well that's annoying. You can tell TypeScript the value really does exist by appending an ! at the end of the property access:

this.inputRef.indeterminate = this.props.indeterminate!;

[zinckiwi]

Thanks for the reviews both, will get to work on the changes. Quickly about the nested radios, perhaps not the greatest example but I wanted to show that nested controls would be a good use case, not necessarily do the whole show/hide implementation. Can revise that if you think it's misleading.

[gjones]

@cchaos, only minor comment on my end is the lack of an active/selected state (outside the radio itself). I understand where you're coming from when comparing them to the switches and other radio groups. I think we were looking at these as something closer to EuiCard-selectable than a regular form element.

I'm cool removing the border colour, but when it comes to EuiCard we at least tend to increase the shadow, show some extra depth to allow it to stand out at a glance, is this something we could think about here?

I'm just trying to ensure that if we have 6 or 8 of these, potentially two per row, that the contrast between selected and unselected is enough to instantly identify it even if there are eye catching elements (ie. product logos) inside the contents of the Panel itself.

That said, I trust your opinion on this, and how it fits into the greater framework. Just wanted to bring up a use case that may not have been considered.

[cchaos]

@gjones I think it comes down to context and documentation then. You compare it to the selectable type of EuiCard and not as just another version of a radio/checkbox group. I think that's a big distinction that we will have to make clear to the consumer and there are just two tasks involved to do that.

1. Change the name of this component to align with "Card" instead of "Panel". EuiCheckablePanel --> EuiCheckRadioCard (or something... naming is hard).
2. Move both the component and the docs example to be nested in the card folders.

This should gear this new component's purpose more towards layout than a type of form element.

The only oddity then would be the inconsistency in selected state colors, but changing this component's border to green becomes too crayola when mixed with the blue of the radio.

[zinckiwi]

@cchaos Are those two bullets requests, then? No problem, but want to confirm given that currently it's very much an extension to form elements (to the point of passing the props through).

@chandlerprall yeah, my goof on the typing... that was why I converted checkbox to begin with, just blanked when I was actually finished with it!

[zinckiwi]

Alternatively, we can keep this Cloud-side for now until there's another would-be consumer of it. I can pull the new stuff out and not waste the TSing of checkbox if you'd like to save the prospect of CheckablePanel (or whatever it becomes) for another day.

[cchaos]

@zinckiwi Don't worry about my last comment. I can do some re-organizing of docs / components to fit with how we expect the usage afterward. You can also keep the blue border/shading in for the selected state as well.

[zinckiwi]

Okay, I think that's all the feedback dealt with. Would appreciate another pass!

[cchaos]

I have pushed 5 commits (details in the commit messages). Mainly moving this new component to /card and renaming it EuiCheckableCard but also simplifying a few styles and adding a11y fixes.

Then I just have one last comment about the changelog, but I'm a 👍

[chandlerprall]

One small change requested ( left it in #2555 (comment) ), otherwise changes LGTM; pulled & tested locally.

[chandlerprall]

@zinckiwi one more thing I caught while switching branches - you removed @types/sinon from the package.json but it is still listed in the yarn.lock. Please re-run yarn and commit the updated yarn.lock file.