refactor: Upgrade Badge component to Ant Design 5 #29124
Conversation
superset-frontend/package.json
Outdated
| "antd": "4.10.3", | ||
| "antd-v5": "npm:antd@^5.18.0", |
There was a problem hiding this comment.
The previous and target versions of Ant Design can be supported by using different names in the package configuration.
| @@ -22,7 +22,6 @@ import Badge from '.'; | |||
| const mockedProps = { | |||
| count: 9, | |||
| text: 'Text', | |||
| textColor: 'orange', | |||
There was a problem hiding this 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.
| & > sup.ant-badge-count { | ||
| ${ | ||
| count !== undefined | ||
| ? `background: ${color || theme.colors.primary.base};` |
There was a problem hiding this 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.
| margin-left: ${theme.gridUnit * 2}px; | ||
| &>sup { | ||
| padding: 0 ${theme.gridUnit}px; | ||
| &>sup.ant-badge-count { |
There was a problem hiding this 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.
| @@ -35,35 +36,48 @@ const { common } = getBootstrapData(); | |||
|
|
|||
| const extensionsRegistry = getExtensionsRegistry(); | |||
|
|
|||
| const antdTheme = { | |||
There was a problem hiding this comment.
Mapping of the Ant Design theme with Superset theme
| @@ -35,35 +36,48 @@ const { common } = getBootstrapData(); | |||
|
|
|||
| const extensionsRegistry = getExtensionsRegistry(); | |||
|
|
|||
| const antdTheme = { | |||
| components: { | |||
| Badge: { | |||
There was a problem hiding this comment.
Some GPT inputs that may help when defining the pattern for global vs component properties.
geido
force-pushed
the
geido/feat/antdesign-5
branch
from
55a7455
to
91582f3
Compare
geido
changed the title
geido
requested review from
jinghua-qa,
eschutho,
rusackas and
betodealmeida
as code owners
geido
force-pushed
the
geido/feat/antdesign-5
branch
from
9445618
to
26a4035
Compare
Co-authored-by: Evan Rusackas <[email protected]>
SUMMARY
This was a Proof of Concept (POC) aiming to demonstrate the capability to upgrade Ant Design components to the latest version in isolation, facilitating incremental upgrades.
Two versions of Ant Design will be maintained until all components are upgraded to the latest version.
Please refer to the inline comments in this PR and the Superset Improvement Proposal #29268
This PR is dependent on PR #29328
Promoting this PR to a mergiable state.
BEFORE
AFTER
Note that the prop
textColorhas been removed as it was unused.TESTING INSTRUCTIONS
ADDITIONAL INFORMATION