From ca648cb026e2334e1c48a8ddff5352c6d8879424 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Tue, 14 Jan 2025 23:28:19 -0800 Subject: [PATCH 01/13] chore: refactor Alert-related components Chiseling at https://github.com/apache/superset/pull/31590 that has gotten big / unruly, in this PR is a refactor of Alert-related components, going vanilla AntD. Also. Deprecating colors.alerts since it's ambiguous/redundant with warning/error and does not exist in antd-v5 --- .../superset-ui-core/src/style/index.tsx | 7 - .../src/ReactCalendar.jsx | 8 +- .../src/components/Alert/Alert.test.tsx | 20 +- .../src/components/Alert/index.tsx | 46 +-- .../src/components/AlteredSliceTag/index.tsx | 2 +- .../src/components/Chart/Chart.tsx | 20 +- .../components/Chart/ChartErrorMessage.tsx | 7 +- .../src/components/ErrorBoundary/index.tsx | 22 +- .../ErrorMessage/DatabaseErrorMessage.tsx | 28 +- .../DatasetNotFoundErrorMessage.tsx | 12 +- .../ErrorMessage/ErrorAlert.stories.tsx | 151 ++++++++ .../ErrorMessage/ErrorAlert.test.tsx | 231 ++++-------- .../components/ErrorMessage/ErrorAlert.tsx | 341 ++++++------------ .../ErrorMessageWithStackTrace.test.tsx | 2 +- .../ErrorMessageWithStackTrace.tsx | 37 +- .../FrontendNetworkErrorMessage.tsx | 33 ++ .../InvalidSQLErrorMessage.test.tsx | 135 +++---- .../ErrorMessage/InvalidSQLErrorMessage.tsx | 16 +- .../ErrorMessage/OAuth2RedirectMessage.tsx | 9 +- .../ParameterErrorMessage.test.tsx | 2 +- .../ErrorMessage/ParameterErrorMessage.tsx | 15 +- .../ErrorMessage/TimeoutErrorMessage.tsx | 15 +- .../src/components/Label/Label.stories.tsx | 1 - .../src/components/Label/index.tsx | 15 +- .../WarningIconWithTooltip/index.tsx | 2 +- superset-frontend/src/components/index.ts | 1 + .../DashboardBuilder/DashboardWrapper.tsx | 2 +- .../src/explore/components/ControlHeader.tsx | 9 +- .../components/ControlPanelsContainer.tsx | 6 +- .../src/explore/components/ExploreAlert.tsx | 8 +- .../controls/ColorSchemeControl/index.tsx | 2 +- .../FormattingPopoverContent.tsx | 4 +- .../DatasourceControl.test.tsx | 2 +- .../controls/DatasourceControl/index.jsx | 44 +-- .../alerts/components/AlertStatusIcon.tsx | 4 +- .../databases/DatabaseModal/index.test.tsx | 8 +- .../databases/DatabaseModal/index.tsx | 5 +- .../src/setup/setupErrorMessages.ts | 5 + superset-frontend/src/theme/index.ts | 9 - superset-frontend/src/views/App.tsx | 11 +- 40 files changed, 571 insertions(+), 726 deletions(-) create mode 100644 superset-frontend/src/components/ErrorMessage/ErrorAlert.stories.tsx create mode 100644 superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx diff --git a/superset-frontend/packages/superset-ui-core/src/style/index.tsx b/superset-frontend/packages/superset-ui-core/src/style/index.tsx index ee0b6e10ac41..c4964172be62 100644 --- a/superset-frontend/packages/superset-ui-core/src/style/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/style/index.tsx @@ -98,13 +98,6 @@ const defaultTheme = { light2: '#FAEDEE', }, warning: { - base: '#FF7F44', - dark1: '#BF5E33', - dark2: '#7F3F21', - light1: '#FEC0A1', - light2: '#FFF2EC', - }, - alert: { base: '#FCC700', dark1: '#BC9501', dark2: '#7D6300', diff --git a/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx b/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx index f3fcc807d8a0..46cb1de852ad 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx +++ b/superset-frontend/plugins/legacy-plugin-chart-calendar/src/ReactCalendar.jsx @@ -157,13 +157,13 @@ export default styled(Calendar)` } .cal-heatmap-container .q1 { - background-color: ${theme.colors.alert.light2}; - fill: ${theme.colors.alert.light2}; + background-color: ${theme.colors.warning.light2}; + fill: ${theme.colors.warning.light2}; } .cal-heatmap-container .q2 { - background-color: ${theme.colors.alert.light1}; - fill: ${theme.colors.alert.light1}; + background-color: ${theme.colors.warning.light1}; + fill: ${theme.colors.warning.light1}; } .cal-heatmap-container .q3 { diff --git a/superset-frontend/src/components/Alert/Alert.test.tsx b/superset-frontend/src/components/Alert/Alert.test.tsx index 89f221e0c5eb..f7cb342a75d5 100644 --- a/superset-frontend/src/components/Alert/Alert.test.tsx +++ b/superset-frontend/src/components/Alert/Alert.test.tsx @@ -27,19 +27,17 @@ test('renders with default props', async () => { render(); expect(screen.getByRole('alert')).toHaveTextContent('Message'); - expect(await screen.findByLabelText('info icon')).toBeInTheDocument(); - expect(await screen.findByLabelText('close icon')).toBeInTheDocument(); + expect(screen.getByRole('img', { name: 'info-circle' })).toBeInTheDocument(); }); -test('renders each type', async () => { +test('renders message for each alert type', () => { const types: AlertTypeValue[] = ['info', 'error', 'warning', 'success']; - await Promise.all( - types.map(async type => { - render(); - expect(await screen.findByLabelText(`${type} icon`)).toBeInTheDocument(); - }), - ); + types.forEach(type => { + const { rerender } = render(); + expect(screen.getByText('Test message')).toBeInTheDocument(); + rerender(<>); // Clean up between renders + }); }); test('renders without close button', async () => { @@ -51,7 +49,7 @@ test('renders without close button', async () => { test('disappear when closed', async () => { render(); - userEvent.click(screen.getByLabelText('close icon')); + userEvent.click(screen.getByRole('img', { name: 'close' })); await waitFor(() => { expect(screen.queryByRole('alert')).not.toBeInTheDocument(); }); @@ -74,6 +72,6 @@ test('renders message and description', async () => { test('calls onClose callback when closed', () => { const onCloseMock = jest.fn(); render(); - userEvent.click(screen.getByLabelText('close icon')); + userEvent.click(screen.getByRole('img', { name: 'close' })); expect(onCloseMock).toHaveBeenCalledTimes(1); }); diff --git a/superset-frontend/src/components/Alert/index.tsx b/superset-frontend/src/components/Alert/index.tsx index 6a85739950fa..c567091aea0a 100644 --- a/superset-frontend/src/components/Alert/index.tsx +++ b/superset-frontend/src/components/Alert/index.tsx @@ -19,8 +19,6 @@ import { PropsWithChildren } from 'react'; 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< Omit & { roomBelow?: boolean } @@ -32,59 +30,17 @@ export default function Alert(props: AlertProps) { description, showIcon = true, closable = true, - roomBelow = false, children, } = props; - const theme = useTheme(); - 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 = alertColor; - AlertIcon = Icons.AlertSolid; - } else if (type === 'success') { - baseColor = success; - AlertIcon = Icons.CircleCheckSolid; - } - return ( - - - ) - } - closeIcon={closable && } + 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} /> ); diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index 9aca6b46b84d..46fc58b3ffef 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -221,7 +221,7 @@ const AlteredSliceTag: FC = props => {