refactor(Radio):

- small changes from PR reviews - removes children from Radio.GroupWrapper
apache · Jan 30, 2025 · 5dead35 · 5dead35
1 parent 96306af
commit 5dead35
Show file tree

Hide file tree

Showing 11 changed files with 117 additions and 94 deletions.
diff --git a/superset-frontend/src/components/Chart/DrillBy/useDisplayModeToggle.tsx b/superset-frontend/src/components/Chart/DrillBy/useDisplayModeToggle.tsx
@@ -19,17 +19,14 @@
 
 import { useMemo, useState } from 'react';
 import { css, SupersetTheme, t } from '@superset-ui/core';
-import { Radio, CheckboxOptionType } from 'src/components/Radio';
+import { Radio } from 'src/components/Radio';
 import { DrillByType } from '../types';
 
 export const useDisplayModeToggle = () => {
   const [drillByDisplayMode, setDrillByDisplayMode] = useState<DrillByType>(
     DrillByType.Chart,
   );
-  const customButtons: CheckboxOptionType[] = [
-    { label: t('Chart'), value: DrillByType.Chart },
-    { label: t('Table'), value: DrillByType.Table },
-  ];
+
   const displayModeToggle = useMemo(
     () => (
       <div
@@ -43,14 +40,16 @@ export const useDisplayModeToggle = () => {
             setDrillByDisplayMode(value);
           }}
           defaultValue={DrillByType.Chart}
-          options={customButtons}
-          value={drillByDisplayMode}
+          options={[
+            { label: t('Chart'), value: DrillByType.Chart },
+            { label: t('Table'), value: DrillByType.Table },
+          ]}
           optionType="button"
           buttonStyle="outline"
         />
       </div>
     ),
-    [drillByDisplayMode],
+    [],
   );
   return { displayModeToggle, drillByDisplayMode };
 };
diff --git a/superset-frontend/src/components/Radio/Radio.stories.tsx b/superset-frontend/src/components/Radio/Radio.stories.tsx
@@ -32,10 +32,18 @@ export default {
 };
 
 const RadioArgsType = {
-  onChange: { action: 'changed' },
-  checked: { control: 'boolean' },
-  disabled: { control: 'boolean' },
-  defaultChecked: { control: 'boolean' },
+  value: {
+    control: 'text',
+    description: 'The value of the radio button.',
+  },
+  disabled: {
+    control: 'boolean',
+    description: 'Whether the radio button is disabled or not.',
+  },
+  checked: {
+    control: 'boolean',
+    description: 'The checked state of the radio button.',
+  },
 };
 
 const radioGroupWrapperArgsType = {
@@ -64,14 +72,20 @@ const radioGroupWrapperArgsType = {
     description: 'Alignment of the Space layout',
     if: { arg: 'enableSpaceConfig', truthy: true },
   },
+  'spaceConfig.wrap': {
+    control: 'boolean',
+    description:
+      'Controls whether the items inside the Space component should wrap to the next line when space is insufficient',
+    if: { arg: 'enableSpaceConfig', truthy: true },
+  },
 };
 
 export const RadioStory = {
   args: {
-    children: 'Radio',
     value: 'radio1',
     disabled: false,
     checked: false,
+    children: 'Radio',
   },
   argTypes: RadioArgsType,
 };
@@ -86,27 +100,16 @@ RadioButtonStory.args = {
 };
 RadioButtonStory.argTypes = RadioArgsType;
 
