geido commented on code in PR #29124:
URL: https://github.com/apache/superset/pull/29124#discussion_r1632355278
##########
superset-frontend/src/components/Badge/Badge.test.tsx:
##########
@@ -22,7 +22,6 @@ import Badge from '.';
const mockedProps = {
count: 9,
text: 'Text',
- textColor: 'orange',
Review Comment:
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.
##########
superset-frontend/src/components/Badge/index.tsx:
##########
@@ -28,14 +28,25 @@ 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} />,
+ ) => (
+ <AntdBadge
+ text={text}
+ color={text ? color : undefined}
+ status={props.status || 'default'}
+ {...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};
- }
+ ${({ theme, color, count }) => `
+ & > sup,
+ & > sup.ant-badge-count {
+ ${
+ count !== undefined
+ ? `background: ${color || theme.colors.primary.base};`
Review Comment:
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.
##########
superset-frontend/src/dashboard/components/FiltersBadge/index.tsx:
##########
@@ -84,16 +84,15 @@ const StyledFilterCount = styled.div`
const StyledBadge = styled(Badge)`
${({ theme }) => `
- vertical-align: middle;
margin-left: ${theme.gridUnit * 2}px;
- &>sup {
- padding: 0 ${theme.gridUnit}px;
+ &>sup.ant-badge-count {
Review Comment:
This is another example of a forced customization to meet Superset’s
requirements to fit the Badge within the available space. Such customizations
are undesirable as they introduce maintenance costs. Another issue to consider
is the reliance on a specific className, which poses risks if this className is
removed or changed in future versions of Ant Design.
##########
superset-frontend/package.json:
##########
@@ -121,6 +121,7 @@
"abortcontroller-polyfill": "^1.1.9",
"ace-builds": "^1.4.14",
"antd": "4.10.3",
+ "antd-v5": "npm:antd@^5.18.0",
Review Comment:
The previous and target versions of Ant Design can be supported by using
different names in the package configuration.
##########
superset-frontend/src/views/RootContextProviders.tsx:
##########
@@ -35,35 +36,48 @@ const { common } = getBootstrapData();
const extensionsRegistry = getExtensionsRegistry();
+const antdTheme = {
Review Comment:
Mapping of the Ant Design theme with Superset theme
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
