refactor(Alert): Migrate Alert component to Ant Design V5 (apache#31168)

Co-authored-by: Diego Pucci <[email protected]>
preset-io · Dec 6, 2024 · 79aff68 · 79aff68
1 parent 079e732
commit 79aff68
Show file tree

Hide file tree

Showing 10 changed files with 126 additions and 181 deletions.
diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/line.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/visualizations/line.test.ts
@@ -62,7 +62,7 @@ describe('Visualization > Line', () => {
       'not.exist',
     );
 
-    cy.get('.ant-alert-warning').should('not.exist');
+    cy.get('.antd5-alert-warning').should('not.exist');
   });
 
   it('should allow negative values in Y bounds', () => {
@@ -71,7 +71,7 @@ describe('Visualization > Line', () => {
     cy.get('#controlSections-tab-display').click();
     cy.get('span').contains('Y Axis Bounds').scrollIntoView();
     cy.get('input[placeholder="Min"]').type('-0.1', { delay: 100 });
-    cy.get('.ant-alert-warning').should('not.exist');
+    cy.get('.antd5-alert-warning').should('not.exist');
   });
 
   it('should allow type to search color schemes and apply the scheme', () => {

diff --git a/superset-frontend/cypress-base/cypress/support/directories.ts b/superset-frontend/cypress-base/cypress/support/directories.ts
@@ -94,7 +94,7 @@ export const databasesPage = {
     dbDropdown: '[class="ant-select-selection-search-input"]',
     dbDropdownMenu: '.rc-virtual-list-holder-inner',
     dbDropdownMenuItem: '[class="ant-select-item-option-content"]',
-    infoAlert: '.ant-alert',
+    infoAlert: '.antd5-alert',
     serviceAccountInput: '[name="credentials_info"]',
     connectionStep: {
       modal: '.ant-modal-content',
@@ -103,7 +103,7 @@ export const databasesPage = {
       helperBottom: '.helper-bottom',
       postgresDatabase: '[name="database"]',
       dbInput: '[name="database_name"]',
-      alertMessage: '.ant-alert-message',
+      alertMessage: '.antd5-alert-message',
       errorField: '[role="alert"]',
       uploadJson: '[title="Upload JSON file"]',
       chooseFile: '[class="ant-btn input-upload-btn"]',
@@ -166,7 +166,7 @@ export const sqlLabView = {
   renderedTableHeader: '.ReactVirtualized__Table__headerRow',
   renderedTableRow: '.ReactVirtualized__Table__row',
   errorBody: '.error-body',
-  alertMessage: '.ant-alert-message',
+  alertMessage: '.antd5-alert-message',
   historyTable: {
     header: '[role=columnheader]',
     table: '.QueryTable',
@@ -325,7 +325,7 @@ export const nativeFilters = {
     confirmCancelButton: dataTestLocator(
       'native-filter-modal-confirm-cancel-button',
     ),
-    alertXUnsavedFilters: '.ant-alert-message',
+    alertXUnsavedFilters: '.antd5-alert-message',
     tabsList: {
       filterItemsContainer: dataTestLocator('filter-title-container'),
       tabsContainer: '[class="ant-tabs-nav-list"]',

diff --git a/superset-frontend/src/components/Alert/Alert.stories.tsx b/superset-frontend/src/components/Alert/Alert.stories.tsx
@@ -18,13 +18,12 @@
  */
 import Alert, { AlertProps } from './index';
 
-type AlertType = Pick<AlertProps, 'type'>;
-type AlertTypeValue = AlertType[keyof AlertType];
+type AlertType = Required<Pick<AlertProps, 'type'>>;
+type AlertTypeValue = AlertType['type'];
 
 const types: AlertTypeValue[] = ['info', 'error', 'warning', 'success'];
 
 const smallText = 'Lorem ipsum dolor sit amet';
-
 const bigText =
   'Lorem ipsum dolor sit amet, consectetur adipiscing elit. ' +
   'Nam id porta neque, a vehicula orci. Maecenas rhoncus elit sit amet ' +
@@ -38,49 +37,55 @@ export default {
 export const AlertGallery = () => (
   <>
     {types.map(type => (
-      <div key={type} style={{ marginBottom: 40, width: 600 }}>
-        <h4>{type}</h4>
-        <Alert
-          type={type}
-          showIcon
-          closable
-          message={bigText}
-          style={{ marginBottom: 20 }}
-        />
-        <Alert
-          type={type}
-          showIcon
-          message={smallText}
-          description={bigText}
-          closable
-        />
+      <div key={type} style={{ marginBottom: '40px', width: '600px' }}>
+        <h4 style={{ textTransform: 'capitalize' }}>{type} Alerts</h4>
+        <div style={{ display: 'flex', flexDirection: 'column', gap: '20px' }}>
+          <Alert
+            type={type}
+            showIcon
+            message={smallText}
+            description={bigText}
+            closable
+            closeIcon={
+              <span
+                aria-label="close icon"
+                style={{
+                  fontSize: '12px',
+                  fontWeight: 'bold',
+                }}
+              >
+                x
+              </span>
+            }
+          />
+        </div>
       </div>
     ))}
   </>
 );
 
-AlertGallery.parameters = {
-  actions: {
-    disable: true,
-  },
-  controls: {
-    disable: true,
-  },
-};
-
 export const InteractiveAlert = (args: AlertProps) => (
   <>
     <Alert {...args} />
-    Some content to test the `roomBelow` prop
+    <div
+      style={{
+        marginTop: args.roomBelow ? '40px' : '0px',
+        border: '1px dashed gray',
+        padding: '10px',
+        textAlign: 'center',
+      }}
+    >
+      Content below the Alert to test the `roomBelow` property
+    </div>
   </>
 );
 
 InteractiveAlert.args = {
   closable: true,
   roomBelow: false,
   type: 'info',
-  message: smallText,
-  description: bigText,
+  message: 'This is a sample alert message.',
+  description: 'Sample description for additional context.',
   showIcon: true,
 };
 
@@ -89,5 +94,18 @@ InteractiveAlert.argTypes = {
   type: {
     control: { type: 'select' },
     options: types,
+    description: 'Type of the alert (e.g., info, error, warning, success).',
+  },
+  closable: {
+    control: { type: 'boolean' },
+    description: 'Whether the Alert can be closed with a close button.',
+  },
+  showIcon: {
+    control: { type: 'boolean' },
+    description: 'Whether to display an icon in the Alert.',
+  },
+  roomBelow: {
+    control: { type: 'boolean' },
+    description: 'Adds margin below the Alert for layout spacing.',
   },
 };
diff --git a/superset-frontend/src/components/Alert/Alert.test.tsx b/superset-frontend/src/components/Alert/Alert.test.tsx
@@ -27,50 +27,53 @@ test('renders with default props', async () => {
   render(<Alert message="Message" />);
 
   expect(screen.getByRole('alert')).toHaveTextContent('Message');
-  expect(await screen.findByLabelText(`info icon`)).toBeInTheDocument();
+  expect(await screen.findByLabelText('info icon')).toBeInTheDocument();
   expect(await screen.findByLabelText('close icon')).toBeInTheDocument();
 });
 
 test('renders each type', async () => {
   const types: AlertTypeValue[] = ['info', 'error', 'warning', 'success'];
-  for (let i = 0; i < types.length; i += 1) {
-    const type = types[i];
-    render(<Alert type={type} message="Message" />);
-    // eslint-disable-next-line no-await-in-loop
-    expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument();
-  }
+
+  await Promise.all(
+    types.map(async type => {
+      render(<Alert type={type} message="Message" />);
+      expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument();
+    }),
+  );
 });
 
 test('renders without close button', async () => {
   render(<Alert message="Message" closable={false} />);
-
   await waitFor(() => {
     expect(screen.queryByLabelText('close icon')).not.toBeInTheDocument();
   });
 });
 
-test('disappear when closed', () => {
+test('disappear when closed', async () => {
   render(<Alert message="Message" />);
-  userEvent.click(screen.queryByLabelText('close icon')!);
-  expect(screen.queryByRole('alert')).not.toBeInTheDocument();
+  userEvent.click(screen.getByLabelText('close icon'));
+  await waitFor(() => {
+    expect(screen.queryByRole('alert')).not.toBeInTheDocument();
+  });
 });
 
 test('renders without icon', async () => {
-  const type = 'info';
-  render(<Alert type={type} message="Message" showIcon={false} />);
+  render(<Alert type="info" message="Message" showIcon={false} />);
   await waitFor(() => {
-    expect(screen.queryByLabelText(`${type} icon`)).not.toBeInTheDocument();
+    expect(screen.queryByLabelText('info icon')).not.toBeInTheDocument();
   });
 });
 
-test('renders message', async () => {
-  render(<Alert message="Message" />);
-  expect(await screen.findByRole('alert')).toHaveTextContent('Message');
-});
-
 test('renders message and description', async () => {
   render(<Alert message="Message" description="Description" />);
   const alert = await screen.findByRole('alert');
   expect(alert).toHaveTextContent('Message');
   expect(alert).toHaveTextContent('Description');
 });
+
+test('calls onClose callback when closed', () => {
+  const onCloseMock = jest.fn();
+  render(<Alert message="Message" onClose={onCloseMock} />);
+  userEvent.click(screen.getByLabelText('close icon'));
+  expect(onCloseMock).toHaveBeenCalledTimes(1);
+});
diff --git a/superset-frontend/src/components/Alert/index.tsx b/superset-frontend/src/components/Alert/index.tsx
@@ -17,12 +17,13 @@
  * under the License.
  */
 import { PropsWithChildren } from 'react';
-import AntdAlert, { AlertProps as AntdAlertProps } from 'antd/lib/alert';
-import { useTheme } from '@superset-ui/core';
+import { Alert as AntdAlert } from 'antd-v5';
+import { AlertProps as AntdAlertProps } from 'antd-v5/lib/alert';
+import { css, useTheme } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
 
 export type AlertProps = PropsWithChildren<
-  AntdAlertProps & { roomBelow?: boolean }
+  Omit<AntdAlertProps, 'children'> & { roomBelow?: boolean }
 >;
 
 export default function Alert(props: AlertProps) {
@@ -36,16 +37,16 @@ export default function Alert(props: AlertProps) {
   } = props;
 
   const theme = useTheme();
-  const { colors, typography, gridUnit } = theme;
-  const { alert, error, info, success } = colors;
+  const { colors } = theme;
+  const { alert: alertColor, error, info, success } = colors;
 
   let baseColor = info;
   let AlertIcon = Icons.InfoSolid;
   if (type === 'error') {
     baseColor = error;
     AlertIcon = Icons.ErrorSolid;
   } else if (type === 'warning') {
-    baseColor = alert;
+    baseColor = alertColor;
     AlertIcon = Icons.AlertSolid;
   } else if (type === 'success') {
     baseColor = success;
@@ -55,33 +56,36 @@ export default function Alert(props: AlertProps) {
   return (
     <AntdAlert
       role="alert"
+      aria-live={type === 'error' ? 'assertive' : 'polite'}
       showIcon={showIcon}
-      icon={<AlertIcon aria-label={`${type} icon`} />}
-      closeText={closable && <Icons.XSmall aria-label="close icon" />}
-      css={{
-        marginBottom: roomBelow ? gridUnit * 4 : 0,
-        padding: `${gridUnit * 2}px ${gridUnit * 3}px`,
-        alignItems: 'flex-start',
-        border: 0,
-        backgroundColor: baseColor.light2,
-        '& .ant-alert-icon': {
-          marginRight: gridUnit * 2,
-        },
-        '& .ant-alert-message': {
-          color: baseColor.dark2,
-          fontSize: typography.sizes.m,
-          fontWeight: description
-            ? typography.weights.bold
-            : typography.weights.normal,
-        },
-        '& .ant-alert-description': {
-          color: baseColor.dark2,
-          fontSize: typography.sizes.m,
-        },
-      }}
+      icon={
+        showIcon && (
+          <span
+            role="img"
+            aria-label={`${type} icon`}
+            style={{
+              color: baseColor.base,
+            }}
+          >
+            <AlertIcon />
+          </span>
+        )
+      }
+      closeIcon={closable && <Icons.XSmall aria-label="close icon" />}
+      message={children || 'Default message'}
+      description={description}
+      css={css`
+        margin-bottom: ${roomBelow ? theme.gridUnit * 4 : 0}px;
+        a {
+          text-decoration: underline;
+        }
+        .antd5-alert-message {
+          font-weight: ${description
+            ? theme.typography.weights.bold
+            : 'inherit'};
+        }
+      `}
       {...props}
-    >
-      {children}
-    </AntdAlert>
+    />
   );
 }
diff --git a/superset-frontend/src/components/ImportModal/styles.ts b/superset-frontend/src/components/ImportModal/styles.ts
@@ -20,24 +20,9 @@
 import { css, SupersetTheme } from '@superset-ui/core';
 
 export const antdWarningAlertStyles = (theme: SupersetTheme) => css`
-  border: 1px solid ${theme.colors.warning.light1};
-  padding: ${theme.gridUnit * 4}px;
   margin: ${theme.gridUnit * 4}px 0;
-  color: ${theme.colors.warning.dark2};
 
-  .ant-alert-message {
+  .antd5-alert-message {
     margin: 0;
   }
-
-  .ant-alert-description {
-    font-size: ${theme.typography.sizes.s + 1}px;
-    line-height: ${theme.gridUnit * 4}px;
-
-    .ant-alert-icon {
-      margin-right: ${theme.gridUnit * 2.5}px;
-      font-size: ${theme.typography.sizes.l + 1}px;
-      position: relative;
-      top: ${theme.gridUnit / 4}px;
-    }
-  }
 `;
diff --git a/.../dashboard/components/nativeFilters/FiltersConfigModal/Footer/CancelConfirmationAlert.tsx b/.../dashboard/components/nativeFilters/FiltersConfigModal/Footer/CancelConfirmationAlert.tsx
@@ -43,7 +43,7 @@ export function CancelConfirmationAlert({
       css={{
         textAlign: 'left',
         flex: 1,
-        '& .ant-alert-action': { alignSelf: 'center' },
+        '& .antd5-alert-action': { alignSelf: 'center' },
       }}
       description={children}
       action={