Browse Source

ref(forms): Remove unnecessary RadioBoolean (#40026)

Evan Purkhiser 2 years ago
parent
commit
296f098f75

+ 0 - 18
docs-ui/stories/components/form-fields.stories.js

@@ -8,7 +8,6 @@ import NewBooleanField from 'sentry/components/forms/fields/booleanField';
 import CheckboxField from 'sentry/components/forms/fields/checkboxField';
 import DatePickerField from 'sentry/components/forms/fields/datePickerField';
 import FileField from 'sentry/components/forms/fields/fileField';
-import RadioBooleanField from 'sentry/components/forms/fields/radioBooleanField';
 import RadioField from 'sentry/components/forms/fields/radioField';
 import SelectField from 'sentry/components/forms/fields/selectField';
 import TextareaField from 'sentry/components/forms/fields/textareaField';
@@ -213,23 +212,6 @@ __FileField.storyName = 'File';
 
 _RadioField.storyName = 'Radio';
 
-export const _RadioBooleanField = ({disabled}) => (
-  <Form>
-    <RadioBooleanField
-      name="subscribe"
-      yesLabel="Yes, I would like to receive updates via email"
-      noLabel="No, I'd prefer not to receive these updates"
-      help="Help text for making an informed decision"
-      disabled={disabled}
-    />
-  </Form>
-);
-
-_RadioBooleanField.storyName = 'Radio - Boolean';
-_RadioBooleanField.args = {
-  disabled: false,
-};
-
 export const _SelectField = () => (
   <Form>
     <SelectField

+ 1 - 31
static/app/components/deprecatedforms/radioBooleanField.spec.jsx

@@ -1,7 +1,6 @@
-import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {render} from 'sentry-test/reactTestingLibrary';
 
 import {Form, RadioBooleanField} from 'sentry/components/deprecatedforms';
-import NewRadioBooleanField from 'sentry/components/forms/fields/radioBooleanField';
 
 describe('RadioBooleanField', function () {
   it('renders without form context', function () {
@@ -19,33 +18,4 @@ describe('RadioBooleanField', function () {
     );
     expect(wrapper.container).toSnapshot();
   });
-
-  it('renders new field without form context', function () {
-    const wrapper = render(
-      <NewRadioBooleanField name="fieldName" yesLabel="Yes" noLabel="No" />
-    );
-    expect(wrapper.container).toSnapshot();
-  });
-
-  it('can change values', function () {
-    const changeMock = jest.fn();
-    const blurMock = jest.fn();
-    render(
-      <NewRadioBooleanField
-        onChange={changeMock}
-        onBlur={blurMock}
-        name="fieldName"
-        yesLabel="Yes"
-        noLabel="No"
-      />
-    );
-
-    userEvent.click(screen.getByRole('radio', {name: 'Yes'}));
-    expect(changeMock).toHaveBeenCalledWith(true, expect.anything());
-
-    userEvent.click(screen.getByRole('radio', {name: 'No'}));
-    expect(changeMock).toHaveBeenCalledWith(false, expect.anything());
-
-    expect(blurMock).toHaveBeenCalledTimes(2);
-  });
 });

+ 0 - 104
static/app/components/forms/controls/radioBoolean.tsx

@@ -1,104 +0,0 @@
-import {forwardRef} from 'react';
-
-type OnChangeHandler = (
-  value: boolean,
-  event: React.ChangeEvent<HTMLInputElement>
-) => void;
-
-type OptionProps = {
-  label: string;
-  value: string;
-  checked?: boolean;
-  disabled?: boolean;
-  name?: string;
-  onBlur?: (event: React.ChangeEvent<HTMLInputElement>) => void;
-  onChange?: OnChangeHandler;
-};
-
-const Option = forwardRef(function Option(
-  {name, disabled, label, value, checked, onChange, onBlur}: OptionProps,
-  ref: React.Ref<HTMLInputElement>
-) {
-  function handleChange(e: React.ChangeEvent<HTMLInputElement>) {
-    const isTrue = e.target.value === 'true';
-
-    onChange?.(isTrue, e);
-    // Manually trigger blur to trigger saving on change
-    onBlur?.(e);
-  }
-
-  return (
-    <div className="radio">
-      <label style={{fontWeight: 'normal'}}>
-        <input
-          ref={ref}
-          type="radio"
-          value={value}
-          name={name}
-          checked={checked}
-          onChange={handleChange}
-          disabled={disabled}
-        />{' '}
-        {label}
-      </label>
-    </div>
-  );
-});
-
-type Props = {
-  disabled?: boolean;
-  name?: string;
-  noLabel?: string;
-  onBlur?: (event: React.ChangeEvent<HTMLInputElement>) => void;
-  onChange?: OnChangeHandler;
-  value?: boolean;
-  yesFirst?: boolean;
-  yesLabel?: string;
-};
-
-const RadioBoolean = forwardRef(function RadioBoolean(
-  {
-    disabled,
-    name,
-    onChange,
-    onBlur,
-    value,
-    yesFirst = true,
-    yesLabel = 'Yes',
-    noLabel = 'No',
-  }: Props,
-  ref: React.Ref<HTMLInputElement>
-) {
-  const yesOption = (
-    <Option
-      ref={ref}
-      value="true"
-      checked={value === true}
-      name={name}
-      disabled={disabled}
-      label={yesLabel}
-      onChange={onChange}
-      onBlur={onBlur}
-    />
-  );
-  const noOption = (
-    <Option
-      value="false"
-      checked={value === false}
-      name={name}
-      disabled={disabled}
-      label={noLabel}
-      onChange={onChange}
-      onBlur={onBlur}
-    />
-  );
-
-  return (
-    <div>
-      {yesFirst ? yesOption : noOption}
-      {yesFirst ? noOption : yesOption}
-    </div>
-  );
-});
-
-export default RadioBoolean;

+ 6 - 8
static/app/components/forms/controls/radioGroup.tsx

@@ -11,14 +11,6 @@ interface ContainerProps extends React.HTMLAttributes<HTMLDivElement> {
   orientInline?: boolean;
 }
 
-const Container = styled('div')<ContainerProps>`
-  display: grid;
-  gap: ${p => space(p.orientInline ? 3 : 1)};
-  grid-auto-flow: ${p => (p.orientInline ? 'column' : 'row')};
-  grid-auto-rows: max-content;
-  grid-auto-columns: max-content;
-`;
-
 interface BaseRadioGroupProps<C extends string> {
   /**
    * An array of [id, name, description]
@@ -93,6 +85,12 @@ const RadioGroup = <C extends string>({
   </Container>
 );
 
+const Container = styled('div')<ContainerProps>`
+  display: flex;
+  gap: ${p => space(p.orientInline ? 3 : 1)};
+  flex-direction: ${p => (p.orientInline ? 'row' : 'column')};
+`;
+
 const shouldForwardProp = (p: PropertyKey) =>
   typeof p === 'string' && !['disabled', 'animate'].includes(p) && isPropValid(p);
 

+ 0 - 16
static/app/components/forms/fields/radioBooleanField.tsx

@@ -1,16 +0,0 @@
-import omit from 'lodash/omit';
-
-import RadioBoolean from 'sentry/components/forms/controls/radioBoolean';
-
-import InputField, {InputFieldProps} from './inputField';
-
-export default function RadioBooleanField(props: Omit<InputFieldProps, 'field'>) {
-  return (
-    <InputField
-      {...props}
-      field={fieldProps => (
-        <RadioBoolean {...omit(fieldProps, ['onKeyDown', 'children'])} />
-      )}
-    />
-  );
-}

+ 1 - 1
static/app/components/forms/index.tsx

@@ -7,7 +7,7 @@ export {default as EmailField} from './fields/emailField';
 export {default as HiddenField} from './fields/hiddenField';
 export {default as InputField} from './fields/inputField';
 export {default as NumberField} from './fields/numberField';
-export {default as RadioBooleanField} from './fields/radioBooleanField';
+export {default as RadioField} from './fields/radioField';
 export {default as RangeField} from './fields/rangeField';
 export {default as SecretField} from './fields/secretField';
 export {default as SelectField} from './fields/selectField';

+ 4 - 4
static/app/views/admin/installWizard/index.spec.jsx

@@ -41,12 +41,12 @@ describe('InstallWizard', function () {
     render(<InstallWizard onConfigured={jest.fn()} />);
     expect(
       screen.getByRole('radio', {
-        name: 'Please keep my usage information anonymous',
+        name: 'Select Please keep my usage information anonymous',
       })
     ).not.toBeChecked();
     expect(
       screen.getByRole('radio', {
-        name: 'Send my contact information along with usage statistics',
+        name: 'Select Send my contact information along with usage statistics',
       })
     ).not.toBeChecked();
   });
@@ -71,12 +71,12 @@ describe('InstallWizard', function () {
     render(<InstallWizard onConfigured={jest.fn()} />);
     expect(
       screen.getByRole('radio', {
-        name: 'Please keep my usage information anonymous',
+        name: 'Select Please keep my usage information anonymous',
       })
     ).not.toBeChecked();
     expect(
       screen.getByRole('radio', {
-        name: 'Send my contact information along with usage statistics',
+        name: 'Select Send my contact information along with usage statistics',
       })
     ).not.toBeChecked();
   });

+ 9 - 8
static/app/views/admin/options.tsx

@@ -4,7 +4,7 @@ import {
   BooleanField,
   EmailField,
   NumberField,
-  RadioBooleanField,
+  RadioField,
   TextField,
 } from 'sentry/components/forms';
 import {t, tct} from 'sentry/locale';
@@ -15,10 +15,13 @@ type Section = {
   heading?: string;
 };
 
+// TODO(epurkhiser): This should really use the types from the form system, but
+// they're still pretty bad so that's difficult I guess?
 type Field = {
   key: string;
   label: React.ReactNode;
   allowEmpty?: boolean;
+  choices?: [value: string, label: string][];
   component?: React.ComponentType<any>;
   defaultValue?: () => string | number | false;
   disabled?: boolean;
@@ -26,12 +29,9 @@ type Field = {
   help?: React.ReactNode;
   max?: number;
   min?: number;
-  noLabel?: string;
   placeholder?: string;
   required?: boolean;
   step?: number;
-  yesFirst?: boolean;
-  yesLabel?: string;
 };
 
 // This are ordered based on their display order visually
@@ -246,11 +246,12 @@ const definitions: Field[] = [
   {
     key: 'beacon.anonymous',
     label: 'Usage Statistics',
-    component: RadioBooleanField,
+    component: RadioField,
     // yes and no are inverted here due to the nature of this configuration
-    noLabel: 'Send my contact information along with usage statistics',
-    yesLabel: 'Please keep my usage information anonymous',
-    yesFirst: false,
+    choices: [
+      ['false', 'Send my contact information along with usage statistics'],
+      ['true', 'Please keep my usage information anonymous'],
+    ],
     help: tct(
       'If enabled, any stats reported to sentry.io will exclude identifying information (such as your administrative email address). By anonymizing your installation the Sentry team will be unable to contact you about security updates. For more information on what data is sent to Sentry, see the [link:documentation].',
       {

+ 7 - 12
static/app/views/newsletterConsent.tsx

@@ -1,8 +1,8 @@
 import {useEffect} from 'react';
 
-import {ApiForm, InputField} from 'sentry/components/forms';
-import RadioBoolean from 'sentry/components/forms/controls/radioBoolean';
+import {ApiForm} from 'sentry/components/forms';
 import FieldWrapper from 'sentry/components/forms/field/fieldWrapper';
+import RadioField from 'sentry/components/forms/fields/radioField';
 import ExternalLink from 'sentry/components/links/externalLink';
 import NarrowLayout from 'sentry/components/narrowLayout';
 import {t, tct} from 'sentry/locale';
@@ -30,9 +30,12 @@ function NewsletterConsent({onSubmitSuccess}: Props) {
         <FieldWrapper stacked={false} hasControlState={false}>
           {t('Pardon the interruption, we just need to get a quick answer from you.')}
         </FieldWrapper>
-        <InputField
+        <RadioField
           name="subscribed"
-          key="subscribed"
+          choices={[
+            ['true', t('Yes, I would like to receive updates via email')],
+            ['false', t("No, I'd prefer not to receive these updates")],
+          ]}
           label={t('Email Updates')}
           required
           inline={false}
@@ -44,14 +47,6 @@ function NewsletterConsent({onSubmitSuccess}: Props) {
                `,
             {link: <ExternalLink href="https://sentry.io/privacy/" />}
           )}
-          field={fieldProps => (
-            <RadioBoolean
-              {...fieldProps}
-              label={t('Email Updates')}
-              yesLabel={t('Yes, I would like to receive updates via email')}
-              noLabel={t("No, I'd prefer not to receive these updates")}
-            />
-          )}
         />
       </ApiForm>
     </NarrowLayout>