-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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(ProgressBar): Upgrade ProgressBar to Antd 5 #29666
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -64,6 +64,11 @@ const baseConfig: ThemeConfig = { | |||
borderRadiusSM: supersetTheme.gridUnit / 2, | |||
defaultBg: supersetTheme.colors.grayscale.light4, | |||
}, | |||
Progress: { | |||
fontSize: supersetTheme.typography.sizes.s, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geido Is it possible to specify these at the component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a decision we made to keep the style centralized rather than scattered across. The component tokens are relevant to general theme, as the general theme might need to be tweaked if we realize that multiple components are using the same styles and thus should be promoted. We can reconsider this choice once the big part of the transition is done and the general theme should be stable.
@supersetbot label 4.1 |
(cherry picked from commit 3de2b7c)
SUMMARY
Please refer to the Superset Improvement Proposal #29268
BEFORE
AFTER
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION