Re: [PR] fix(theming): Fix visual regressions from theming P4 [superset]

[via GitHub]

msyavuz commented on code in PR #33703:
URL: https://github.com/apache/superset/pull/33703#discussion_r2137767148


##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx:
##########
@@ -76,6 +76,7 @@ const StyledModalWrapper = styled(StyledModal)<{ expanded: 
boolean }>`
 
   .ant-modal-body {
     padding: 0px;
+    overflow: unset;

Review Comment:
   This causes an issue with filter modal where in smaller screens buttons are 
overflowing, similar issue might exist in other places where `overflow: unset` 
is used:
   
![image](https://github.com/user-attachments/assets/2e8cef95-590a-4f07-b19d-f6550540e630)
   



##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -1271,9 +1291,6 @@ class DatasourceEditor extends PureComponent {
                         </Button>
                       </div>
                     }
-                    errorMessage={
-                      this.props.database?.error && 
this.renderSqlErrorMessage()

Review Comment:
   Eslint should catch this but renderSqlErrorMessage is unused now. Also there 
is some additional stuff rendered in that function that is lost with this. 
Maybe we should modify that instead?



##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -1245,7 +1265,7 @@ class DatasourceEditor extends PureComponent {
                           <Icons.ExportOutlined
                             iconSize="s"
                             css={theme => ({
-                              color: theme.colors.primary.dark1,
+                              color: theme.colors.primary.light5,

Review Comment:
   Can we use first level tokens instead?



##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterTitlePane.tsx:
##########
@@ -99,7 +99,7 @@ const FilterTitlePane: FC<Props> = ({
           alignItems: 'flex-start',
           paddingTop: theme.sizeUnit * 3,
           position: 'sticky',
-          bottom: theme.sizeUnit,
+          marginBottom: theme.sizeUnit * 3,

Review Comment:
   Is this intended?



-- 
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: notifications-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org