This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 30695d75d7 fix(DatabaseModal): Resolve Connect button issue for
SQLAlchemy URI database connections (#34112)
30695d75d7 is described below
commit 30695d75d7ca8350d619c787f6ecd151b8a00cac
Author: Enzo Martellucci <52219496+enx...@users.noreply.github.com>
AuthorDate: Fri Jul 11 13:03:36 2025 +0200
fix(DatabaseModal): Resolve Connect button issue for SQLAlchemy URI
database connections (#34112)
---
.../components/DeleteModal/DeleteModal.test.tsx | 6 +-
.../src/components/Modal/Modal.tsx | 210 +++++++++++----------
.../src/components/ErrorMessage/ErrorAlert.tsx | 1 +
.../PropertiesModal/PropertiesModal.test.tsx | 2 +-
.../src/features/databases/DatabaseModal/index.tsx | 26 ++-
5 files changed, 134 insertions(+), 111 deletions(-)
diff --git
a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx
b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx
index be2868afe9..dcf4cb508d 100644
---
a/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx
+++
b/superset-frontend/packages/superset-ui-core/src/components/DeleteModal/DeleteModal.test.tsx
@@ -62,12 +62,12 @@ test('Calling "onHide"', async () => {
expect(props.onConfirm).toHaveBeenCalledTimes(0);
// type "del" in the input
- await userEvent.type(screen.getByTestId('delete-modal-input'), 'del');
+ userEvent.type(screen.getByTestId('delete-modal-input'), 'del');
expect(screen.getByTestId('delete-modal-input')).toHaveValue('del');
// close the modal
- expect(screen.getByText('×')).toBeInTheDocument();
- await userEvent.click(screen.getByText('×'));
+ expect(screen.getByTestId('close-modal-btn')).toBeInTheDocument();
+ userEvent.click(screen.getByTestId('close-modal-btn'));
expect(props.onHide).toHaveBeenCalledTimes(1);
expect(props.onConfirm).toHaveBeenCalledTimes(0);
diff --git
a/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx
b/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx
index 5b9f1c2531..4b7125dea5 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx
@@ -18,7 +18,7 @@
*/
import { isValidElement, cloneElement, useMemo, useRef, useState } from
'react';
import { isNil } from 'lodash';
-import { css, styled, t } from '@superset-ui/core';
+import { css, styled, t, useTheme } from '@superset-ui/core';
import { Modal as AntdModal, ModalProps as AntdModalProps } from 'antd';
import { Resizable } from 're-resizable';
import Draggable, {
@@ -26,6 +26,7 @@ import Draggable, {
DraggableData,
DraggableEvent,
} from 'react-draggable';
+import { Icons } from '../Icons';
import { Button } from '../Button';
import type { ModalProps, StyledModalProps } from './types';
@@ -45,8 +46,16 @@ export const BaseModal = (props: AntdModalProps) => (
);
export const StyledModal = styled(BaseModal)<StyledModalProps>`
- ${({ theme, responsive, maxWidth }) =>
- responsive &&
+ ${({
+ theme,
+ responsive,
+ maxWidth,
+ resizable,
+ height,
+ draggable,
+ hideFooter,
+ }) => css`
+ ${responsive &&
css`
max-width: ${maxWidth ?? '900px'};
padding-left: ${theme.sizeUnit * 3}px;
@@ -55,120 +64,120 @@ export const StyledModal =
styled(BaseModal)<StyledModalProps>`
top: 0;
`}
- .ant-modal-content {
- background-color: ${({ theme }) => theme.colorBgContainer};
- display: flex;
- flex-direction: column;
- max-height: ${({ theme }) => `calc(100vh - ${theme.sizeUnit * 8}px)`};
- margin-bottom: ${({ theme }) => theme.sizeUnit * 4}px;
- margin-top: ${({ theme }) => theme.sizeUnit * 4}px;
- padding: 0;
- }
-
- .ant-modal-header {
- flex: 0 0 auto;
- border-radius: ${({ theme }) => theme.borderRadius}px
- ${({ theme }) => theme.borderRadius}px 0 0;
- padding: ${({ theme }) => theme.sizeUnit * 4}px
- ${({ theme }) => theme.sizeUnit * 6}px;
-
- .ant-modal-title {
- font-weight: ${({ theme }) => theme.fontWeightStrong};
+ .ant-modal-content {
+ background-color: ${theme.colorBgContainer};
+ display: flex;
+ flex-direction: column;
+ max-height: calc(100vh - ${theme.sizeUnit * 8}px);
+ margin-bottom: ${theme.sizeUnit * 4}px;
+ margin-top: ${theme.sizeUnit * 4}px;
+ padding: 0;
+ }
+
+ .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;
+
+ .ant-modal-title {
+ font-weight: ${theme.fontWeightStrong};
+ }
+
+ .ant-modal-title h4 {
+ display: flex;
+ margin: 0;
+ align-items: center;
+ }
}
- .ant-modal-title h4 {
+ .ant-modal-close {
+ width: ${theme.sizeUnit * 14}px;
+ height: ${theme.sizeUnit * 14}px;
+ padding: ${theme.sizeUnit * 6}px ${theme.sizeUnit * 4}px
+ ${theme.sizeUnit * 4}px;
+ top: 0;
+ right: 0;
display: flex;
- margin: 0;
- align-items: center;
+ justify-content: center;
}
- }
-
- .ant-modal-close {
- width: ${({ theme }) => theme.sizeUnit * 14}px;
- height: ${({ theme }) => theme.sizeUnit * 14}px;
- top: 0;
- right: 0;
- }
-
- .ant-modal-close:hover {
- background: transparent;
- }
-
- .ant-modal-close-x {
- display: flex;
- align-items: center;
-
- .close {
- flex: 1 1 auto;
- margin-bottom: ${({ theme }) => theme.sizeUnit}px;
- color: ${({ theme }) => theme.colorPrimaryText};
- font-size: 32px;
- font-weight: ${({ theme }) => theme.fontWeightLight};
+
+ .ant-modal-close:hover {
+ background: transparent;
}
- }
-
- .ant-modal-body {
- flex: 0 1 auto;
- padding: ${({ theme }) => theme.sizeUnit * 4}px;
- overflow: auto;
- ${({ resizable, height }) => !resizable && height && `height: ${height};`}
- }
- .ant-modal-footer {
- flex: 0 0 1;
- border-top: ${({ theme }) => theme.sizeUnit / 4}px solid
- ${({ theme }) => theme.colorSplit};
- padding: ${({ theme }) => theme.sizeUnit * 4}px;
- margin-top: 0;
-
- .btn {
- font-size: 12px;
+
+ .ant-modal-close-x {
+ display: flex;
+ align-items: center;
+ [data-test='close-modal-btn'] {
+ justify-content: center;
+ }
+ .close {
+ flex: 1 1 auto;
+ margin-bottom: ${theme.sizeUnit}px;
+ color: ${theme.colorPrimaryText};
+ font-weight: ${theme.fontWeightLight};
+ }
}
- .btn + .btn {
- margin-left: ${({ theme }) => theme.sizeUnit * 2}px;
+ .ant-modal-body {
+ flex: 0 1 auto;
+ padding: ${theme.sizeUnit * 4}px;
+ overflow: auto;
+ ${!resizable && height && `height: ${height};`}
}
- }
- &.no-content-padding .ant-modal-body {
- padding: 0;
- }
+ .ant-modal-footer {
+ flex: 0 0 1;
+ border-top: ${theme.sizeUnit / 4}px solid ${theme.colorSplit};
+ padding: ${theme.sizeUnit * 4}px;
+ margin-top: 0;
- ${({ draggable, theme }) =>
- draggable &&
- `
- .ant-modal-header {
+ .btn {
+ font-size: 12px;
+ }
+
+ .btn + .btn {
+ margin-left: ${theme.sizeUnit * 2}px;
+ }
+ }
+
+ &.no-content-padding .ant-modal-body {
padding: 0;
- .draggable-trigger {
+ }
+
+ ${draggable &&
+ css`
+ .ant-modal-header {
+ padding: 0;
+
+ .draggable-trigger {
cursor: move;
padding: ${theme.sizeUnit * 4}px;
width: 100%;
}
- }
- `};
+ }
+ `}
- ${({ resizable, hideFooter }) =>
- resizable &&
- `
- .resizable {
- pointer-events: all;
+ ${resizable &&
+ css`
+ .resizable {
+ pointer-events: all;
- .resizable-wrapper {
- height: 100%;
- }
+ .resizable-wrapper {
+ height: 100%;
+ }
- .ant-modal-content {
- height: 100%;
+ .ant-modal-content {
+ height: 100%;
- .ant-modal-body {
- /* 100% - header height - footer height */
- height: ${
- hideFooter
- ? `calc(100% - ${MODAL_HEADER_HEIGHT}px);`
- : `calc(100% - ${MODAL_HEADER_HEIGHT}px -
${MODAL_FOOTER_HEIGHT}px);`
+ .ant-modal-body {
+ height: ${hideFooter
+ ? `calc(100% - ${MODAL_HEADER_HEIGHT}px)`
+ : `calc(100% - ${MODAL_HEADER_HEIGHT}px -
${MODAL_FOOTER_HEIGHT}px)`};
}
}
}
- }
+ `}
`}
`;
@@ -221,6 +230,7 @@ const CustomModal = ({
const draggableRef = useRef<HTMLDivElement>(null);
const [bounds, setBounds] = useState<DraggableBounds>();
const [dragDisabled, setDragDisabled] = useState<boolean>(true);
+ const theme = useTheme();
const handleOnHide = () => {
openerRef?.current?.focus();
@@ -312,9 +322,13 @@ const CustomModal = ({
open={show}
title={<ModalTitle />}
closeIcon={
- <span data-test="close-modal-btn" className="close" aria-hidden="true">
- ×
- </span>
+ <Icons.CloseOutlined
+ iconColor={theme.colorText}
+ iconSize="l"
+ data-test="close-modal-btn"
+ className="close"
+ aria-hidden="true"
+ />
}
footer={!hideFooter ? modalFooter : null}
hideFooter={hideFooter}
diff --git a/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
b/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
index 1c503de955..3985ef952d 100644
--- a/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
+++ b/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx
@@ -70,6 +70,7 @@ export const ErrorAlert: React.FC<ErrorAlertProps> = ({
const preStyle = {
whiteSpace: 'pre-wrap',
fontFamily: theme.fontFamilyCode,
+ margin: `${theme.sizeUnit}px 0`,
};
const renderDescription = () => (
<div>
diff --git
a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
index 61e4d6c772..4bfa0cff85 100644
---
a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
+++
b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
@@ -173,7 +173,7 @@ test('Should have modal header', async () => {
await waitFor(() => {
expect(screen.getByText('Edit Chart Properties')).toBeVisible();
- expect(screen.getByText('×')).toBeVisible();
+ expect(screen.getByTestId('close-modal-btn')).toBeVisible();
expect(screen.getByRole('button', { name: 'Close' })).toBeVisible();
});
});
diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx
b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
index cbb6ced2a9..70484205aa 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
@@ -252,8 +252,9 @@ export type DBReducerActionType =
};
const StyledBtns = styled.div`
- margin-bottom: ${({ theme }) => theme.sizeUnit * 3}px;
- margin-left: ${({ theme }) => theme.sizeUnit * 3}px;
+ display: flex;
+ justify-content: center;
+ padding: ${({ theme }) => theme.sizeUnit * 5}px;
`;
export function dbReducer(
@@ -702,6 +703,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
// Test Connection logic
const testConnection = () => {
+ handleClearValidationErrors();
if (!db?.sqlalchemy_uri) {
addDangerToast(t('Please enter a SQLAlchemy URI to test'));
return;
@@ -728,10 +730,12 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
(errorMsg: string) => {
setTestInProgress(false);
addDangerToast(errorMsg);
+ setHasValidated(false);
},
(errorMsg: string) => {
setTestInProgress(false);
addSuccessToast(errorMsg);
+ setHasValidated(true);
},
);
};
@@ -761,6 +765,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
const handleClearValidationErrors = useCallback(() => {
setValidationErrors(null);
setHasValidated(false);
+ clearError();
}, [setValidationErrors, setHasValidated]);
const handleParametersChange = useCallback(
@@ -820,7 +825,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
const onSave = async () => {
let dbConfigExtraExtensionOnSaveError;
setLoading(true);
-
+ setHasValidated(false);
dbConfigExtraExtension
?.onSave(extraExtensionComponentState, db)
.then(({ error }: { error: any }) => {
@@ -1166,6 +1171,7 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
};
const handleBackButtonOnConnect = () => {
+ handleClearValidationErrors();
if (editNewDb) setHasConnectedDb(false);
if (importingModal) setImportingModal(false);
if (importErrored) {
@@ -1925,14 +1931,15 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
target,
}: {
target: HTMLInputElement;
- }) =>
+ }) => {
+ setHasValidated(false);
onChange(ActionType.InputChange, {
type: target.type,
name: target.name,
checked: target.checked,
value: target.value,
- })
- }
+ });
+ }}
conf={conf}
testConnection={testConnection}
testInProgress={testInProgress}
@@ -2156,7 +2163,8 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
<Button
data-test="sqla-connect-btn"
buttonStyle="link"
- onClick={() =>
+ onClick={() => {
+ handleClearValidationErrors();
setDB({
type: ActionType.ConfigMethodChange,
payload: {
@@ -2165,8 +2173,8 @@ const DatabaseModal:
FunctionComponent<DatabaseModalProps> = ({
ConfigurationMethod.SqlalchemyUri,
database_name: db.database_name,
},
- })
- }
+ });
+ }}
css={buttonLinkStyles}
>
{t(
