Browse Source

feat(coreui): Make MultipleCheckbox a compound component (#42697)

`MultipleCheckbox` in its current form is difficult to customize due to
the `choices` prop. This prevents us from disabling a single checkbox
item or rendering the children differently.

Other changes:

- The `name` prop was added. The current component isn't properly
setting `name` on the inputs.
- Now uses `<Checkbox />` instead of `<input type="checkbox" />`

Before:

```tsx
<MultipleCheckbox choices={['one', 'One']} value={['one']} />
```

After:

```tsx
<MultipleCheckbox value={['one']}>
  <MultipleCheckbox.Item value="one">One</MultipleCheckbox.Item>
  <MultipleCheckbox.Item value="two">Two</MultipleCheckbox.Item>
</MultipleCheckbox>
```
Malachi Willey 2 years ago
parent
commit
b2bcda6ff1

+ 22 - 11
docs-ui/stories/components/multipleCheckbox.stories.js

@@ -1,3 +1,4 @@
+import {useState} from 'react';
 import {action} from '@storybook/addon-actions';
 
 import MultipleCheckbox from 'sentry/components/forms/controls/multipleCheckbox';
@@ -6,20 +7,30 @@ export default {
   title: 'Components/Forms/Controls/Multiple Checkbox',
   component: MultipleCheckbox,
   args: {
-    choices: [
-      ['foo', 'Foo'],
-      ['bar', 'Bar'],
-      ['baz', 'Baz'],
-      ['quux', 'Quux'],
-    ],
-    value: ['bar'],
-    onChange: (v, e) => {
-      action('MultipleCheckbox change')(v, e);
-    },
+    disabled: false,
+    name: 'multiple-checkbox-example',
   },
 };
 
-export const _MultipleCheckbox = ({...args}) => <MultipleCheckbox {...args} />;
+export const _MultipleCheckbox = ({...args}) => {
+  const [value, setValue] = useState(['bar']);
+
+  return (
+    <MultipleCheckbox
+      value={value}
+      onChange={(newValue, e) => {
+        setValue(newValue);
+        action('MultipleCheckbox change')(newValue, e);
+      }}
+      {...args}
+    >
+      <MultipleCheckbox.Item value="foo">Foo</MultipleCheckbox.Item>
+      <MultipleCheckbox.Item value="bar">Bar</MultipleCheckbox.Item>
+      <MultipleCheckbox.Item value="baz">Baz</MultipleCheckbox.Item>
+      <MultipleCheckbox.Item value="quux">Quux</MultipleCheckbox.Item>
+    </MultipleCheckbox>
+  );
+};
 
 _MultipleCheckbox.storyName = 'Multiple Checkbox';
 _MultipleCheckbox.parameters = {

+ 15 - 26
static/app/components/forms/controls/multipleCheckbox.spec.tsx

@@ -5,14 +5,11 @@ import MultipleCheckbox from 'sentry/components/forms/controls/multipleCheckbox'
 describe('MultipleCheckbox', function () {
   it('renders', function () {
     const {container} = render(
-      <MultipleCheckbox
-        choices={[
-          [0, 'Choice A'],
-          [1, 'Choice B'],
-          [2, 'Choice C'],
-        ]}
-        value={[1]}
-      />
+      <MultipleCheckbox name="test" value={[1]}>
+        <MultipleCheckbox.Item value={0}>Choice A</MultipleCheckbox.Item>
+        <MultipleCheckbox.Item value={1}>Choice B</MultipleCheckbox.Item>
+        <MultipleCheckbox.Item value={2}>Choice C</MultipleCheckbox.Item>
+      </MultipleCheckbox>
     );
 
     expect(container).toSnapshot();
@@ -21,15 +18,11 @@ describe('MultipleCheckbox', function () {
   it('unselects a checked input', function () {
     const onChange = jest.fn();
     render(
-      <MultipleCheckbox
-        choices={[
-          [0, 'Choice A'],
-          [1, 'Choice B'],
-          [2, 'Choice C'],
-        ]}
-        value={[1]}
-        onChange={onChange}
-      />
+      <MultipleCheckbox name="test" value={[1]} onChange={onChange}>
+        <MultipleCheckbox.Item value={0}>Choice A</MultipleCheckbox.Item>
+        <MultipleCheckbox.Item value={1}>Choice B</MultipleCheckbox.Item>
+        <MultipleCheckbox.Item value={2}>Choice C</MultipleCheckbox.Item>
+      </MultipleCheckbox>
     );
 
     userEvent.click(screen.getByLabelText('Choice B'));
@@ -39,15 +32,11 @@ describe('MultipleCheckbox', function () {
   it('selects an unchecked input', function () {
     const onChange = jest.fn();
     render(
-      <MultipleCheckbox
-        choices={[
-          [0, 'Choice A'],
-          [1, 'Choice B'],
-          [2, 'Choice C'],
-        ]}
-        value={[1]}
-        onChange={onChange}
-      />
+      <MultipleCheckbox name="test" value={[1]} onChange={onChange}>
+        <MultipleCheckbox.Item value={0}>Choice A</MultipleCheckbox.Item>
+        <MultipleCheckbox.Item value={1}>Choice B</MultipleCheckbox.Item>
+        <MultipleCheckbox.Item value={2}>Choice C</MultipleCheckbox.Item>
+      </MultipleCheckbox>
     );
 
     userEvent.click(screen.getByLabelText('Choice A'));

+ 96 - 48
static/app/components/forms/controls/multipleCheckbox.tsx

@@ -1,81 +1,129 @@
-import {useCallback} from 'react';
+import React, {createContext, ReactNode, useCallback, useContext, useMemo} from 'react';
 import styled from '@emotion/styled';
+import noop from 'lodash/noop';
 
 import Checkbox from 'sentry/components/checkbox';
 import space from 'sentry/styles/space';
-import {Choices} from 'sentry/types';
-import {defined} from 'sentry/utils';
 
-const MultipleCheckboxWrapper = styled('div')`
-  display: flex;
-  flex-wrap: wrap;
-`;
+type CheckboxValue = string | number;
 
-const Label = styled('label')`
-  display: inline-flex;
-  align-items: center;
-  font-weight: normal;
-  white-space: nowrap;
-  margin-right: 10px;
-  margin-bottom: 10px;
-  width: 20%;
-`;
-
-const CheckboxLabel = styled('span')`
-  margin-left: ${space(1)};
-`;
-
-type SelectedValue = (string | number)[];
+type SelectedValue = CheckboxValue[];
 
 type Props = {
-  choices: Choices;
+  children: ReactNode;
+  name: string;
   value: (string | number)[];
   disabled?: boolean;
   onChange?: (value: SelectedValue, event: React.ChangeEvent<HTMLInputElement>) => void;
 };
 
-function MultipleCheckbox({choices, value, disabled, onChange}: Props) {
-  const handleChange = useCallback(
-    (selectedValue: string | number, e: React.ChangeEvent<HTMLInputElement>) => {
-      let newValue: SelectedValue = [];
+type CheckboxItemProps = {
+  children: ReactNode;
+  value: string | number;
+  disabled?: boolean;
+  onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void;
+};
 
+type MultipleCheckboxContextValue = {
+  disabled: Props['disabled'];
+  handleChange: (
+    itemValue: CheckboxValue,
+    event: React.ChangeEvent<HTMLInputElement>
+  ) => void;
+  name: string;
+  value: Props['value'];
+};
+
+const MultipleCheckboxContext = createContext<MultipleCheckboxContextValue>({
+  handleChange: noop,
+  value: [],
+  name: '',
+  disabled: false,
+});
+
+function MultipleCheckbox({children, value, disabled, onChange, name}: Props) {
+  const handleChange = useCallback(
+    (itemValue: CheckboxValue, e: React.ChangeEvent<HTMLInputElement>) => {
       if (typeof onChange !== 'function') {
         return;
       }
 
-      if (e.target.checked) {
-        newValue = value ? [...value, selectedValue] : [value];
-      } else {
-        newValue = value.filter(v => v !== selectedValue);
-      }
+      const newValue = e.target.checked
+        ? [...value, itemValue]
+        : value.filter(v => v !== itemValue);
 
       onChange(newValue, e);
     },
     [value, onChange]
   );
 
+  const contextValue = useMemo(
+    () => ({
+      value,
+      handleChange,
+      name,
+      disabled,
+    }),
+    [disabled, handleChange, name, value]
+  );
+
   return (
-    <MultipleCheckboxWrapper>
-      {choices.map(([choiceValue, choiceLabel]) => (
-        <LabelContainer key={choiceValue}>
-          <Label>
-            <Checkbox
-              type="checkbox"
-              value={choiceValue}
-              onChange={e => handleChange(choiceValue, e)}
-              disabled={disabled}
-              checked={defined(value) && value.indexOf(choiceValue) !== -1}
-            />
-            <CheckboxLabel>{choiceLabel}</CheckboxLabel>
-          </Label>
-        </LabelContainer>
-      ))}
-    </MultipleCheckboxWrapper>
+    <MultipleCheckboxContext.Provider value={contextValue}>
+      <MultipleCheckboxWrapper>{children}</MultipleCheckboxWrapper>
+    </MultipleCheckboxContext.Provider>
   );
 }
 
+function Item({
+  value: itemValue,
+  children,
+  disabled: itemDisabled,
+  onChange,
+}: CheckboxItemProps) {
+  const {disabled, value, handleChange, name} = useContext(MultipleCheckboxContext);
+
+  return (
+    <LabelContainer>
+      <Label>
+        <Checkbox
+          name={name}
+          checked={value.includes(itemValue)}
+          disabled={disabled || itemDisabled}
+          onChange={e => {
+            handleChange(itemValue, e);
+            onChange?.(e);
+          }}
+          value={value.toString()}
+        />
+        <CheckboxLabel>{children}</CheckboxLabel>
+      </Label>
+    </LabelContainer>
+  );
+}
+
+MultipleCheckbox.Item = Item;
+
 export default MultipleCheckbox;
 
+const MultipleCheckboxWrapper = styled('div')`
+  display: flex;
+  flex-wrap: wrap;
+`;
+
+const Label = styled('label')`
+  display: inline-flex;
+  align-items: center;
+  font-weight: normal;
+  white-space: nowrap;
+  margin-right: 10px;
+  margin-bottom: 10px;
+  width: 20%;
+`;
+
+const CheckboxLabel = styled('span')`
+  margin-left: ${space(1)};
+`;
+
 const LabelContainer = styled('div')`
   width: 100%;
 

+ 8 - 8
static/app/views/settings/account/apiNewToken.tsx

@@ -9,13 +9,11 @@ import {Panel, PanelBody, PanelHeader} from 'sentry/components/panels';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {API_ACCESS_SCOPES, DEFAULT_API_ACCESS_SCOPES} from 'sentry/constants';
 import {t, tct} from 'sentry/locale';
-import {Choices} from 'sentry/types';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHeader';
 import TextBlock from 'sentry/views/settings/components/text/textBlock';
 
 const SORTED_DEFAULT_API_ACCESS_SCOPES = DEFAULT_API_ACCESS_SCOPES.sort();
-const API_CHOICES: Choices = API_ACCESS_SCOPES.map(s => [s, s]);
 const API_INDEX_ROUTE = '/settings/account/api/auth-tokens/';
 
 export default class ApiNewToken extends Component {
@@ -61,12 +59,14 @@ export default class ApiNewToken extends Component {
             >
               <PanelBody>
                 <FormField name="scopes" label={t('Scopes')} inline={false} required>
-                  {({value, onChange}) => (
-                    <MultipleCheckbox
-                      onChange={onChange}
-                      value={value}
-                      choices={API_CHOICES}
-                    />
+                  {({name, value, onChange}) => (
+                    <MultipleCheckbox onChange={onChange} value={value} name={name}>
+                      {API_ACCESS_SCOPES.map(scope => (
+                        <MultipleCheckbox.Item value={scope} key={scope}>
+                          {scope}
+                        </MultipleCheckbox.Item>
+                      ))}
+                    </MultipleCheckbox>
                   )}
                 </FormField>
               </PanelBody>

+ 9 - 9
static/app/views/settings/organizationApiKeys/organizationApiKeyDetails.tsx

@@ -9,7 +9,7 @@ import FormField from 'sentry/components/forms/formField';
 import {Panel, PanelBody, PanelHeader} from 'sentry/components/panels';
 import {API_ACCESS_SCOPES} from 'sentry/constants';
 import {t} from 'sentry/locale';
-import {Choices, Organization} from 'sentry/types';
+import {Organization} from 'sentry/types';
 import recreateRoute from 'sentry/utils/recreateRoute';
 import routeTitleGen from 'sentry/utils/routeTitle';
 import withOrganization from 'sentry/utils/withOrganization';
@@ -18,8 +18,6 @@ import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHea
 
 import {DeprecatedApiKey} from './types';
 
-const API_CHOICES: Choices = API_ACCESS_SCOPES.map(s => [s, s]);
-
 type RouteParams = {
   apiKey: string;
 };
@@ -93,12 +91,14 @@ class OrganizationApiKeyDetails extends AsyncView<Props, State> {
               <TextField label={t('API Key')} name="key" disabled />
 
               <FormField name="scope_list" label={t('Scopes')} inline={false} required>
-                {({value, onChange}) => (
-                  <MultipleCheckbox
-                    value={value}
-                    onChange={onChange}
-                    choices={API_CHOICES}
-                  />
+                {({name, value, onChange}) => (
+                  <MultipleCheckbox value={value} onChange={onChange} name={name}>
+                    {API_ACCESS_SCOPES.map(scope => (
+                      <MultipleCheckbox.Item value={scope} key={scope}>
+                        {scope}
+                      </MultipleCheckbox.Item>
+                    ))}
+                  </MultipleCheckbox>
                 )}
               </FormField>
 

+ 10 - 8
static/app/views/settings/project/serviceHookSettingsForm.tsx

@@ -8,10 +8,10 @@ import TextField from 'sentry/components/forms/fields/textField';
 import FormField from 'sentry/components/forms/formField';
 import {Panel, PanelBody, PanelHeader} from 'sentry/components/panels';
 import {t} from 'sentry/locale';
-import {Choices, ServiceHook} from 'sentry/types';
+import {ServiceHook} from 'sentry/types';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 
-const EVENT_CHOICES: Choices = ['event.alert', 'event.created'].map(e => [e, e]);
+const EVENT_CHOICES = ['event.alert', 'event.created'];
 
 type Props = {
   initialData: Partial<ServiceHook> & {isActive: boolean};
@@ -61,12 +61,14 @@ export default class ServiceHookSettingsForm extends Component<Props> {
               inline={false}
               help={t('The event types you wish to subscribe to.')}
             >
-              {({value, onChange}) => (
-                <MultipleCheckbox
-                  onChange={onChange}
-                  value={value}
-                  choices={EVENT_CHOICES}
-                />
+              {({name, value, onChange}) => (
+                <MultipleCheckbox onChange={onChange} value={value} name={name}>
+                  {EVENT_CHOICES.map(event => (
+                    <MultipleCheckbox.Item key={event} value={event}>
+                      {event}
+                    </MultipleCheckbox.Item>
+                  ))}
+                </MultipleCheckbox>
               )}
             </FormField>
           </PanelBody>