-export const RadioGroupWithCustomRadioStory = (
-  args: RadioGroupWrapperProps,
-) => (
-  <Radio.GroupWrapper {...args}>
-    <Radio value="option1">Custom Option 1</Radio>
-    <Radio value="option2">Custom Option 2</Radio>
-    <Radio value="option3">Custom Option 3</Radio>
-  </Radio.GroupWrapper>
-);
-RadioGroupWithCustomRadioStory.args = {
-  spaceConfig: { direction: 'vertical', size: 'middle', align: 'center' },
-  size: 'middle',
-  disabled: false,
-};
-RadioGroupWithCustomRadioStory.argTypes = radioGroupWrapperArgsType;
-
 export const RadioGroupWithOptionsStory = (args: RadioGroupWrapperProps) => (
   <Radio.GroupWrapper {...args} />
 );
 RadioGroupWithOptionsStory.args = {
-  spaceConfig: { direction: 'vertical', size: 'middle', align: 'center' },
+  spaceConfig: {
+    direction: 'vertical',
+    size: 'middle',
+    align: 'center',
+    wrap: false,
+  },
   size: 'middle',
   options: [
     {

diff --git a/superset-frontend/src/components/Radio/index.tsx b/superset-frontend/src/components/Radio/index.tsx
@@ -32,36 +32,25 @@ export type RadioGroupWrapperProps = RadioGroupProps & {
     align?: SpaceProps['align'];
     wrap?: SpaceProps['wrap'];
   };
-  options?: CheckboxOptionType[];
-  children?: React.ReactNode;
+  options: CheckboxOptionType[];
 };
 
 const RadioGroup = ({
-  spaceConfig = {
-    direction: 'horizontal',
-    size: 'middle',
-    align: 'center',
-    wrap: false,
-  },
+  spaceConfig,
   options,
-  children,
   ...props
 }: RadioGroupWrapperProps) => {
-  const content = options
-    ? options.map((option: CheckboxOptionType) => (
-        <Radio key={option.value} value={option.value}>
-          {option.label}
-        </Radio>
-      ))
-    : children;
-
+  const content = options.map((option: CheckboxOptionType) => (
+    <Radio key={option.value} value={option.value}>
+      {option.label}
+    </Radio>
+  ));
   return (
     <Radio.Group {...props}>
       {spaceConfig ? <Space {...spaceConfig}>{content}</Space> : content}
     </Radio.Group>
   );
 };
-
 export type {
   RadioChangeEvent,
   RadioGroupProps,

diff --git a/superset-frontend/src/components/Table/header-renderers/HeaderWithRadioGroup.tsx b/superset-frontend/src/components/Table/header-renderers/HeaderWithRadioGroup.tsx
@@ -56,7 +56,12 @@ function HeaderWithRadioGroup(props: HeaderWithRadioGroupProps) {
               {groupTitle}
             </div>
             <Radio.GroupWrapper
-              spaceConfig={{ direction: 'vertical', size: 4 }}
+              spaceConfig={{
+                direction: 'vertical',
+                size: 4,
+                wrap: false,
+                align: 'center',
+              }}
               value={value}
               onChange={e => {
                 onChange(e.target.value);

diff --git a/...board/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/...board/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -1107,14 +1107,15 @@ const FiltersConfigForm = (
                       label={<StyledLabel>{t('Sort type')}</StyledLabel>}
                     >
                       <Radio.GroupWrapper
+                        options={[
+                          { value: true, label: t('Sort ascending') },
+                          { value: false, label: t('Sort descending') },
+                        ]}
                         onChange={value => {
                           onSortChanged(value.target.value);
                           formChanged();
                         }}
-                      >
-                        <Radio value>{t('Sort ascending')}</Radio>
-                        <Radio value={false}>{t('Sort descending')}</Radio>
-                      </Radio.GroupWrapper>
+                      />
                     </StyledRowFormItem>
                     {hasMetrics && (
                       <StyledRowSubFormItem
@@ -1186,17 +1187,18 @@ const FiltersConfigForm = (
                           onEnableSingleValueChanged(value.target.value);
                           formChanged();
                         }}
-                      >
-                        <Radio value={SingleValueType.Minimum}>
-                          {t('Minimum')}
-                        </Radio>
-                        <Radio value={SingleValueType.Exact}>
-                          {t('Exact')}
-                        </Radio>
-                        <Radio value={SingleValueType.Maximum}>
-                          {t('Maximum')}
-                        </Radio>
-                      </Radio.GroupWrapper>
+                        options={[
+                          {
+                            label: t('Minimum'),
+                            value: SingleValueType.Minimum,
+                          },
+                          { label: t('Exact'), value: SingleValueType.Exact },
+                          {
+                            label: t('Maximum'),
+                            value: SingleValueType.Maximum,
+                          },
+                        ]}
+                      />
                     </StyledRowFormItem>
                   </CollapsibleControl>
                 </CleanFormItem>

diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx
@@ -141,14 +141,20 @@ const FormatPicker = ({
   value: FormatPickerValue;
 }) => (
   <Radio.GroupWrapper
-    spaceConfig={{ direction: 'vertical', size: 15 }}
+    spaceConfig={{
+      direction: 'vertical',
+      size: 15,
+      align: 'center',
+      wrap: false,
+    }}
     size="large"
     value={value}
     onChange={onChange}
-  >
-    <Radio value={FormatPickerValue.Formatted}>{t('Formatted date')}</Radio>
-    <Radio value={FormatPickerValue.Original}>{t('Original value')}</Radio>
-  </Radio.GroupWrapper>
+    options={[
+      { label: t('Formatted date'), value: FormatPickerValue.Formatted },
+      { label: t('Original value'), value: FormatPickerValue.Original },
+    ]}
+  />
 );
 
 const FormatPickerContainer = styled.div`

diff --git a/...t-frontend/src/explore/components/controls/DateFilterControl/components/CalendarFrame.tsx b/...t-frontend/src/explore/components/controls/DateFilterControl/components/CalendarFrame.tsx
@@ -46,7 +46,12 @@ export function CalendarFrame({ onChange, value }: FrameComponentProps) {
         {t('Configure Time Range: Previous...')}
       </div>
       <Radio.GroupWrapper
-        spaceConfig={{ direction: 'vertical', size: 15 }}
+        spaceConfig={{
+          direction: 'vertical',
+          size: 15,
+          align: 'center',
+          wrap: false,
+        }}
         size="large"
         value={value}
         onChange={(e: any) => onChange(e.target.value)}

diff --git a/...set-frontend/src/explore/components/controls/DateFilterControl/components/CommonFrame.tsx b/...set-frontend/src/explore/components/controls/DateFilterControl/components/CommonFrame.tsx
@@ -42,7 +42,12 @@ export function CommonFrame(props: FrameComponentProps) {
         {t('Configure Time Range: Last...')}
       </div>
       <Radio.GroupWrapper
-        spaceConfig={{ direction: 'vertical', size: 15 }}
+        spaceConfig={{
+          direction: 'vertical',
+          size: 15,
+          align: 'center',
+          wrap: false,
+        }}
         size="large"
         value={commonRange}
         onChange={(e: any) => props.onChange(e.target.value)}

diff --git a/...end/src/explore/components/controls/DateFilterControl/components/CurrentCalendarFrame.tsx b/...end/src/explore/components/controls/DateFilterControl/components/CurrentCalendarFrame.tsx
@@ -42,7 +42,12 @@ export function CurrentCalendarFrame({ onChange, value }: FrameComponentProps) {
         {t('Configure Time Range: Current...')}
       </div>
       <Radio.GroupWrapper
-        spaceConfig={{ direction: 'vertical', size: 15 }}
+        spaceConfig={{
+          direction: 'vertical',
+          size: 15,
+          align: 'center',
+          wrap: false,
+        }}
         size="large"
         onChange={(e: any) => {
           let newValue = e.target.value;

diff --git a/...set-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx b/...set-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx
@@ -239,17 +239,14 @@ export function CustomFrame(props: FrameComponentProps) {
             <Row align="middle">
               <Col>
                 <Radio.GroupWrapper
+                  options={[
+                    { value: 'now', label: t('Now') },
+                    { value: 'specific', label: t('Date/Time') },
+                  ]}
                   onChange={onAnchorMode}
                   defaultValue="now"
                   value={anchorMode}
-                >
-                  <Radio key="now" value="now">
-                    {t('NOW')}
-                  </Radio>
-                  <Radio key="specific" value="specific">
-                    {t('Date/Time')}
-                  </Radio>
-                </Radio.GroupWrapper>
+                />
               </Col>
               {anchorMode !== 'now' && (
                 <Col>

diff --git a/superset-frontend/src/features/reports/ReportModal/index.tsx b/superset-frontend/src/features/reports/ReportModal/index.tsx
@@ -256,24 +256,31 @@ function ReportModal({
       </StyledMessageContentTitle>
       <div className="inline-container">
         <Radio.GroupWrapper
-          spaceConfig={{ direction: 'vertical', size: 'middle' }}
+          spaceConfig={{
+            direction: 'vertical',
+            size: 'middle',
+            align: 'center',
+            wrap: false,
+          }}
           onChange={(event: RadioChangeEvent) => {
             setCurrentReport({ report_format: event.target.value });
           }}
           value={currentReport.report_format || defaultNotificationFormat}
-        >
-          {isTextBasedChart && (
-            <Radio value={NotificationFormats.Text}>
-              {t('Text embedded in email')}
-            </Radio>
-          )}
-          <Radio value={NotificationFormats.PNG}>
-            {t('Image (PNG) embedded in email')}
-          </Radio>
-          <Radio value={NotificationFormats.CSV}>
-            {t('Formatted CSV attached in email')}
-          </Radio>
-        </Radio.GroupWrapper>
+          options={[
+            {
+              label: t('Text embedded in email'),
+              value: NotificationFormats.Text,
+            },
+            {
+              label: t('Image (PNG) embedded in email'),
+              value: NotificationFormats.PNG,
+            },
+            {
+              label: t('Formatted CSV attached in email'),
+              value: NotificationFormats.CSV,
+            },
+          ]}
+        />
       </div>
     </>
   );