From 79aff6827c03cc24fe44be407be3ca11683f3993 Mon Sep 17 00:00:00 2001 From: Levis Mbote <111055098+LevisNgigi@users.noreply.github.com> Date: Fri, 6 Dec 2024 22:26:04 +0300 Subject: [PATCH] refactor(Alert): Migrate Alert component to Ant Design V5 (#31168) Co-authored-by: Diego Pucci --- .../e2e/explore/visualizations/line.test.ts | 4 +- .../cypress/support/directories.ts | 8 +- .../src/components/Alert/Alert.stories.tsx | 80 ++++++++++++------- .../src/components/Alert/Alert.test.tsx | 41 +++++----- .../src/components/Alert/index.tsx | 68 ++++++++-------- .../src/components/ImportModal/styles.ts | 17 +--- .../Footer/CancelConfirmationAlert.tsx | 2 +- .../databases/DatabaseModal/styles.ts | 61 +------------- .../features/reports/ReportModal/styles.tsx | 17 ---- superset-frontend/src/theme/index.ts | 9 +++ 10 files changed, 126 insertions(+), 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 index 070a762e808a8..ef7b9b53dad47 100644 --- 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 index 77f455479c001..9d5a554d3a146 100644 --- 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 index 10a9f8d677330..9aff2afee617b 100644 --- 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; -type AlertTypeValue = AlertType[keyof AlertType]; +type AlertType = Required>; +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,40 +37,46 @@ export default { export const AlertGallery = () => ( <> {types.map(type => ( -
-

{type}

- - +
+

{type} Alerts

+
+ + x + + } + /> +
))} ); -AlertGallery.parameters = { - actions: { - disable: true, - }, - controls: { - disable: true, - }, -}; - export const InteractiveAlert = (args: AlertProps) => ( <> - Some content to test the `roomBelow` prop +
+ Content below the Alert to test the `roomBelow` property +
); @@ -79,8 +84,8 @@ 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 index 61c9b74fd9744..89f221e0c5eb1 100644 --- 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(); 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(); - // eslint-disable-next-line no-await-in-loop - expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument(); - } + + await Promise.all( + types.map(async type => { + render(); + expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument(); + }), + ); }); test('renders without close button', async () => { render(); - await waitFor(() => { expect(screen.queryByLabelText('close icon')).not.toBeInTheDocument(); }); }); -test('disappear when closed', () => { +test('disappear when closed', async () => { render(); - 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(); + render(); await waitFor(() => { - expect(screen.queryByLabelText(`${type} icon`)).not.toBeInTheDocument(); + expect(screen.queryByLabelText('info icon')).not.toBeInTheDocument(); }); }); -test('renders message', async () => { - render(); - expect(await screen.findByRole('alert')).toHaveTextContent('Message'); -}); - test('renders message and description', async () => { render(); 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(); + 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 index 5c2e14704e3fc..6a85739950fa3 100644 --- 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 & { roomBelow?: boolean } >; export default function Alert(props: AlertProps) { @@ -36,8 +37,8 @@ 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; @@ -45,7 +46,7 @@ export default function Alert(props: AlertProps) { 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 ( } - closeText={closable && } - 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 && ( + + + + ) + } + closeIcon={closable && } + 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} - + /> ); } diff --git a/superset-frontend/src/components/ImportModal/styles.ts b/superset-frontend/src/components/ImportModal/styles.ts index c73dc7c1ab277..f0120f62398bf 100644 --- 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/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/Footer/CancelConfirmationAlert.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/Footer/CancelConfirmationAlert.tsx index 16d15a06b0d71..813322ccdfa58 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/Footer/CancelConfirmationAlert.tsx +++ b/superset-frontend/src/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={ diff --git a/superset-frontend/src/features/databases/DatabaseModal/styles.ts b/superset-frontend/src/features/databases/DatabaseModal/styles.ts index 95d77781b57ed..89f87a8bc3388 100644 --- a/superset-frontend/src/features/databases/DatabaseModal/styles.ts +++ b/superset-frontend/src/features/databases/DatabaseModal/styles.ts @@ -166,82 +166,25 @@ export const antDModalStyles = (theme: SupersetTheme) => css` `; export const antDAlertStyles = (theme: SupersetTheme) => css` - border: 1px solid ${theme.colors.info.base}; - padding: ${theme.gridUnit * 4}px; margin: ${theme.gridUnit * 4}px 0; - - .ant-alert-message { - color: ${theme.colors.info.dark2}; - font-size: ${theme.typography.sizes.m}px; - font-weight: ${theme.typography.weights.bold}; - } - - .ant-alert-description { - color: ${theme.colors.info.dark2}; - font-size: ${theme.typography.sizes.m}px; - line-height: ${theme.gridUnit * 5}px; - - a { - text-decoration: underline; - } - - .ant-alert-icon { - margin-right: ${theme.gridUnit * 2.5}px; - font-size: ${theme.typography.sizes.l}px; - position: relative; - top: ${theme.gridUnit / 4}px; - } - } `; export const StyledAlertMargin = styled.div` ${({ theme }) => css` - margin: 0 ${theme.gridUnit * 4}px -${theme.gridUnit * 4}px; + margin: 0 ${theme.gridUnit * 4}px ${theme.gridUnit * 4}px; `} `; export const antDErrorAlertStyles = (theme: SupersetTheme) => css` - border: ${theme.colors.error.base} 1px solid; - padding: ${theme.gridUnit * 4}px; margin: ${theme.gridUnit * 8}px ${theme.gridUnit * 4}px; - color: ${theme.colors.error.dark2}; - .ant-alert-message { - font-size: ${theme.typography.sizes.m}px; - font-weight: ${theme.typography.weights.bold}; - } - .ant-alert-description { - font-size: ${theme.typography.sizes.m}px; - line-height: ${theme.gridUnit * 5}px; - .ant-alert-icon { - margin-right: ${theme.gridUnit * 2.5}px; - font-size: ${theme.typography.sizes.l}px; - position: relative; - top: ${theme.gridUnit / 4}px; - } - } `; 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; - } - } `; export const formHelperStyles = (theme: SupersetTheme) => css` diff --git a/superset-frontend/src/features/reports/ReportModal/styles.tsx b/superset-frontend/src/features/reports/ReportModal/styles.tsx index dd0a410ef51d6..91b6e5e826b53 100644 --- a/superset-frontend/src/features/reports/ReportModal/styles.tsx +++ b/superset-frontend/src/features/reports/ReportModal/styles.tsx @@ -113,23 +113,6 @@ export const StyledRadioGroup = styled(Radio.Group)` `; export const antDErrorAlertStyles = (theme: SupersetTheme) => css` - border: ${theme.colors.error.base} 1px solid; - padding: ${theme.gridUnit * 4}px; margin: ${theme.gridUnit * 4}px; margin-top: 0; - color: ${theme.colors.error.dark2}; - .ant-alert-message { - font-size: ${theme.typography.sizes.m}px; - font-weight: bold; - } - .ant-alert-description { - font-size: ${theme.typography.sizes.m}px; - line-height: ${theme.gridUnit * 4}px; - .ant-alert-icon { - margin-right: ${theme.gridUnit * 2.5}px; - font-size: ${theme.typography.sizes.l}px; - position: relative; - top: ${theme.gridUnit / 4}px; - } - } `; diff --git a/superset-frontend/src/theme/index.ts b/superset-frontend/src/theme/index.ts index 858f92fe76d23..70d6cd5eb7486 100644 --- a/superset-frontend/src/theme/index.ts +++ b/superset-frontend/src/theme/index.ts @@ -55,6 +55,15 @@ const baseConfig: ThemeConfig = { zIndexPopupBase: supersetTheme.zIndex.max, }, components: { + Alert: { + borderRadius: supersetTheme.borderRadius, + colorBgContainer: supersetTheme.colors.grayscale.light5, + colorBorder: supersetTheme.colors.grayscale.light3, + fontSize: supersetTheme.typography.sizes.m, + fontSizeLG: supersetTheme.typography.sizes.m, + fontSizeIcon: supersetTheme.typography.sizes.l, + colorText: supersetTheme.colors.grayscale.dark1, + }, Avatar: { containerSize: 32, fontSize: supersetTheme.typography.sizes.s,