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: Upgrade Badge component to Ant Design 5 #29124

Merged
merged 10 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ describe('Horizontal FilterBar', () => {
applyNativeFilterValueWithIndex(8, testItems.filterDefaultValue);
cy.get(nativeFilters.applyFilter).click({ force: true });
cy.wait('@chart');
cy.get('.ant-scroll-number.ant-badge-count').should(
cy.get('.antd5-scroll-number.antd5-badge-count').should(
'have.attr',
'title',
'1',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const profile = {
favoritesSpace: '#rc-tabs-0-panel-2',
};
export const securityAccess = {
rolesBubble: '.ant-badge-count',
rolesBubble: '.antd5-badge-count',
};
export const homePage = {
homeSection: {
Expand Down
74 changes: 33 additions & 41 deletions superset-frontend/src/components/Badge/Badge.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { AntdThemeProvider } from 'src/components/AntdThemeProvider';
import Badge, { BadgeProps } from '.';

export default {
Expand Down Expand Up @@ -58,15 +59,20 @@ const SIZES = {
defaultValue: undefined,
};

export const InteractiveBadge = (args: BadgeProps) => <Badge {...args} />;
export const InteractiveBadge = (args: BadgeProps) => (
<AntdThemeProvider>
<Badge {...args} />
</AntdThemeProvider>
);

InteractiveBadge.args = {
count: null,
color: null,
count: undefined,
color: undefined,
text: 'Text',
textColor: null,
status: 'success',
size: 'default',
showZero: false,
overflowCount: 99,
};

InteractiveBadge.argTypes = {
Expand All @@ -75,6 +81,8 @@ InteractiveBadge.argTypes = {
type: 'select',
},
options: [undefined, ...STATUSES],
description:
'only works if `count` is `undefined` (or is set to 0) and `color` is set to `undefined`',
},
size: {
control: {
Expand All @@ -88,17 +96,22 @@ InteractiveBadge.argTypes = {
},
options: [undefined, ...COLORS.options],
},
textColor: {
control: {
type: 'select',
},
options: [undefined, ...COLORS.options],
},
count: {
control: {
type: 'select',
defaultValue: undefined,
},
options: [undefined, ...Array(100).keys()],
defaultValue: undefined,
},
showZero: {
control: 'boolean',
defaultValue: false,
},
overflowCount: {
control: 'number',
description:
'The threshold at which the number overflows with a `+` e.g if you set this to 10, and the value is 11, you get `11+`',
},
};

Expand All @@ -107,33 +120,21 @@ export const BadgeGallery = () => (
{SIZES.options.map(size => (
<div key={size} style={{ marginBottom: 40 }}>
<h4>{size}</h4>
{COLORS.options.map(color => (
<Badge
count={9}
textColor={color}
size={size}
key={`${color}_${size}`}
style={{ marginRight: '15px' }}
/>
))}
<AntdThemeProvider>
{COLORS.options.map(color => (
<Badge
count={9}
size={size}
key={`${color}_${size}`}
style={{ marginRight: '15px' }}
/>
))}
</AntdThemeProvider>
</div>
))}
</>
);

export const BadgeTextGallery = () => (
<>
{COLORS.options.map(color => (
<Badge
text="Hello"
color={color}
key={color}
style={{ marginRight: '15px' }}
/>
))}
</>
);

BadgeGallery.parameters = {
actions: {
disable: true,
Expand All @@ -142,12 +143,3 @@ BadgeGallery.parameters = {
disable: true,
},
};

BadgeTextGallery.parameters = {
actions: {
disable: true,
},
controls: {
disable: true,
},
};
9 changes: 0 additions & 9 deletions superset-frontend/src/components/Badge/Badge.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import Badge from '.';
const mockedProps = {
count: 9,
text: 'Text',
textColor: 'orange',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property is custom. Since it is not used in the code, it does not make sense to maintain it. Additionally, one of the goals of the proposal is to minimize the introduction of custom properties to reduce maintenance costs.

};

test('should render', () => {
Expand All @@ -39,11 +38,3 @@ test('should render the text', () => {
render(<Badge {...mockedProps} />);
expect(screen.getByText('Text')).toBeInTheDocument();
});

test('should render with the chosen textColor', () => {
render(<Badge {...mockedProps} />);
const badge = screen.getAllByText('9')[0];
expect(badge).toHaveStyle(`
color: 'orange';
`);
});
31 changes: 14 additions & 17 deletions superset-frontend/src/components/Badge/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,22 @@
* under the License.
*/
import { styled } from '@superset-ui/core';
import { Badge as AntdBadge } from 'antd';
import { BadgeProps as AntdBadgeProps } from 'antd/lib/badge';
import { Badge as AntdBadge } from 'antd-v5';
import { BadgeProps as AntdBadgeProps } from 'antd-v5/lib/badge';

export interface BadgeProps extends AntdBadgeProps {
textColor?: string;
}
export type { AntdBadgeProps as BadgeProps };

const Badge = styled(
(
// eslint-disable-next-line @typescript-eslint/no-unused-vars
{ textColor, color, text, ...props }: BadgeProps,
) => <AntdBadge text={text} color={text ? color : undefined} {...props} />,
)`
& > sup {
padding: 0 ${({ theme }) => theme.gridUnit * 2}px;
background: ${({ theme, color }) => color || theme.colors.primary.base};
color: ${({ theme, textColor }) =>
textColor || theme.colors.grayscale.light5};
}
const Badge = styled((props: AntdBadgeProps) => <AntdBadge {...props} />)`
${({ theme, color, count }) => `
& > sup,
& > sup.antd5-badge-count {
${
count !== undefined
? `background: ${color || theme.colors.primary.base};`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of a necessary customization for backward compatibility. Ant Design uses colorError as the background color for the count component of the Badge. However, in Superset, the count requires a different color. Changing the colorError of the Badge in the Ant Design ConfigProvider would affect all variations of the Badge, which is not desirable. See all variations here: https://ant.design/components/badge.

: ''
}
}
`}
`;

export default Badge;
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ const StyledTableTabs = styled(Tabs)`
`;

const StyledBadge = styled(Badge)`
.ant-badge-count {
.antd5-badge-count {
line-height: ${({ theme }) => theme.gridUnit * 4}px;
height: ${({ theme }) => theme.gridUnit * 4}px;
margin-left: ${({ theme }) => theme.gridUnit}px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,16 @@ const StyledFilterCount = styled.div`

const StyledBadge = styled(Badge)`
${({ theme }) => `
vertical-align: middle;
margin-left: ${theme.gridUnit * 2}px;
&>sup {
&>sup.antd5-badge-count {
padding: 0 ${theme.gridUnit}px;
min-width: ${theme.gridUnit * 4}px;
height: ${theme.gridUnit * 4}px;
line-height: 1.5;
font-weight: ${theme.typography.weights.medium};
font-size: ${theme.typography.sizes.s - 1}px;
box-shadow: none;
padding: 0 ${theme.gridUnit}px;
}
`}
`;
Expand Down
Loading