Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(Radio): Upgrade Radio Component to Ant Design 5 #32004

Merged
merged 18 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion superset-frontend/cypress-base/cypress/support/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ Cypress.Commands.add('login', () => {
}).then(response => {
if (response.status === 302) {
// If there's a redirect, follow it manually
const redirectUrl = response.headers['location'];
const redirectUrl = response.headers.location;
cy.request({
method: 'GET',
url: redirectUrl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,49 +19,38 @@

import { useMemo, useState } from 'react';
import { css, SupersetTheme, t } from '@superset-ui/core';
import { Radio } from 'src/components/Radio';
import { Radio, CheckboxOptionType } from 'src/components/Radio';
import { DrillByType } from '../types';

export const useDisplayModeToggle = () => {
const [drillByDisplayMode, setDrillByDisplayMode] = useState<DrillByType>(
DrillByType.Chart,
);

const customButtons: CheckboxOptionType[] = [
EnxDev marked this conversation as resolved.
Show resolved Hide resolved
{ label: t('Chart'), value: DrillByType.Chart },
{ label: t('Table'), value: DrillByType.Table },
];
const displayModeToggle = useMemo(
() => (
<div
css={(theme: SupersetTheme) => css`
margin-bottom: ${theme.gridUnit * 6}px;
.ant-radio-button-wrapper-checked:not(
.ant-radio-button-wrapper-disabled
):focus-within {
box-shadow: none;
}
`}
data-test="drill-by-display-toggle"
>
<Radio.Group
<Radio.GroupWrapper
onChange={({ target: { value } }) => {
setDrillByDisplayMode(value);
}}
defaultValue={DrillByType.Chart}
>
<Radio.Button
value={DrillByType.Chart}
data-test="drill-by-chart-radio"
>
{t('Chart')}
</Radio.Button>
<Radio.Button
value={DrillByType.Table}
data-test="drill-by-table-radio"
>
{t('Table')}
</Radio.Button>
</Radio.Group>
options={customButtons}
value={drillByDisplayMode}
optionType="button"
buttonStyle="outline"
/>
</div>
),
[],
[drillByDisplayMode],
EnxDev marked this conversation as resolved.
Show resolved Hide resolved
);
return { displayModeToggle, drillByDisplayMode };
};
90 changes: 52 additions & 38 deletions superset-frontend/src/components/Radio/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,46 +16,60 @@
* specific language governing permissions and limitations
* under the License.
*/
import { styled } from '@superset-ui/core';
import { Radio as AntdRadio } from 'antd';
import { Radio as Antd5Radio } from 'antd-v5';
import type {
RadioChangeEvent,
RadioProps,
RadioGroupProps,
CheckboxOptionType,
} from 'antd-v5';
import { Space, SpaceProps } from 'src/components/Space';
EnxDev marked this conversation as resolved.
Show resolved Hide resolved

const StyledRadio = styled(AntdRadio)`
.ant-radio-inner {
top: -1px;
left: 2px;
width: ${({ theme }) => theme.gridUnit * 4}px;
height: ${({ theme }) => theme.gridUnit * 4}px;
border-width: 2px;
border-color: ${({ theme }) => theme.colors.grayscale.light2};
}
export type RadioGroupWrapperProps = RadioGroupProps & {
useSpace?: boolean;
direction?: SpaceProps['direction'];
spaceSize?: SpaceProps['size'];
EnxDev marked this conversation as resolved.
Show resolved Hide resolved
align?: SpaceProps['align'];
options?: CheckboxOptionType[];
children?: React.ReactNode;
};

.ant-radio.ant-radio-checked {
.ant-radio-inner {
border-width: ${({ theme }) => theme.gridUnit + 1}px;
border-color: ${({ theme }) => theme.colors.primary.base};
}
const RadioGroup = ({
useSpace,
direction,
spaceSize,
options,
children,
...props
}: RadioGroupWrapperProps) => {
const content = options
? options.map((option: CheckboxOptionType) => (
<Radio key={option.value} value={option.value}>
{option.label}
</Radio>
))
: children;
EnxDev marked this conversation as resolved.
Show resolved Hide resolved

.ant-radio-inner::after {
background-color: ${({ theme }) => theme.colors.grayscale.light5};
top: 0;
left: 0;
width: ${({ theme }) => theme.gridUnit + 2}px;
height: ${({ theme }) => theme.gridUnit + 2}px;
}
}
return (
<Radio.Group {...props}>
{useSpace ? (
<Space direction={direction} size={spaceSize}>
{content}
</Space>
) : (
content
)}
</Radio.Group>
);
};

.ant-radio:hover,
.ant-radio:focus {
.ant-radio-inner {
border-color: ${({ theme }) => theme.colors.primary.dark1};
}
}
`;
const StyledGroup = styled(AntdRadio.Group)`
font-size: inherit;
`;

export const Radio = Object.assign(StyledRadio, {
Group: StyledGroup,
Button: AntdRadio.Button,
export type {
RadioChangeEvent,
RadioGroupProps,
RadioProps,
CheckboxOptionType,
};
export const Radio = Object.assign(Antd5Radio, {
GroupWrapper: RadioGroup,
Button: Antd5Radio.Button,
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import { useState } from 'react';
import { css, useTheme } from '@superset-ui/core';
import { Radio } from 'src/components/Radio';
import { Space } from 'src/components/Space';
import Icons from 'src/components/Icons';
import Popover from 'src/components/Popover';

Expand Down Expand Up @@ -56,21 +55,17 @@ function HeaderWithRadioGroup(props: HeaderWithRadioGroupProps) {
>
{groupTitle}
</div>
<Radio.Group
<Radio.GroupWrapper
useSpace
direction="vertical"
spaceSize={4}
value={value}
onChange={e => {
onChange(e.target.value);
setPopoverVisible(false);
}}
>
<Space direction="vertical">
{groupOptions.map(option => (
<Radio key={option.value} value={option.value}>
{option.label}
</Radio>
))}
</Space>
</Radio.Group>
options={groupOptions}
/>
</div>
}
placement="bottomLeft"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1106,15 +1106,15 @@ const FiltersConfigForm = (
initialValue={sort}
label={<StyledLabel>{t('Sort type')}</StyledLabel>}
>
<Radio.Group
<Radio.GroupWrapper
onChange={value => {
onSortChanged(value.target.value);
formChanged();
}}
>
<Radio value>{t('Sort ascending')}</Radio>
<Radio value={false}>{t('Sort descending')}</Radio>
</Radio.Group>
</Radio.GroupWrapper>
</StyledRowFormItem>
{hasMetrics && (
<StyledRowSubFormItem
Expand Down Expand Up @@ -1181,7 +1181,7 @@ const FiltersConfigForm = (
<StyledLabel>{t('Single value type')}</StyledLabel>
}
>
<Radio.Group
<Radio.GroupWrapper
onChange={value => {
onEnableSingleValueChanged(value.target.value);
formChanged();
Expand All @@ -1196,7 +1196,7 @@ const FiltersConfigForm = (
<Radio value={SingleValueType.Maximum}>
{t('Maximum')}
</Radio>
</Radio.Group>
</Radio.GroupWrapper>
</StyledRowFormItem>
</CollapsibleControl>
</CleanFormItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
import { Global } from '@emotion/react';
import { Column } from 'react-table';
import { debounce } from 'lodash';
import { Space } from 'src/components/Space';
import { Input } from 'src/components/Input';
import {
BOOL_FALSE_DISPLAY,
Expand Down Expand Up @@ -141,12 +140,15 @@ const FormatPicker = ({
onChange: any;
value: FormatPickerValue;
}) => (
<Radio.Group value={value} onChange={onChange}>
<Space direction="vertical">
<Radio value={FormatPickerValue.Formatted}>{t('Formatted date')}</Radio>
<Radio value={FormatPickerValue.Original}>{t('Original value')}</Radio>
</Space>
</Radio.Group>
<Radio.GroupWrapper
useSpace
direction="vertical"
value={value}
onChange={onChange}
>
<Radio value={FormatPickerValue.Formatted}>{t('Formatted date')}</Radio>
<Radio value={FormatPickerValue.Original}>{t('Original value')}</Radio>
</Radio.GroupWrapper>
);

const FormatPickerContainer = styled.div`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,6 @@ const ContentStyleWrapper = styled.div`
margin: 8px 0;
}

.vertical-radio {
display: block;
height: 40px;
line-height: 40px;
}

.section-title {
font-style: normal;
font-weight: ${theme.typography.weights.bold};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,15 @@ export function CalendarFrame({ onChange, value }: FrameComponentProps) {
<div className="section-title">
{t('Configure Time Range: Previous...')}
</div>
<Radio.Group
<Radio.GroupWrapper
useSpace
size="large"
direction="vertical"
spaceSize={15}
value={value}
onChange={(e: any) => onChange(e.target.value)}
>
{CALENDAR_RANGE_OPTIONS.map(({ value, label }) => (
<Radio key={value} value={value} className="vertical-radio">
{label}
</Radio>
))}
</Radio.Group>
options={CALENDAR_RANGE_OPTIONS}
/>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,15 @@ export function CommonFrame(props: FrameComponentProps) {
<div className="section-title" data-test={DateFilterTestKey.CommonFrame}>
{t('Configure Time Range: Last...')}
</div>
<Radio.Group
<Radio.GroupWrapper
useSpace
size="large"
direction="vertical"
spaceSize={15}
value={commonRange}
onChange={(e: any) => props.onChange(e.target.value)}
>
{COMMON_RANGE_OPTIONS.map(({ value, label }) => (
<Radio key={value} value={value} className="vertical-radio">
{label}
</Radio>
))}
</Radio.Group>
options={COMMON_RANGE_OPTIONS}
/>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,19 @@ export function CurrentCalendarFrame({ onChange, value }: FrameComponentProps) {
<div className="section-title">
{t('Configure Time Range: Current...')}
</div>
<Radio.Group
value={value}
<Radio.GroupWrapper
useSpace
size="large"
direction="vertical"
spaceSize={15}
onChange={(e: any) => {
let newValue = e.target.value;
// Sanitization: Trim whitespace
newValue = newValue.trim();
// Validation: Check if the value is non-empty
if (newValue === '') {
return;
}
if (newValue === '') return;
onChange(newValue);
}}
>
{CURRENT_RANGE_OPTIONS.map(({ value, label }) => (
<Radio key={value} value={value} className="vertical-radio">
{label}
</Radio>
))}
</Radio.Group>
options={CURRENT_RANGE_OPTIONS}
/>
</>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export function CustomFrame(props: FrameComponentProps) {
<div className="control-label">{t('Anchor to')}</div>
<Row align="middle">
<Col>
<Radio.Group
<Radio.GroupWrapper
EnxDev marked this conversation as resolved.
Show resolved Hide resolved
onChange={onAnchorMode}
defaultValue="now"
value={anchorMode}
Expand All @@ -249,7 +249,7 @@ export function CustomFrame(props: FrameComponentProps) {
<Radio key="specific" value="specific">
{t('Date/Time')}
</Radio>
</Radio.Group>
</Radio.GroupWrapper>
</Col>
{anchorMode !== 'now' && (
<Col>
Expand Down
Loading
Loading