geido commented on code in PR #31420:
URL: https://github.com/apache/superset/pull/31420#discussion_r1888938608
##########
superset-frontend/src/features/databases/UploadDataModel/styles.ts:
##########
@@ -76,21 +76,21 @@ export const formStyles = (theme: SupersetTheme) => css`
`;
export const antDModalStyles = (theme: SupersetTheme) => css`
- .ant-modal-header {
+ .antd5-modal-header {
padding: ${theme.gridUnit * 4.5}px ${theme.gridUnit * 4}px
${theme.gridUnit * 4}px;
}
- .ant-modal-close-x .close {
+ .antd5-modal-close-x .close {
color: ${theme.colors.grayscale.dark1};
Review Comment:
Same question about centralizing this
##########
superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx:
##########
@@ -70,7 +70,7 @@ const ErrorModal = styled(Modal)<{ level: ErrorLevel }>`
color: ${({ level, theme }) => theme.colors[level].dark2};
overflow-wrap: break-word;
- .ant-modal-header {
+ .antd5-modal-header {
background-color: ${({ level, theme }) => theme.colors[level].light2};
Review Comment:
Can this be centralized / removed / or moved to theme level?
##########
superset-frontend/src/theme/index.ts:
##########
@@ -96,6 +96,13 @@ const baseConfig: ThemeConfig = {
colorSplit: supersetTheme.colors.grayscale.light3,
colorText: supersetTheme.colors.grayscale.dark1,
},
+ Modal: {
+ colorBgMask: `${supersetTheme.colors.grayscale.dark2}73`,
Review Comment:
What is that `73` for?
##########
superset-frontend/src/theme/index.ts:
##########
@@ -96,6 +96,13 @@ const baseConfig: ThemeConfig = {
colorSplit: supersetTheme.colors.grayscale.light3,
colorText: supersetTheme.colors.grayscale.dark1,
},
+ Modal: {
+ colorBgMask: `${supersetTheme.colors.grayscale.dark2}73`,
+ contentBg: supersetTheme.colors.grayscale.light5,
+ titleFontSize: supersetTheme.gridUnit * 4,
+ titleColor: `${supersetTheme.colors.grayscale.dark2}D9`,
Review Comment:
What is that `D9` for?
##########
superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx:
##########
@@ -70,7 +70,7 @@ const ErrorModal = styled(Modal)<{ level: ErrorLevel }>`
color: ${({ level, theme }) => theme.colors[level].dark2};
Review Comment:
Can this be centralized / removed / or moved to theme level?
##########
superset-frontend/src/components/Modal/Modal.tsx:
##########
@@ -106,30 +111,41 @@ export const StyledModal =
styled(BaseModal)<StyledModalProps>`
top: 0;
`}
- .ant-modal-content {
+ .antd5-modal-content {
display: flex;
flex-direction: column;
max-height: ${({ theme }) => `calc(100vh - ${theme.gridUnit * 8}px)`};
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
margin-top: ${({ theme }) => theme.gridUnit * 4}px;
+ padding: 0;
}
- .ant-modal-header {
+ .antd5-modal-header {
flex: 0 0 auto;
- background-color: ${({ theme }) => theme.colors.grayscale.light4};
border-radius: ${({ theme }) => theme.borderRadius}px
Review Comment:
Can this be centralized / removed / or moved to theme level?
##########
superset-frontend/src/features/databases/DatabaseModal/styles.ts:
##########
@@ -146,21 +146,21 @@ export const antDModalStyles = (theme: SupersetTheme) =>
css`
height: ${theme.gridUnit * 40}px;
}
- .ant-modal-header {
+ .antd5-modal-header {
padding: ${theme.gridUnit * 4.5}px ${theme.gridUnit * 4}px
${theme.gridUnit * 4}px;
}
- .ant-modal-close-x .close {
+ .antd5-modal-close-x .close {
color: ${theme.colors.grayscale.dark1};
Review Comment:
Can we centralize this, is this the same everywhere?
##########
superset-frontend/src/components/Modal/Modal.tsx:
##########
@@ -106,30 +111,41 @@ export const StyledModal =
styled(BaseModal)<StyledModalProps>`
top: 0;
`}
- .ant-modal-content {
+ .antd5-modal-content {
display: flex;
flex-direction: column;
max-height: ${({ theme }) => `calc(100vh - ${theme.gridUnit * 8}px)`};
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
margin-top: ${({ theme }) => theme.gridUnit * 4}px;
+ padding: 0;
}
- .ant-modal-header {
+ .antd5-modal-header {
flex: 0 0 auto;
- background-color: ${({ theme }) => theme.colors.grayscale.light4};
border-radius: ${({ theme }) => theme.borderRadius}px
${({ theme }) => theme.borderRadius}px 0 0;
- padding-left: ${({ theme }) => theme.gridUnit * 4}px;
- padding-right: ${({ theme }) => theme.gridUnit * 4}px;
+ padding: ${({ theme }) => theme.gridUnit * 4}px
+ ${({ theme }) => theme.gridUnit * 6}px;
+
+ .antd5-modal-title {
+ font-weight: ${({ theme }) => theme.typography.weights.medium};
Review Comment:
Can this be centralized / removed / or moved to theme level?
--
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]
