fix(theming): Theming visual fixes (#34253)

superset-frontend/packages/superset-ui-core/src/components/AsyncAceEditor/index.tsx

@@ -202,7 +202,7 @@ export function AsyncAceEditor(
 
                 /* Basic editor styles with dark mode support */
                 .ace_editor.ace-github,
-                .ace_editor.ace-textmate {
+                .ace_editor.ace-tm {
                   background-color: ${token.colorBgContainer} !important;
                   color: ${token.colorText} !important;
                 }

superset-frontend/packages/superset-ui-core/src/components/Button/index.tsx

@@ -70,7 +70,6 @@ export function Button(props: ButtonProps) {
   if (!buttonStyle || buttonStyle === 'primary') {
     variant = 'solid';
     antdType = 'primary';
-    color = 'primary';
   } else if (buttonStyle === 'secondary') {
     variant = 'filled';
     color = 'primary';
@@ -78,7 +77,6 @@ export function Button(props: ButtonProps) {
     variant = 'outlined';
     color = 'default';
   } else if (buttonStyle === 'dashed') {
-    color = 'primary';
     variant = 'dashed';
     antdType = 'dashed';
   } else if (buttonStyle === 'danger') {
@@ -134,6 +132,11 @@ export function Button(props: ButtonProps) {
         '& > span > :first-of-type': {
           marginRight: firstChildMargin,
         },
+        ':not(:hover)': effectiveButtonStyle === 'secondary' && {
+          // NOTE: This is the best we can do contrast wise for the secondary button using antd tokens
+          // and abusing the semantics. Should be revisited when possible. https://github.com/apache/superset/pull/34253#issuecomment-3104834692
+          color: `${theme.colorPrimaryTextHover} !important`,
+        },
       }}
       icon={icon}
       {...restProps}

superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx

@@ -31,11 +31,6 @@ const StyledDiv = styled.div`
   }
 `;
 
-const DescriptionContainer = styled.div`
-  line-height: ${({ theme }) => theme.sizeUnit * 4}px;
-  padding-top: 16px;
-`;
-
 export function DeleteModal({
   description,
   onConfirm,
@@ -81,12 +76,12 @@ export function DeleteModal({
       onHide={hide}
       onHandledPrimaryAction={confirm}
       primaryButtonName={t('Delete')}
-      primaryButtonType="danger"
+      primaryButtonStyle="danger"
       show={open}
       title={title}
       centered
     >
-      <DescriptionContainer>{description}</DescriptionContainer>
+      {description}
       <StyledDiv>
         <FormLabel htmlFor="delete">
           {t('Type "%s" to confirm', t('DELETE'))}

superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.stories.tsx

@@ -33,7 +33,7 @@ export const InteractiveModal = (props: ModalProps) => (
 InteractiveModal.args = {
   disablePrimaryButton: false,
   primaryButtonName: 'Danger',
-  primaryButtonType: 'danger',
+  primaryButtonStyle: 'danger',
   show: true,
   title: "I'm a modal!",
   resizable: false,

superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx

@@ -77,7 +77,7 @@ export const StyledModal = styled(BaseModal)<StyledModalProps>`
     .ant-modal-header {
       flex: 0 0 auto;
       border-radius: ${theme.borderRadius}px ${theme.borderRadius}px 0 0;
-      padding: ${theme.sizeUnit * 4}px ${theme.sizeUnit * 6}px;
+      padding: ${theme.sizeUnit * 4}px ${theme.sizeUnit * 4}px;
 
       .ant-modal-title {
         font-weight: ${theme.fontWeightStrong};
@@ -122,6 +122,7 @@ export const StyledModal = styled(BaseModal)<StyledModalProps>`
     .ant-modal-body {
       flex: 0 1 auto;
       padding: ${theme.sizeUnit * 4}px;
+      padding-bottom: ${theme.sizeUnit * 2}px;
       overflow: auto;
       ${!resizable && height && `height: ${height};`}
     }
@@ -208,7 +209,7 @@ const CustomModal = ({
   onHide,
   onHandledPrimaryAction,
   primaryButtonName = t('OK'),
-  primaryButtonType = 'primary',
+  primaryButtonStyle = 'primary',
   show,
   name,
   title,
@@ -261,7 +262,7 @@ const CustomModal = ({
         </Button>,
         <Button
           key="submit"
-          buttonStyle={primaryButtonType}
+          buttonStyle={primaryButtonStyle}
           disabled={disablePrimaryButton}
           tooltip={primaryTooltipMessage}
           loading={primaryButtonLoading}

superset-frontend/packages/superset-ui-core/src/components/Modal/types.ts

@@ -20,6 +20,7 @@ import type { CSSProperties, ReactNode } from 'react';
 import type { ModalFuncProps } from 'antd';
 import type { ResizableProps } from 're-resizable';
 import type { DraggableProps } from 'react-draggable';
+import { ButtonStyle } from '../Button/types';
 
 export interface ModalProps {
   className?: string;
@@ -30,7 +31,7 @@ export interface ModalProps {
   onHide: () => void;
   onHandledPrimaryAction?: () => void;
   primaryButtonName?: string;
-  primaryButtonType?: 'primary' | 'danger';
+  primaryButtonStyle?: ButtonStyle;
   show: boolean;
   name?: string;
   title: ReactNode;

superset-frontend/src/components/ImportModal/index.tsx

@@ -362,7 +362,7 @@ export const ImportModal: FunctionComponent<ImportModelsModalProps> = ({
       onHandledPrimaryAction={onUpload}
       onHide={hide}
       primaryButtonName={needsOverwriteConfirm ? t('Overwrite') : t('Import')}
-      primaryButtonType={needsOverwriteConfirm ? 'danger' : 'primary'}
+      primaryButtonStyle={needsOverwriteConfirm ? 'danger' : 'primary'}
       width="750px"
       show={show}
       title={<h4>{t('Import %s', resourceLabel)}</h4>}

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/ActionButtons/index.tsx

@@ -46,7 +46,7 @@ const containerStyle = (theme: SupersetTheme) => css`
   display: flex;
 
   && > .filter-clear-all-button {
-    color: ${theme.colors.grayscale.base};
+    color: ${theme.colorTextSecondary};
     margin-left: 0;
     &:hover {
       color: ${theme.colorPrimaryText};
@@ -54,7 +54,7 @@ const containerStyle = (theme: SupersetTheme) => css`
 
     &[disabled],
     &[disabled]:hover {
-      color: ${theme.colors.grayscale.light1};
+      color: ${theme.colorTextDisabled};
     }
   }
 `;
@@ -62,7 +62,6 @@ const containerStyle = (theme: SupersetTheme) => css`
 const verticalStyle = (theme: SupersetTheme, width: number) => css`
   flex-direction: column;
   align-items: center;
-  pointer-events: none;
   position: fixed;
   z-index: 100;
 
@@ -74,14 +73,10 @@ const verticalStyle = (theme: SupersetTheme, width: number) => css`
   padding-top: ${theme.sizeUnit * 6}px;
 
   background: linear-gradient(
-    ${rgba(theme.colors.grayscale.light5, 0)},
-    ${theme.colors.grayscale.light5} 60%
+    ${rgba(theme.colorBgLayout, 0)},
+    ${theme.colorBgElevated} 20%
   );
 
-  & > button {
-    pointer-events: auto;
-  }
-
   & > .filter-apply-button {
     margin-bottom: ${theme.sizeUnit * 3}px;
   }
@@ -94,13 +89,6 @@ const horizontalStyle = (theme: SupersetTheme) => css`
     text-transform: capitalize;
     font-weight: ${theme.fontWeightNormal};
   }
-  & > .filter-apply-button {
-    &[disabled],
-    &[disabled]:hover {
-      color: ${theme.colors.grayscale.light1};
-      background: ${theme.colors.grayscale.light3};
-    }
-  }
 `;
 
 const ButtonsContainer = styled.div<{ isVertical: boolean; width: number }>`

superset-frontend/src/explore/components/DatasourcePanel/DatasourcePanelItem.tsx

@@ -28,6 +28,7 @@ import {
 
 import { Icons } from '@superset-ui/core/components/Icons';
 import { Tooltip } from '@superset-ui/core/components/Tooltip';
+import { Typography } from '@superset-ui/core/components';
 import DatasourcePanelDragOption from './DatasourcePanelDragOption';
 import { DndItemType } from '../DndItemType';
 import { DndItemValue, FlattenedItem, Folder } from './types';
@@ -94,7 +95,7 @@ const SectionHeaderTextContainer = styled.div`
   width: 100%;
 `;
 
-const SectionHeader = styled.span`
+const SectionHeader = styled(Typography.Text)`
   ${({ theme }) => css`
     font-size: ${theme.fontSize}px;
     font-weight: ${theme.fontWeightStrong};

superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx

@@ -102,9 +102,9 @@ const getCommonElements = () => ({
   cancelButton: screen.getByRole('button', { name: 'Cancel' }),
   uploadButton: screen.getByRole('button', { name: 'Upload' }),
   selectButton: screen.getByRole('button', { name: 'Select' }),
-  panel1: screen.getByRole('heading', { name: /General information/i }),
-  panel2: screen.getByRole('heading', { name: /file settings/i }),
-  panel3: screen.getByRole('heading', { name: /columns/i }),
+  panel1: screen.getByText(/General information/i, { selector: 'strong' }),
+  panel2: screen.getByText(/file settings/i, { selector: 'strong' }),
+  panel3: screen.getByText(/columns/i, { selector: 'strong' }),
   selectDatabase: screen.getByRole('combobox', { name: /select a database/i }),
   inputTableName: screen.getByRole('textbox', { name: /table name/i }),
   inputSchema: screen.getByRole('combobox', { name: /schema/i }),
@@ -130,7 +130,7 @@ describe('UploadDataModal - General Information Elements', () => {
 
     const common = getCommonElements();
     const title = screen.getByRole('heading', { name: /csv upload/i });
-    const panel4 = screen.getByRole('heading', { name: /rows/i });
+    const panel4 = screen.getByText(/rows/i);
     const selectDelimiter = screen.getByRole('combobox', {
       name: /choose a delimiter/i,
     });
@@ -156,7 +156,7 @@ describe('UploadDataModal - General Information Elements', () => {
 
     const common = getCommonElements();
     const title = screen.getByRole('heading', { name: /excel upload/i });
-    const panel4 = screen.getByRole('heading', { name: /rows/i });
+    const panel4 = screen.getByText(/rows/i);
     const selectSheetName = screen.getByRole('combobox', {
       name: /choose sheet name/i,
     });
@@ -177,9 +177,7 @@ describe('UploadDataModal - General Information Elements', () => {
     ]);
 
     // Check elements that should NOT be visible
-    expect(
-      screen.queryByRole('heading', { name: /csv upload/i }),
-    ).not.toBeInTheDocument();
+    expect(screen.queryByText(/csv upload/i)).not.toBeInTheDocument();
     expect(
       screen.queryByRole('combobox', { name: /choose a delimiter/i }),
     ).not.toBeInTheDocument();
@@ -206,8 +204,8 @@ describe('UploadDataModal - General Information Elements', () => {
 
     // Check elements that should NOT be visible
     expectElementsNotVisible([
-      screen.queryByRole('heading', { name: /csv upload/i }),
-      screen.queryByRole('heading', { name: /rows/i }),
+      screen.queryByText(/csv upload/i),
+      screen.queryByText(/rows/i),
       screen.queryByRole('combobox', { name: /choose a delimiter/i }),
       screen.queryByRole('combobox', { name: /choose sheet name/i }),
     ]);
@@ -216,7 +214,7 @@ describe('UploadDataModal - General Information Elements', () => {
 
 describe('UploadDataModal - File Settings Elements', () => {
   const openFileSettings = async () => {
-    const panelHeader = screen.getByRole('heading', { name: /file settings/i });
+    const panelHeader = screen.getByText(/file settings/i);
     await userEvent.click(panelHeader);
   };
 
@@ -294,7 +292,7 @@ describe('UploadDataModal - File Settings Elements', () => {
 
 describe('UploadDataModal - Columns Elements', () => {
   const openColumns = async () => {
-    const panelHeader = screen.getByRole('heading', { name: /columns/i });
+    const panelHeader = screen.getByText(/columns/i, { selector: 'strong' });
     await userEvent.click(panelHeader);
   };
 
@@ -365,7 +363,7 @@ describe('UploadDataModal - Rows Elements', () => {
   test('CSV/Excel rows render correctly', async () => {
     render(<UploadDataModal {...csvProps} />, { useRedux: true });
 
-    const panelHeader = screen.getByRole('heading', { name: /rows/i });
+    const panelHeader = screen.getByText(/rows/i);
     await userEvent.click(panelHeader);
 
     const elements = [
@@ -380,7 +378,7 @@ describe('UploadDataModal - Rows Elements', () => {
   test('Columnar does not render rows', () => {
     render(<UploadDataModal {...columnarProps} />, { useRedux: true });
 
-    const panelHeader = screen.queryByRole('heading', { name: /rows/i });
+    const panelHeader = screen.queryByText(/rows/i);
     expect(panelHeader).not.toBeInTheDocument();
   });
 });
@@ -608,11 +606,11 @@ describe('UploadDataModal Collapse Tabs', () => {
       useRedux: true,
     });
     const generalInfoTab = screen.getByRole('tab', {
-      name: /expanded General information Upload a file to a database./i,
+      name: /expanded General information/i,
     });
     expect(generalInfoTab).toHaveAttribute('aria-expanded', 'true');
     const fileSettingsTab = screen.getByRole('tab', {
-      name: /collapsed File settings Adjust how spaces, blank lines, null values are handled and other file wide settings./i,
+      name: /collapsed File settings/i,
     });
     userEvent.click(fileSettingsTab);
     expect(fileSettingsTab).toHaveAttribute('aria-expanded', 'true');
@@ -626,11 +624,11 @@ describe('UploadDataModal Collapse Tabs', () => {
       useRedux: true,
     });
     const generalInfoTab = screen.getByRole('tab', {
-      name: /expanded General information Upload a file to a database./i,
+      name: /expanded General information/i,
     });
     expect(generalInfoTab).toHaveAttribute('aria-expanded', 'true');
     const fileSettingsTab = screen.getByRole('tab', {
-      name: /collapsed File settings Adjust how spaces, blank lines, null values are handled and other file wide settings./i,
+      name: /collapsed File settings/i,
     });
     userEvent.click(fileSettingsTab);
     expect(fileSettingsTab).toHaveAttribute('aria-expanded', 'true');
@@ -644,11 +642,11 @@ describe('UploadDataModal Collapse Tabs', () => {
       useRedux: true,
     });
     const generalInfoTab = screen.getByRole('tab', {
-      name: /expanded General information Upload a file to a database./i,
+      name: /expanded General information/i,
     });
     expect(generalInfoTab).toHaveAttribute('aria-expanded', 'true');
     const fileSettingsTab = screen.getByRole('tab', {
-      name: /collapsed File settings Adjust how spaces, blank lines, null values are handled and other file wide settings./i,
+      name: /collapsed File settings/i,
     });
     userEvent.click(fileSettingsTab);
     expect(fileSettingsTab).toHaveAttribute('aria-expanded', 'true');

superset-frontend/src/features/databases/UploadDataModel/index.tsx

@@ -26,6 +26,7 @@ import {
 } from 'react';
 
 import {
+  css,
   getClientErrorObject,
   SupersetClient,
   SupersetTheme,
@@ -45,6 +46,7 @@ import {
   Upload,
   type UploadChangeParam,
   type UploadFile,
+  Typography,
 } from '@superset-ui/core/components';
 import { Switch, SwitchProps } from '@superset-ui/core/components/Switch';
 import { Icons } from '@superset-ui/core/components/Icons';
@@ -576,7 +578,17 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
 
   const UploadTitle: FC = () => {
     const title = uploadTitles[type] || t('Upload');
-    return <h4>{title}</h4>;
+    return (
+      <Typography.Title
+        level={5}
+        css={css`
+          margin-top: 0;
+          margin-bottom: 0;
+        `}
+      >
+        {title}
+      </Typography.Title>
+    );
   };
 
   return (
@@ -592,7 +604,7 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
       onHandledPrimaryAction={form.submit}
       onHide={onClose}
       width="500px"
-      primaryButtonName="Upload"
+      primaryButtonName={t('Upload')}
       centered
       show={show}
       title={<UploadTitle />}
@@ -615,10 +627,9 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
             {
               key: 'general',
               label: (
-                <div>
-                  <h4>{t('General information')}</h4>
-                  <p className="helper">{t('Upload a file to a database.')}</p>
-                </div>
+                <Typography.Text strong>
+                  {t('General information')}
+                </Typography.Text>
               ),
               children: (
                 <>
@@ -772,14 +783,7 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
             {
               key: 'file-settings',
               label: (
-                <div>
-                  <h4>{t('File settings')}</h4>
-                  <p className="helper">
-                    {t(
-                      'Adjust how spaces, blank lines, null values are handled and other file wide settings.',
-                    )}
-                  </p>
-                </div>
+                <Typography.Text strong>{t('File settings')}</Typography.Text>
               ),
               children: (
                 <>
@@ -901,16 +905,7 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
             },
             {
               key: 'columns',
-              label: (
-                <div>
-                  <h4>{t('Columns')}</h4>
-                  <p className="helper">
-                    {t(
-                      'Adjust column settings such as specifying the columns to read, how duplicates are handled, column data types, and more.',
-                    )}
-                  </p>
-                </div>
-              ),
+              label: <Typography.Text strong>{t('Columns')}</Typography.Text>,
               children: (
                 <>
                   <Row>
@@ -1010,14 +1005,7 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
                   {
                     key: 'rows',
                     label: (
-                      <div>
-                        <h4>{t('Rows')}</h4>
-                        <p className="helper">
-                          {t(
-                            'Set header rows and the number of rows to read or skip.',
-                          )}
-                        </p>
-                      </div>
+                      <Typography.Text strong>{t('Rows')}</Typography.Text>
                     ),
                     children: (
                       <Row>

superset-frontend/src/features/databases/UploadDataModel/styles.ts

@@ -46,7 +46,7 @@ export const antDModalNoPaddingStyles = css`
 
 export const formStyles = (theme: SupersetTheme) => css`
   .switch-label {
-    color: ${theme.colors.grayscale.base};
+    color: ${theme.colorTextSecondary};
     margin-left: ${theme.sizeUnit * 4}px;
   }
 `;