This is an automated email from the ASF dual-hosted git repository.
elizabeth 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 e6bd03fe98 fix(theming): solve modal dark theme issues + styling and
code improvements (#35539)
e6bd03fe98 is described below
commit e6bd03fe98cf576b7a1c98a0d7c7b437d158463c
Author: Gabriel Torres Ruiz <[email protected]>
AuthorDate: Tue Oct 14 14:08:18 2025 -0400
fix(theming): solve modal dark theme issues + styling and code improvements
(#35539)
---
.../components/ConfirmModal/ConfirmModal.test.tsx | 129 +++++
.../src/components/ConfirmModal/index.tsx | 87 ++++
.../superset-ui-core/src/components/index.ts | 1 +
.../src/SqlLab/components/ResultSet/index.tsx | 183 +++----
.../EmbeddedModal/EmbeddedModal.test.tsx | 122 +----
.../dashboard/components/EmbeddedModal/index.tsx | 149 +++---
.../dashboard/components/PropertiesModal/index.tsx | 13 +-
.../explore/components/ControlPanelsContainer.tsx | 19 +-
.../PropertiesModal/PropertiesModal.test.tsx | 1 +
.../explore/components/PropertiesModal/index.tsx | 27 +-
.../src/hooks/useConfirmModal/index.tsx | 78 +++
.../hooks/useConfirmModal/useConfirmModal.test.tsx | 196 ++++++++
.../useUnsavedChangesPrompt.test.tsx | 31 +-
.../src/pages/ThemeList/ThemeList.test.tsx | 524 +++++++++++++++------
.../src/pages/ThemeList/index.test.tsx | 288 -----------
superset-frontend/src/pages/ThemeList/index.tsx | 151 +++---
16 files changed, 1191 insertions(+), 808 deletions(-)
diff --git
a/superset-frontend/packages/superset-ui-core/src/components/ConfirmModal/ConfirmModal.test.tsx
b/superset-frontend/packages/superset-ui-core/src/components/ConfirmModal/ConfirmModal.test.tsx
new file mode 100644
index 0000000000..9a153a0899
--- /dev/null
+++
b/superset-frontend/packages/superset-ui-core/src/components/ConfirmModal/ConfirmModal.test.tsx
@@ -0,0 +1,129 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen, userEvent } from '@superset-ui/core/spec';
+import { ThemeProvider, supersetTheme } from '@superset-ui/core';
+import { ConfirmModal } from '.';
+
+const defaultProps = {
+ show: true,
+ onHide: jest.fn(),
+ onConfirm: jest.fn(),
+ title: 'Confirm Action',
+ body: 'Are you sure you want to proceed?',
+};
+
+const renderWithTheme = (component: React.ReactElement) =>
+ render(<ThemeProvider theme={supersetTheme}>{component}</ThemeProvider>);
+
+test('renders modal with title and body', () => {
+ renderWithTheme(<ConfirmModal {...defaultProps} />);
+
+ expect(screen.getByText('Confirm Action')).toBeInTheDocument();
+ expect(
+ screen.getByText('Are you sure you want to proceed?'),
+ ).toBeInTheDocument();
+});
+
+test('renders default confirm and cancel buttons', () => {
+ renderWithTheme(<ConfirmModal {...defaultProps} />);
+
+ expect(screen.getByRole('button', { name: 'Confirm' })).toBeInTheDocument();
+ expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument();
+});
+
+test('renders custom button text', () => {
+ renderWithTheme(
+ <ConfirmModal {...defaultProps} confirmText="Delete" cancelText="Keep" />,
+ );
+
+ expect(screen.getByRole('button', { name: 'Delete' })).toBeInTheDocument();
+ expect(screen.getByRole('button', { name: 'Keep' })).toBeInTheDocument();
+});
+
+test('calls onConfirm when confirm button is clicked', () => {
+ const onConfirm = jest.fn();
+ renderWithTheme(<ConfirmModal {...defaultProps} onConfirm={onConfirm} />);
+
+ userEvent.click(screen.getByRole('button', { name: 'Confirm' }));
+
+ expect(onConfirm).toHaveBeenCalledTimes(1);
+});
+
+test('calls onHide when cancel button is clicked', () => {
+ const onHide = jest.fn();
+ renderWithTheme(<ConfirmModal {...defaultProps} onHide={onHide} />);
+
+ userEvent.click(screen.getByRole('button', { name: 'Cancel' }));
+
+ expect(onHide).toHaveBeenCalledTimes(1);
+});
+
+test('renders danger button style', () => {
+ renderWithTheme(
+ <ConfirmModal {...defaultProps} confirmButtonStyle="danger" />,
+ );
+
+ const confirmButton = screen.getByRole('button', { name: 'Confirm' });
+ expect(confirmButton).toBeInTheDocument();
+});
+
+test('shows loading state on confirm button', () => {
+ renderWithTheme(<ConfirmModal {...defaultProps} loading />);
+
+ const confirmButton = screen.getByRole('button', { name: /Confirm/ });
+ expect(confirmButton).toBeInTheDocument();
+ expect(confirmButton).toHaveClass('ant-btn-loading');
+});
+
+test('disables buttons when loading', () => {
+ renderWithTheme(<ConfirmModal {...defaultProps} loading />);
+
+ const cancelButton = screen.getByRole('button', { name: 'Cancel' });
+ expect(cancelButton).toBeDisabled();
+});
+
+test('renders custom icon', () => {
+ const CustomIcon = () => <span data-test="custom-icon">!</span>;
+ renderWithTheme(<ConfirmModal {...defaultProps} icon={<CustomIcon />} />);
+
+ expect(screen.getByTestId('custom-icon')).toBeInTheDocument();
+});
+
+test('renders ReactNode as body', () => {
+ renderWithTheme(
+ <ConfirmModal
+ {...defaultProps}
+ body={
+ <div>
+ <p>Line 1</p>
+ <p>Line 2</p>
+ </div>
+ }
+ />,
+ );
+
+ expect(screen.getByText('Line 1')).toBeInTheDocument();
+ expect(screen.getByText('Line 2')).toBeInTheDocument();
+});
+
+test('does not render when show is false', () => {
+ renderWithTheme(<ConfirmModal {...defaultProps} show={false} />);
+
+ expect(screen.queryByText('Confirm Action')).not.toBeInTheDocument();
+});
diff --git
a/superset-frontend/packages/superset-ui-core/src/components/ConfirmModal/index.tsx
b/superset-frontend/packages/superset-ui-core/src/components/ConfirmModal/index.tsx
new file mode 100644
index 0000000000..ee26403ee5
--- /dev/null
+++
b/superset-frontend/packages/superset-ui-core/src/components/ConfirmModal/index.tsx
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { t, styled } from '@superset-ui/core';
+import { Icons, Modal, Typography, Button } from
'@superset-ui/core/components';
+import type { FC, ReactElement, ReactNode } from 'react';
+
+const IconWrapper = styled.span`
+ margin-right: ${({ theme }) => theme.sizeUnit * 2}px;
+`;
+
+const DEFAULT_ICON = <Icons.QuestionCircleOutlined iconSize="m" />;
+
+export type ConfirmModalProps = {
+ show: boolean;
+ onHide: () => void;
+ onConfirm: () => void;
+ title: string;
+ body: string | ReactNode;
+ confirmText?: string;
+ cancelText?: string;
+ confirmButtonStyle?: 'primary' | 'danger' | 'dashed';
+ icon?: ReactNode;
+ loading?: boolean;
+};
+
+export const ConfirmModal: FC<ConfirmModalProps> = ({
+ show,
+ onHide,
+ onConfirm,
+ title,
+ body,
+ confirmText = t('Confirm'),
+ cancelText = t('Cancel'),
+ confirmButtonStyle = 'primary',
+ icon = DEFAULT_ICON,
+ loading = false,
+}: ConfirmModalProps): ReactElement => (
+ <Modal
+ centered
+ responsive
+ onHide={onHide}
+ show={show}
+ width="600px"
+ title={
+ <>
+ <IconWrapper>{icon}</IconWrapper>
+ {title}
+ </>
+ }
+ footer={
+ <>
+ <Button buttonStyle="secondary" onClick={onHide} disabled={loading}>
+ {cancelText}
+ </Button>
+ <Button
+ buttonStyle={confirmButtonStyle}
+ onClick={onConfirm}
+ loading={loading}
+ >
+ {confirmText}
+ </Button>
+ </>
+ }
+ >
+ {typeof body === 'string' ? (
+ <Typography.Text>{body}</Typography.Text>
+ ) : (
+ body
+ )}
+ </Modal>
+);
diff --git
a/superset-frontend/packages/superset-ui-core/src/components/index.ts
b/superset-frontend/packages/superset-ui-core/src/components/index.ts
index 6590688076..f9b6dc217f 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/index.ts
+++ b/superset-frontend/packages/superset-ui-core/src/components/index.ts
@@ -66,6 +66,7 @@ export {
type CheckboxProps,
type CheckboxChangeEvent,
} from './Checkbox';
+export { ConfirmModal, type ConfirmModalProps } from './ConfirmModal';
export {
ColorPicker,
type ColorPickerProps,
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
index 643e4a712c..b90de2cafc 100644
--- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
+++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
@@ -35,7 +35,6 @@ import {
ButtonGroup,
Tooltip,
Card,
- Modal,
Input,
Label,
Loading,
@@ -87,6 +86,7 @@ import {
} from 'src/logger/LogUtils';
import { Icons } from '@superset-ui/core/components/Icons';
import { findPermission } from 'src/utils/findPermission';
+import { useConfirmModal } from 'src/hooks/useConfirmModal';
import ExploreCtasResultsButton from '../ExploreCtasResultsButton';
import ExploreResultsButton from '../ExploreResultsButton';
import HighlightedSql from '../HighlightedSql';
@@ -156,12 +156,14 @@ const ResultSetButtons = styled.div`
padding-right: ${({ theme }) => 2 * theme.sizeUnit}px;
`;
-const copyButtonStyles = css`
+const CopyStyledButton = styled(Button)`
&:hover {
+ color: ${({ theme }) => theme.colorPrimary};
text-decoration: unset;
}
+
span > :first-of-type {
- margin: 0px;
+ margin: 0;
}
`;
@@ -226,6 +228,7 @@ const ResultSet = ({
const history = useHistory();
const dispatch = useDispatch();
const logAction = useLogAction({ queryId, sqlEditorId: query.sqlEditorId });
+ const { showConfirm, ConfirmModal } = useConfirmModal();
const reRunQueryIfSessionTimeoutErrorOnMount = useCallback(() => {
if (
@@ -302,7 +305,7 @@ const ResultSet = ({
const renderControls = () => {
if (search || visualize || csv) {
- const { results, queryLimit, limitingFactor, rows } = query;
+ const { limitingFactor, queryLimit, results, rows } = query;
const limit = queryLimit || results.query.limit;
const rowsCount = Math.min(rows || 0, results?.data?.length || 0);
let { data } = query.results;
@@ -310,7 +313,6 @@ const ResultSet = ({
data = cachedData;
}
const { columns } = query.results;
- // Added compute logic to stop user from being able to Save & Explore
const datasource: ISaveableDatasource = {
columns: query.results.columns as ISimpleColumn[],
@@ -327,6 +329,27 @@ const ResultSet = ({
user?.roles,
);
+ const handleDownloadCsv = (event: React.MouseEvent<HTMLElement>) => {
+ logAction(LOG_ACTIONS_SQLLAB_DOWNLOAD_CSV, {});
+
+ if (limitingFactor === LimitingFactor.Dropdown && limit === rowsCount)
{
+ event.preventDefault();
+
+ showConfirm({
+ title: t('Download is on the way'),
+ body: t(
+ 'Downloading %(rows)s rows based on the LIMIT configuration. If
you want the entire result set, you need to adjust the LIMIT.',
+ { rows: rowsCount.toLocaleString() },
+ ),
+ onConfirm: () => {
+ window.location.href = getExportCsvUrl(query.id);
+ },
+ confirmText: t('OK'),
+ cancelText: t('Close'),
+ });
+ }
+ };
+
return (
<ResultSetControls>
<SaveDatasetModal
@@ -347,45 +370,28 @@ const ResultSet = ({
/>
)}
{csv && canExportData && (
- <Button
- css={copyButtonStyles}
+ <CopyStyledButton
buttonSize="small"
buttonStyle="secondary"
href={getExportCsvUrl(query.id)}
data-test="export-csv-button"
- onClick={() => {
- logAction(LOG_ACTIONS_SQLLAB_DOWNLOAD_CSV, {});
- if (
- limitingFactor === LimitingFactor.Dropdown &&
- limit === rowsCount
- ) {
- Modal.warning({
- title: t('Download is on the way'),
- content: t(
- 'Downloading %(rows)s rows based on the LIMIT
configuration. If you want the entire result set, you need to adjust the
LIMIT.',
- { rows: rowsCount.toLocaleString() },
- ),
- });
- }
- }}
+ onClick={handleDownloadCsv}
>
<Icons.DownloadOutlined iconSize="m" /> {t('Download to CSV')}
- </Button>
+ </CopyStyledButton>
)}
-
{canExportData && (
<CopyToClipboard
text={prepareCopyToClipboardTabularData(data, columns)}
wrapped={false}
copyNode={
- <Button
- css={copyButtonStyles}
+ <CopyStyledButton
buttonSize="small"
buttonStyle="secondary"
data-test="copy-to-clipboard-button"
>
<Icons.CopyOutlined iconSize="s" /> {t('Copy to
Clipboard')}
- </Button>
+ </CopyStyledButton>
}
hideTooltip
onCopyEnd={() =>
@@ -653,68 +659,71 @@ const ResultSet = ({
true,
);
return (
- <ResultContainer>
- {renderControls()}
- {showSql && showSqlInline ? (
- <>
- <div
- css={css`
- display: flex;
- justify-content: space-between;
- align-items: center;
- gap: ${GAP}px;
- `}
- >
- <Card
- css={[
- css`
- height: 28px;
- width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
- code {
- width: 100%;
- overflow: hidden;
- white-space: nowrap !important;
- text-overflow: ellipsis;
- display: block;
- }
- `,
- ]}
+ <>
+ <ResultContainer>
+ {renderControls()}
+ {showSql && showSqlInline ? (
+ <>
+ <div
+ css={css`
+ display: flex;
+ justify-content: space-between;
+ align-items: center;
+ gap: ${GAP}px;
+ `}
>
- {sql}
- </Card>
+ <Card
+ css={[
+ css`
+ height: 28px;
+ width: calc(100% - ${ROWS_CHIP_WIDTH + GAP}px);
+ code {
+ width: 100%;
+ overflow: hidden;
+ white-space: nowrap !important;
+ text-overflow: ellipsis;
+ display: block;
+ }
+ `,
+ ]}
+ >
+ {sql}
+ </Card>
+ {renderRowsReturned(false)}
+ </div>
+ {renderRowsReturned(true)}
+ </>
+ ) : (
+ <>
{renderRowsReturned(false)}
- </div>
- {renderRowsReturned(true)}
- </>
- ) : (
- <>
- {renderRowsReturned(false)}
- {renderRowsReturned(true)}
- {sql}
- </>
- )}
- <div
- css={css`
- flex: 1 1 auto;
- `}
- >
- <AutoSizer disableWidth>
- {({ height }) => (
- <ResultTable
- data={data}
- queryId={query.id}
- orderedColumnKeys={results.columns.map(
- col => col.column_name,
- )}
- height={height}
- filterText={searchText}
- expandedColumns={expandedColumns}
- allowHTML={allowHTML}
- />
- )}
- </AutoSizer>
- </div>
- </ResultContainer>
+ {renderRowsReturned(true)}
+ {sql}
+ </>
+ )}
+ <div
+ css={css`
+ flex: 1 1 auto;
+ `}
+ >
+ <AutoSizer disableWidth>
+ {({ height }) => (
+ <ResultTable
+ data={data}
+ queryId={query.id}
+ orderedColumnKeys={results.columns.map(
+ col => col.column_name,
+ )}
+ height={height}
+ filterText={searchText}
+ expandedColumns={expandedColumns}
+ allowHTML={allowHTML}
+ />
+ )}
+ </AutoSizer>
+ </div>
+ </ResultContainer>
+ {ConfirmModal}
+ </>
);
}
if (data && data.length === 0) {
diff --git
a/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx
b/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx
index b4a2b5aa19..492654ede3 100644
---
a/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx
+++
b/superset-frontend/src/dashboard/components/EmbeddedModal/EmbeddedModal.test.tsx
@@ -16,11 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
-
import {
render,
screen,
- fireEvent,
+ userEvent,
waitFor,
} from 'spec/helpers/testing-library';
import {
@@ -28,9 +27,8 @@ import {
getExtensionsRegistry,
makeApi,
} from '@superset-ui/core';
-import { Modal } from '@superset-ui/core/components';
import setupCodeOverrides from 'src/setup/setupCodeOverrides';
-import DashboardEmbedModal from './index';
+import DashboardEmbedModal from '.';
const defaultResponse = {
result: { uuid: 'uuid', dashboard_id: '1', allowed_domains: ['example.com']
},
@@ -58,16 +56,16 @@ const setMockApiNotFound = () => {
};
const setup = () => {
- resetMockApi();
render(<DashboardEmbedModal {...defaultProps} />, { useRedux: true });
};
beforeEach(() => {
jest.clearAllMocks();
resetMockApi();
+ getExtensionsRegistry().set('embedded.modal', undefined);
});
-test('renders', async () => {
+test('renders the embed modal', async () => {
setup();
expect(await screen.findByText('Embed')).toBeInTheDocument();
});
@@ -79,15 +77,15 @@ test('renders loading state', async () => {
});
});
-test('renders the modal default content', async () => {
- render(<DashboardEmbedModal {...defaultProps} />, { useRedux: true });
+test('renders modal content with settings', async () => {
+ setup();
expect(await screen.findByText('Settings')).toBeInTheDocument();
expect(
screen.getByText(new RegExp(/Allowed Domains/, 'i')),
).toBeInTheDocument();
});
-test('renders the correct actions when dashboard is ready to embed', async ()
=> {
+test('shows Deactivate and Save changes buttons when ready to embed', async ()
=> {
setup();
expect(
await screen.findByRole('button', { name: 'Deactivate' }),
@@ -97,7 +95,7 @@ test('renders the correct actions when dashboard is ready to
embed', async () =>
).toBeInTheDocument();
});
-test('renders the correct actions when dashboard is not ready to embed', async
() => {
+test('shows Enable embedding button when not ready to embed', async () => {
setMockApiNotFound();
render(<DashboardEmbedModal {...defaultProps} />, { useRedux: true });
expect(
@@ -105,7 +103,7 @@ test('renders the correct actions when dashboard is not
ready to embed', async (
).toBeInTheDocument();
});
-test('enables embedding', async () => {
+test('enables embedding when Enable embedding button is clicked', async () => {
setMockApiNotFound();
render(<DashboardEmbedModal {...defaultProps} />, { useRedux: true });
@@ -115,33 +113,33 @@ test('enables embedding', async () => {
expect(enableEmbed).toBeInTheDocument();
resetMockApi();
- fireEvent.click(enableEmbed);
+ await userEvent.click(enableEmbed);
expect(
await screen.findByRole('button', { name: 'Deactivate' }),
).toBeInTheDocument();
});
-test('shows and hides the confirmation modal on deactivation', async () => {
+test('shows and hides confirmation alert when deactivating', async () => {
setup();
const deactivate = await screen.findByRole('button', { name: 'Deactivate' });
- fireEvent.click(deactivate);
+ await userEvent.click(deactivate);
expect(await screen.findByText('Disable embedding?')).toBeInTheDocument();
expect(
screen.getByText('This will remove your current embed configuration.'),
).toBeInTheDocument();
- const okBtn = screen.getByRole('button', { name: 'OK' });
- fireEvent.click(okBtn);
+ const confirmBtn = screen.getByRole('button', { name: 'Deactivate' });
+ await userEvent.click(confirmBtn);
await waitFor(() => {
expect(screen.queryByText('Disable embedding?')).not.toBeInTheDocument();
});
});
-test('enables the "Save Changes" button', async () => {
+test('enables Save Changes button when allowed domains are modified', async ()
=> {
setup();
const allowedDomainsInput = await screen.findByRole('textbox', {
@@ -153,11 +151,12 @@ test('enables the "Save Changes" button', async () => {
expect(saveChangesBtn).toBeDisabled();
expect(allowedDomainsInput).toBeInTheDocument();
- fireEvent.change(allowedDomainsInput, { target: { value: 'test.com' } });
+ await userEvent.clear(allowedDomainsInput);
+ await userEvent.type(allowedDomainsInput, 'test.com');
expect(saveChangesBtn).toBeEnabled();
});
-test('adds extension to DashboardEmbedModal', async () => {
+test('renders extension component when registered', async () => {
const extensionsRegistry = getExtensionsRegistry();
extensionsRegistry.set('embedded.modal', () => (
@@ -165,7 +164,7 @@ test('adds extension to DashboardEmbedModal', async () => {
));
setupCodeOverrides();
- render(<DashboardEmbedModal {...defaultProps} />, { useRedux: true });
+ setup();
expect(
await screen.findByText('dashboard.embed.modal.extension component'),
@@ -173,86 +172,3 @@ test('adds extension to DashboardEmbedModal', async () => {
extensionsRegistry.set('embedded.modal', undefined);
});
-
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('Modal.useModal integration', () => {
- beforeEach(() => {
- jest.clearAllMocks();
- });
-
- test('uses Modal.useModal hook for confirmation dialogs', () => {
- const useModalSpy = jest.spyOn(Modal, 'useModal');
- setup();
-
- // Verify that useModal is called when the component mounts
- expect(useModalSpy).toHaveBeenCalled();
-
- useModalSpy.mockRestore();
- });
-
- test('renders contextHolder for proper theming', async () => {
- const { container } = render(<DashboardEmbedModal {...defaultProps} />, {
- useRedux: true,
- });
-
- // Wait for component to be rendered
- await screen.findByText('Embed');
-
- // The contextHolder is rendered in the component tree
- // Check that modal root elements exist for theming
- const modalRootElements = container.querySelectorAll('.ant-modal-root');
- expect(modalRootElements).toBeDefined();
- });
-
- test('confirmation modal inherits theme context', async () => {
- setup();
-
- // Click deactivate to trigger the confirmation modal
- const deactivate = await screen.findByRole('button', {
- name: 'Deactivate',
- });
- fireEvent.click(deactivate);
-
- // Wait for the modal to appear
- const modalTitle = await screen.findByText('Disable embedding?');
- expect(modalTitle).toBeInTheDocument();
-
- // Check that the modal is rendered within the component tree (not on body
directly)
- const modal = modalTitle.closest('.ant-modal-wrap');
- expect(modal).toBeInTheDocument();
- });
-
- test('does not use Modal.confirm directly', () => {
- // Spy on the static Modal.confirm method
- const confirmSpy = jest.spyOn(Modal, 'confirm');
-
- setup();
-
- // The component should not call Modal.confirm directly
- expect(confirmSpy).not.toHaveBeenCalled();
-
- confirmSpy.mockRestore();
- });
-
- test('modal actions work correctly with useModal', async () => {
- setup();
-
- // Click deactivate
- const deactivate = await screen.findByRole('button', {
- name: 'Deactivate',
- });
- fireEvent.click(deactivate);
-
- // Modal should appear
- expect(await screen.findByText('Disable embedding?')).toBeInTheDocument();
-
- // Click Cancel to close modal
- const cancelBtn = screen.getByRole('button', { name: 'Cancel' });
- fireEvent.click(cancelBtn);
-
- // Modal should close
- await waitFor(() => {
- expect(screen.queryByText('Disable embedding?')).not.toBeInTheDocument();
- });
- });
-});
diff --git a/superset-frontend/src/dashboard/components/EmbeddedModal/index.tsx
b/superset-frontend/src/dashboard/components/EmbeddedModal/index.tsx
index 1928c9d9a4..1ad5fec6ad 100644
--- a/superset-frontend/src/dashboard/components/EmbeddedModal/index.tsx
+++ b/superset-frontend/src/dashboard/components/EmbeddedModal/index.tsx
@@ -33,6 +33,8 @@ import {
Modal,
Loading,
Form,
+ Alert,
+ Space,
} from '@superset-ui/core/components';
import { useToasts } from 'src/components/MessageToasts/withToasts';
import { EmbeddedDashboard } from 'src/dashboard/types';
@@ -64,9 +66,7 @@ export const DashboardEmbedControls = ({ dashboardId, onHide
}: Props) => {
const [loading, setLoading] = useState(false); // whether we are currently
doing an async thing
const [embedded, setEmbedded] = useState<EmbeddedDashboard | null>(null); //
the embedded dashboard config
const [allowedDomains, setAllowedDomains] = useState<string>('');
-
- // Use Modal.useModal hook to ensure proper theming
- const [modal, contextHolder] = Modal.useModal();
+ const [showDeactivateConfirm, setShowDeactivateConfirm] = useState(false);
const endpoint = `/api/v1/dashboard/${dashboardId}/embedded`;
// whether saveable changes have been made to the config
@@ -103,35 +103,33 @@ export const DashboardEmbedControls = ({ dashboardId,
onHide }: Props) => {
}, [endpoint, allowedDomains]);
const disableEmbedded = useCallback(() => {
- modal.confirm({
- title: t('Disable embedding?'),
- content: t('This will remove your current embed configuration.'),
- okType: 'danger',
- onOk: () => {
- setLoading(true);
- makeApi<{}>({ method: 'DELETE', endpoint })({})
- .then(
- () => {
- setEmbedded(null);
- setAllowedDomains('');
- addInfoToast(t('Embedding deactivated.'));
- onHide();
- },
- err => {
- console.error(err);
- addDangerToast(
- t(
- 'Sorry, something went wrong. Embedding could not be
deactivated.',
- ),
- );
- },
- )
- .finally(() => {
- setLoading(false);
- });
- },
- });
- }, [endpoint, modal]);
+ setShowDeactivateConfirm(true);
+ }, []);
+
+ const confirmDeactivate = useCallback(() => {
+ setLoading(true);
+ makeApi<{}>({ method: 'DELETE', endpoint })({})
+ .then(
+ () => {
+ setEmbedded(null);
+ setAllowedDomains('');
+ setShowDeactivateConfirm(false);
+ addInfoToast(t('Embedding deactivated.'));
+ onHide();
+ },
+ err => {
+ console.error(err);
+ addDangerToast(
+ t(
+ 'Sorry, something went wrong. Embedding could not be
deactivated.',
+ ),
+ );
+ },
+ )
+ .finally(() => {
+ setLoading(false);
+ });
+ }, [endpoint, addInfoToast, addDangerToast, onHide]);
useEffect(() => {
setReady(false);
@@ -170,7 +168,6 @@ export const DashboardEmbedControls = ({ dashboardId,
onHide }: Props) => {
return (
<>
- {contextHolder}
{embedded ? (
DocsConfigDetails ? (
<DocsConfigDetails embeddedId={embedded.uuid} />
@@ -222,39 +219,71 @@ export const DashboardEmbedControls = ({ dashboardId,
onHide }: Props) => {
/>
</FormItem>
</Form>
- <ButtonRow
- css={theme => css`
- margin-top: ${theme.margin}px;
- `}
- >
- {embedded ? (
- <>
- <Button
- onClick={disableEmbedded}
- buttonStyle="secondary"
- loading={loading}
- >
- {t('Deactivate')}
- </Button>
+ {showDeactivateConfirm ? (
+ <Alert
+ closable={false}
+ type="warning"
+ message={t('Disable embedding?')}
+ description={t('This will remove your current embed configuration.')}
+ css={{
+ textAlign: 'left',
+ marginTop: '16px',
+ }}
+ action={
+ <Space>
+ <Button
+ key="cancel"
+ buttonStyle="secondary"
+ onClick={() => setShowDeactivateConfirm(false)}
+ >
+ {t('Cancel')}
+ </Button>
+ <Button
+ key="deactivate"
+ buttonStyle="danger"
+ onClick={confirmDeactivate}
+ loading={loading}
+ >
+ {t('Deactivate')}
+ </Button>
+ </Space>
+ }
+ />
+ ) : (
+ <ButtonRow
+ css={theme => css`
+ margin-top: ${theme.margin}px;
+ `}
+ >
+ {embedded ? (
+ <>
+ <Button
+ onClick={disableEmbedded}
+ buttonStyle="secondary"
+ loading={loading}
+ >
+ {t('Deactivate')}
+ </Button>
+ <Button
+ onClick={enableEmbedded}
+ buttonStyle="primary"
+ disabled={!isDirty}
+ loading={loading}
+ >
+ {t('Save changes')}
+ </Button>
+ </>
+ ) : (
<Button
onClick={enableEmbedded}
buttonStyle="primary"
- disabled={!isDirty}
loading={loading}
>
- {t('Save changes')}
+ {t('Enable embedding')}
</Button>
- </>
- ) : (
- <Button
- onClick={enableEmbedded}
- buttonStyle="primary"
- loading={loading}
- >
- {t('Enable embedding')}
- </Button>
- )}
- </ButtonRow>
+ )}
+ </ButtonRow>
+ )}
</>
);
};
diff --git
a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
index 614f042ba5..cf3015b5e6 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
@@ -21,7 +21,6 @@ import { omit } from 'lodash';
import jsonStringify from 'json-stringify-pretty-compact';
import {
Form,
- Modal,
Collapse,
CollapseLabelInModal,
JsonEditor,
@@ -151,11 +150,7 @@ const PropertiesModal = ({
}
}
- Modal.error({
- title: t('Error'),
- content: errorText,
- okButtonProps: { danger: true, className: 'btn-danger' },
- });
+ addDangerToast(errorText);
};
const handleDashboardData = useCallback(
@@ -267,11 +262,7 @@ const PropertiesModal = ({
// only fire if the color_scheme is present and invalid
if (colorScheme && !colorChoices.includes(colorScheme)) {
- Modal.error({
- title: t('Error'),
- content: t('A valid color scheme is required'),
- okButtonProps: { danger: true, className: 'btn-danger' },
- });
+ addDangerToast(t('A valid color scheme is required'));
onHide();
throw new Error('A valid color scheme is required');
}
diff --git
a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
index 7e8d5f147c..6a53656077 100644
--- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
+++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx
@@ -59,13 +59,13 @@ import { kebabCase, isEqual } from 'lodash';
import {
Collapse,
- Modal,
Loading,
Label,
Tooltip,
} from '@superset-ui/core/components';
import Tabs from '@superset-ui/core/components/Tabs';
import { PluginContext } from 'src/components';
+import { useConfirmModal } from 'src/hooks/useConfirmModal';
import { getSectionsToRender } from 'src/explore/controlUtils';
import { ExploreActions } from 'src/explore/actions/exploreActions';
@@ -285,7 +285,7 @@ function useResetOnChangeRef(initialValue: () => any,
resetOnChangeValue: any) {
export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
const theme = useTheme();
const pluginContext = useContext(PluginContext);
- const [modal, contextHolder] = Modal.useModal();
+ const { showConfirm, ConfirmModal } = useConfirmModal();
const prevState = usePrevious(props.exploreState);
const prevDatasource = usePrevious(props.exploreState.datasource);
@@ -323,12 +323,12 @@ export const ControlPanelsContainer = (props:
ControlPanelsContainerProps) => {
filter.subject === x_axis,
);
if (noFilter) {
- modal.confirm({
+ showConfirm({
title: t('The X-axis is not on the filters list'),
- content:
- t(`The X-axis is not on the filters list which will prevent it
from being used in
- time range filters in dashboards. Would you like to add it to the
filters list?`),
- onOk: () => {
+ body: t(
+ `The X-axis is not on the filters list which will prevent it from
being used in time range filters in dashboards. Would you like to add it to the
filters list?`,
+ ),
+ onConfirm: () => {
setControlValue('adhoc_filters', [
...(adhoc_filters || []),
{
@@ -340,6 +340,8 @@ export const ControlPanelsContainer = (props:
ControlPanelsContainerProps) => {
},
]);
},
+ confirmText: t('Yes'),
+ cancelText: t('No'),
});
}
}
@@ -350,6 +352,7 @@ export const ControlPanelsContainer = (props:
ControlPanelsContainerProps) => {
defaultTimeFilter,
previousXAxis,
props.exploreState.datasource,
+ showConfirm,
]);
useEffect(() => {
@@ -851,7 +854,6 @@ export const ControlPanelsContainer = (props:
ControlPanelsContainerProps) => {
return (
<>
- {contextHolder}
<Styles ref={containerRef}>
<Tabs
id="controlSections"
@@ -959,6 +961,7 @@ export const ControlPanelsContainer = (props:
ControlPanelsContainerProps) => {
/>
</div>
</Styles>
+ {ConfirmModal}
</>
);
};
diff --git
a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
index 5bbc04563a..dc50d430c8 100644
---
a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
+++
b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx
@@ -47,6 +47,7 @@ const createProps = () =>
onHide: jest.fn(),
onSave: jest.fn(),
addSuccessToast: jest.fn(),
+ addDangerToast: jest.fn(),
}) as PropertiesModalProps;
fetchMock.get('glob:*/api/v1/chart/318*', {
diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx
b/superset-frontend/src/explore/components/PropertiesModal/index.tsx
index efcf69912c..4ff4637a29 100644
--- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx
+++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx
@@ -21,7 +21,6 @@ import { ChangeEvent, useMemo, useState, useCallback,
useEffect } from 'react';
import {
Input,
AsyncSelect,
- Modal,
Collapse,
CollapseLabelInModal,
type SelectValue,
@@ -54,6 +53,7 @@ export type PropertiesModalProps = {
permissionsError?: string;
existingOwners?: SelectValue;
addSuccessToast: (msg: string) => void;
+ addDangerToast: (msg: string) => void;
};
function PropertiesModal({
@@ -62,6 +62,7 @@ function PropertiesModal({
onSave,
show,
addSuccessToast,
+ addDangerToast,
}: PropertiesModalProps) {
const [submitting, setSubmitting] = useState(false);
// values of form inputs
@@ -133,17 +134,17 @@ function PropertiesModal({
return selectTags;
}, [tags.length]);
- function showError({ error, statusText, message }: any) {
- let errorText = error || statusText || t('An error has occurred');
- if (message === 'Forbidden') {
- errorText = t('You do not have permission to edit this chart');
- }
- Modal.error({
- title: t('Error'),
- content: errorText,
- okButtonProps: { danger: true, className: 'btn-danger' },
- });
- }
+ const showError = useCallback(
+ ({ error, statusText, message }: any) => {
+ let errorText = error || statusText || t('An error has occurred');
+ if (message === 'Forbidden') {
+ errorText = t('You do not have permission to edit this chart');
+ }
+
+ addDangerToast(errorText);
+ },
+ [addDangerToast, t],
+ );
const fetchChartProperties = useCallback(
async function fetchChartProperties() {
@@ -179,7 +180,7 @@ function PropertiesModal({
showError(clientError);
}
},
- [slice.slice_id],
+ [showError, slice.slice_id],
);
const loadOptions = useMemo(
diff --git a/superset-frontend/src/hooks/useConfirmModal/index.tsx
b/superset-frontend/src/hooks/useConfirmModal/index.tsx
new file mode 100644
index 0000000000..e28eb91977
--- /dev/null
+++ b/superset-frontend/src/hooks/useConfirmModal/index.tsx
@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useState, useCallback, ReactNode } from 'react';
+import { ConfirmModal } from '@superset-ui/core/components';
+
+export interface ConfirmConfig {
+ title: string;
+ body: string | ReactNode;
+ onConfirm: () => void | Promise<void>;
+ confirmText?: string;
+ cancelText?: string;
+ confirmButtonStyle?: 'primary' | 'danger' | 'dashed';
+ icon?: ReactNode;
+}
+
+export const useConfirmModal = () => {
+ const [config, setConfig] = useState<ConfirmConfig | null>(null);
+ const [loading, setLoading] = useState(false);
+
+ const showConfirm = useCallback((options: ConfirmConfig) => {
+ setConfig(options);
+ }, []);
+
+ const handleHide = useCallback(() => {
+ if (!loading) {
+ setConfig(null);
+ }
+ }, [loading]);
+
+ const handleConfirm = useCallback(async () => {
+ if (!config) return;
+
+ try {
+ setLoading(true);
+ await config.onConfirm();
+ setConfig(null);
+ } catch (error) {
+ // Let the error propagate but keep modal open
+ // eslint-disable-next-line no-console
+ console.error('Confirm action failed:', error);
+ } finally {
+ setLoading(false);
+ }
+ }, [config]);
+
+ const ConfirmModalComponent = config ? (
+ <ConfirmModal
+ show={!!config}
+ onHide={handleHide}
+ onConfirm={handleConfirm}
+ title={config.title}
+ body={config.body}
+ confirmText={config.confirmText}
+ cancelText={config.cancelText}
+ confirmButtonStyle={config.confirmButtonStyle}
+ icon={config.icon}
+ loading={loading}
+ />
+ ) : null;
+
+ return { showConfirm, ConfirmModal: ConfirmModalComponent };
+};
diff --git
a/superset-frontend/src/hooks/useConfirmModal/useConfirmModal.test.tsx
b/superset-frontend/src/hooks/useConfirmModal/useConfirmModal.test.tsx
new file mode 100644
index 0000000000..351e0af2ec
--- /dev/null
+++ b/superset-frontend/src/hooks/useConfirmModal/useConfirmModal.test.tsx
@@ -0,0 +1,196 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen, userEvent, waitFor } from '@superset-ui/core/spec';
+import { renderHook } from '@testing-library/react-hooks';
+import { ThemeProvider, supersetTheme } from '@superset-ui/core';
+import { useConfirmModal } from '.';
+
+const renderWithTheme = (component: React.ReactElement) =>
+ render(<ThemeProvider theme={supersetTheme}>{component}</ThemeProvider>);
+
+test('initially returns null ConfirmModal', () => {
+ const { result } = renderHook(() => useConfirmModal());
+
+ expect(result.current.ConfirmModal).toBeNull();
+});
+
+test('showConfirm creates modal with config', () => {
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Test Title',
+ body: 'Test Body',
+ onConfirm: jest.fn(),
+ });
+
+ expect(result.current.ConfirmModal).not.toBeNull();
+});
+
+test('renders modal when showConfirm is called', () => {
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Delete Item',
+ body: 'Are you sure you want to delete this item?',
+ onConfirm: jest.fn(),
+ });
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ expect(screen.getByText('Delete Item')).toBeInTheDocument();
+ expect(
+ screen.getByText('Are you sure you want to delete this item?'),
+ ).toBeInTheDocument();
+});
+
+test('calls onConfirm when confirm button is clicked', async () => {
+ const onConfirm = jest.fn();
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Confirm',
+ body: 'Proceed?',
+ onConfirm,
+ });
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ await userEvent.click(screen.getByRole('button', { name: 'Confirm' }));
+
+ await waitFor(() => {
+ expect(onConfirm).toHaveBeenCalledTimes(1);
+ });
+});
+
+test('handles async onConfirm', async () => {
+ const onConfirm = jest.fn().mockResolvedValue(undefined);
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Async Action',
+ body: 'This will take some time',
+ onConfirm,
+ });
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ await userEvent.click(screen.getByRole('button', { name: /Confirm/ }));
+
+ await waitFor(() => {
+ expect(onConfirm).toHaveBeenCalled();
+ });
+});
+
+test('closes modal on cancel', async () => {
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Test',
+ body: 'Test',
+ onConfirm: jest.fn(),
+ });
+
+ expect(result.current.ConfirmModal).not.toBeNull();
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ await userEvent.click(screen.getByRole('button', { name: 'Cancel' }));
+
+ await waitFor(() => {
+ expect(result.current.ConfirmModal).toBeNull();
+ });
+});
+
+test('supports custom button text', () => {
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Delete',
+ body: 'Remove this?',
+ onConfirm: jest.fn(),
+ confirmText: 'Remove',
+ cancelText: 'Keep',
+ });
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ expect(screen.getByRole('button', { name: 'Remove' })).toBeInTheDocument();
+ expect(screen.getByRole('button', { name: 'Keep' })).toBeInTheDocument();
+});
+
+test('supports danger button style', () => {
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Delete',
+ body: 'This is dangerous',
+ onConfirm: jest.fn(),
+ confirmButtonStyle: 'danger',
+ });
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ const confirmButton = screen.getByRole('button', { name: 'Confirm' });
+ expect(confirmButton).toHaveClass('superset-button-danger');
+});
+
+test('handles errors in onConfirm gracefully', async () => {
+ const consoleError = jest.spyOn(console, 'error').mockImplementation();
+ const onConfirm = jest.fn().mockRejectedValue(new Error('Test error'));
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Error Test',
+ body: 'This will fail',
+ onConfirm,
+ });
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ await userEvent.click(screen.getByRole('button', { name: /Confirm/ }));
+
+ await waitFor(() => {
+ expect(consoleError).toHaveBeenCalled();
+ expect(result.current.ConfirmModal).not.toBeNull(); // Modal stays open on
error
+ });
+
+ consoleError.mockRestore();
+});
+
+test('closes modal after successful confirm', async () => {
+ const onConfirm = jest.fn().mockResolvedValue(undefined);
+ const { result } = renderHook(() => useConfirmModal());
+
+ result.current.showConfirm({
+ title: 'Success Test',
+ body: 'This will succeed',
+ onConfirm,
+ });
+
+ expect(result.current.ConfirmModal).not.toBeNull();
+
+ renderWithTheme(<>{result.current.ConfirmModal}</>);
+
+ await userEvent.click(screen.getByRole('button', { name: /Confirm/ }));
+
+ await waitFor(() => {
+ expect(onConfirm).toHaveBeenCalled();
+ expect(result.current.ConfirmModal).toBeNull();
+ });
+});
diff --git
a/superset-frontend/src/hooks/useUnsavedChangesPrompt/useUnsavedChangesPrompt.test.tsx
b/superset-frontend/src/hooks/useUnsavedChangesPrompt/useUnsavedChangesPrompt.test.tsx
index 37cde732cd..444b53197d 100644
---
a/superset-frontend/src/hooks/useUnsavedChangesPrompt/useUnsavedChangesPrompt.test.tsx
+++
b/superset-frontend/src/hooks/useUnsavedChangesPrompt/useUnsavedChangesPrompt.test.tsx
@@ -17,10 +17,9 @@
* under the License.
*/
import { renderHook } from '@testing-library/react-hooks';
-import { useUnsavedChangesPrompt } from 'src/hooks/useUnsavedChangesPrompt';
import { Router } from 'react-router-dom';
import { createMemoryHistory } from 'history';
-import { act } from 'spec/helpers/testing-library';
+import { useUnsavedChangesPrompt } from '.';
let history = createMemoryHistory({
initialEntries: ['/dashboard'],
@@ -58,11 +57,9 @@ test('should block navigation and show modal if there are
unsaved changes', () =
);
// Simulate blocked navigation
- act(() => {
- const unblock = history.block((tx: any) => tx);
- unblock();
- history.push('/another-page');
- });
+ const unblock = history.block((tx: any) => tx);
+ unblock();
+ history.push('/another-page');
expect(result.current.showModal).toBe(true);
});
@@ -79,9 +76,7 @@ test('should trigger onSave and hide modal on
handleSaveAndCloseModal', async ()
{ wrapper },
);
- await act(async () => {
- await result.current.handleSaveAndCloseModal();
- });
+ await result.current.handleSaveAndCloseModal();
expect(onSave).toHaveBeenCalled();
expect(result.current.showModal).toBe(false);
@@ -99,9 +94,7 @@ test('should trigger manual save and not show modal again',
async () => {
{ wrapper },
);
- act(() => {
- result.current.triggerManualSave();
- });
+ result.current.triggerManualSave();
expect(onSave).toHaveBeenCalled();
expect(result.current.showModal).toBe(false);
@@ -120,18 +113,14 @@ test('should close modal when handleConfirmNavigation is
called', () => {
);
// First, trigger navigation to show the modal
- act(() => {
- const unblock = history.block((tx: any) => tx);
- unblock();
- history.push('/another-page');
- });
+ const unblock = history.block((tx: any) => tx);
+ unblock();
+ history.push('/another-page');
expect(result.current.showModal).toBe(true);
// Then call handleConfirmNavigation to discard changes
- act(() => {
- result.current.handleConfirmNavigation();
- });
+ result.current.handleConfirmNavigation();
expect(result.current.showModal).toBe(false);
});
diff --git a/superset-frontend/src/pages/ThemeList/ThemeList.test.tsx
b/superset-frontend/src/pages/ThemeList/ThemeList.test.tsx
index d455cf7f78..eb52646775 100644
--- a/superset-frontend/src/pages/ThemeList/ThemeList.test.tsx
+++ b/superset-frontend/src/pages/ThemeList/ThemeList.test.tsx
@@ -16,51 +16,97 @@
* specific language governing permissions and limitations
* under the License.
*/
-import fetchMock from 'fetch-mock';
import {
render,
screen,
- fireEvent,
+ userEvent,
waitFor,
} from 'spec/helpers/testing-library';
-import { Modal } from '@superset-ui/core/components';
+import fetchMock from 'fetch-mock';
+import * as hooks from 'src/views/CRUD/hooks';
+import { useThemeContext } from 'src/theme/ThemeProvider';
import ThemesList from './index';
-const themesInfoEndpoint = 'glob:*/api/v1/theme/_info*';
-const themesEndpoint = 'glob:*/api/v1/theme/?*';
-const themeEndpoint = 'glob:*/api/v1/theme/*';
+// Mock the getBootstrapData function
+jest.mock('src/utils/getBootstrapData', () => ({
+ __esModule: true,
+ default: () => ({
+ common: {
+ theme: {
+ enableUiThemeAdministration: true,
+ },
+ },
+ user: {
+ userId: 1,
+ username: 'admin',
+ roles: {
+ Admin: [['can_write', 'Theme']],
+ },
+ },
+ }),
+}));
+
+// Mock theme API functions
+jest.mock('src/features/themes/api', () => ({
+ setSystemDefaultTheme: jest.fn(() => Promise.resolve()),
+ setSystemDarkTheme: jest.fn(() => Promise.resolve()),
+ unsetSystemDefaultTheme: jest.fn(() => Promise.resolve()),
+ unsetSystemDarkTheme: jest.fn(() => Promise.resolve()),
+}));
+
+// Mock the CRUD hooks
+jest.mock('src/views/CRUD/hooks', () => ({
+ ...jest.requireActual('src/views/CRUD/hooks'),
+ useListViewResource: jest.fn(),
+}));
+
+// Mock the useThemeContext hook
+const mockSetTemporaryTheme = jest.fn();
+jest.mock('src/theme/ThemeProvider', () => ({
+ ...jest.requireActual('src/theme/ThemeProvider'),
+ useThemeContext: jest.fn(),
+}));
const mockThemes = [
{
id: 1,
- theme_name: 'Dark Theme',
- json_data: '{"colors": {"primary": "#1890ff"}}',
+ theme_name: 'Light Theme',
+ is_system_default: true,
+ is_system_dark: false,
is_system: false,
- changed_on_delta_humanized: '2 days ago',
+ json_data: '{"colors": {"primary": "#ffffff"}}',
+ created_by: { id: 1, first_name: 'Admin', last_name: 'User' },
+ changed_on_delta_humanized: '1 day ago',
changed_by: {
- first_name: 'John',
- last_name: 'Doe',
+ first_name: 'Admin',
+ last_name: 'User',
},
},
{
id: 2,
- theme_name: 'Light Theme',
- json_data: '{"colors": {"primary": "#ffffff"}}',
+ theme_name: 'Dark Theme',
+ is_system_default: false,
+ is_system_dark: true,
is_system: true,
- changed_on_delta_humanized: '1 week ago',
+ json_data: '{"colors": {"primary": "#1890ff"}}',
+ created_by: { id: 1, first_name: 'Admin', last_name: 'User' },
+ changed_on_delta_humanized: '2 days ago',
changed_by: {
- first_name: 'Jane',
- last_name: 'Smith',
+ first_name: 'John',
+ last_name: 'Doe',
},
},
{
id: 3,
theme_name: 'Custom Theme',
- json_data: '{"colors": {"primary": "#52c41a"}}',
+ is_system_default: false,
+ is_system_dark: false,
is_system: false,
- changed_on_delta_humanized: '1 day ago',
+ json_data: '{"colors": {"primary": "#52c41a"}}',
+ created_by: { id: 2, first_name: 'Test', last_name: 'User' },
+ changed_on_delta_humanized: '3 days ago',
changed_by: {
- first_name: 'Admin',
+ first_name: 'Test',
last_name: 'User',
},
},
@@ -72,25 +118,58 @@ const mockUser = {
lastName: 'User',
};
-fetchMock.get(themesInfoEndpoint, {
- permissions: ['can_read', 'can_write', 'can_export'],
-});
+const themesInfoEndpoint = 'glob:*/api/v1/theme/_info*';
+const themesEndpoint = 'glob:*/api/v1/theme/?*';
+const themeEndpoint = 'glob:*/api/v1/theme/*';
+
+const mockRefreshData = jest.fn();
-fetchMock.get(themesEndpoint, {
- ids: [1, 2, 3],
- result: mockThemes,
- count: mockThemes.length,
+beforeEach(() => {
+ // Mock the useListViewResource hook
+ (hooks.useListViewResource as jest.Mock).mockReturnValue({
+ state: {
+ loading: false,
+ resourceCollection: mockThemes,
+ resourceCount: 3,
+ bulkSelectEnabled: false,
+ },
+ setResourceCollection: jest.fn(),
+ hasPerm: jest.fn().mockReturnValue(true),
+ refreshData: mockRefreshData,
+ fetchData: jest.fn(),
+ toggleBulkSelect: jest.fn(),
+ });
+
+ // Mock useThemeContext
+ (useThemeContext as jest.Mock).mockReturnValue({
+ getCurrentCrudThemeId: jest.fn().mockReturnValue('1'),
+ appliedTheme: { theme_name: 'Light Theme', id: 1 },
+ setTemporaryTheme: mockSetTemporaryTheme,
+ });
+
+ fetchMock.reset();
+ fetchMock.get(themesInfoEndpoint, {
+ permissions: ['can_read', 'can_write', 'can_export'],
+ });
+ fetchMock.get(themesEndpoint, {
+ ids: [1, 2, 3],
+ count: 3,
+ result: mockThemes,
+ });
+ fetchMock.delete(themeEndpoint, {});
});
-fetchMock.delete(themeEndpoint, {});
+afterEach(() => {
+ fetchMock.restore();
+ jest.clearAllMocks();
+});
-const renderThemesList = (props = {}) =>
+test('renders themes list with all theme names', async () => {
render(
<ThemesList
user={mockUser}
- addDangerToast={() => {}}
- addSuccessToast={() => {}}
- {...props}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
/>,
{
useRedux: true,
@@ -100,163 +179,312 @@ const renderThemesList = (props = {}) =>
},
);
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('ThemesList', () => {
- beforeEach(() => {
- fetchMock.resetHistory();
+ await waitFor(() => {
+ expect(screen.getByText('Light Theme')).toBeInTheDocument();
+ expect(screen.getByText('Dark Theme')).toBeInTheDocument();
+ expect(screen.getByText('Custom Theme')).toBeInTheDocument();
});
+});
- test('renders', async () => {
- renderThemesList();
- expect(await screen.findByText('Themes')).toBeInTheDocument();
- });
+test('shows system tag for system themes', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- test('renders a ListView', async () => {
- renderThemesList();
- expect(await screen.findByTestId('themes-list-view')).toBeInTheDocument();
- });
+ await screen.findByText('Dark Theme');
- test('renders theme information', async () => {
- renderThemesList();
+ expect(screen.getByText('System')).toBeInTheDocument();
+});
- // Wait for list to load
- await screen.findByTestId('themes-list-view');
+test('shows default tag for system default theme', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- // Wait for data to load
- await waitFor(() => {
- mockThemes.forEach(theme => {
- expect(screen.getByText(theme.theme_name)).toBeInTheDocument();
- });
- });
- });
+ await screen.findByText('Light Theme');
- test('shows system theme tags correctly', async () => {
- renderThemesList();
+ expect(screen.getByText('Default')).toBeInTheDocument();
+});
- // Wait for list to load
- await screen.findByTestId('themes-list-view');
+test('shows dark tag for system dark theme', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- // System theme should have a "System" tag
- await waitFor(() => {
- expect(screen.getByText('System')).toBeInTheDocument();
- });
- });
+ await screen.findByText('Dark Theme');
- test('handles theme deletion for non-system themes', async () => {
- renderThemesList();
+ expect(screen.getByText('Dark')).toBeInTheDocument();
+});
- // Wait for list to load
- await screen.findByTestId('themes-list-view');
+test('shows apply action button for all themes', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- // Find delete buttons (should only exist for non-system themes)
- const deleteButtons = await screen.findAllByTestId('delete-action');
- expect(deleteButtons.length).toBeGreaterThan(0);
+ await screen.findByText('Custom Theme');
- fireEvent.click(deleteButtons[0]);
+ const applyButtons = await screen.findAllByTestId('apply-action');
+ expect(applyButtons.length).toBe(3);
+});
- // Confirm deletion modal should appear
- await waitFor(() => {
- expect(screen.getByText('Delete Theme?')).toBeInTheDocument();
- });
- });
+test('shows delete button only for non-system themes', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- test('shows apply action for themes', async () => {
- renderThemesList();
+ await screen.findByText('Custom Theme');
- // Wait for list to load
- await screen.findByTestId('themes-list-view');
+ const deleteButtons = await screen.findAllByTestId('delete-action');
+ // Should have delete buttons for Light Theme and Custom Theme (not Dark
Theme which is system)
+ expect(deleteButtons.length).toBe(2);
+});
- // Find apply buttons
- const applyButtons = await screen.findAllByTestId('apply-action');
- expect(applyButtons.length).toBe(mockThemes.length);
- });
+test('shows set default action for non-default themes', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- test('fetches themes data on load', async () => {
- renderThemesList();
+ await screen.findByText('Custom Theme');
- await waitFor(() => {
- const calls = fetchMock.calls(/api\/v1\/theme\/\?/);
- expect(calls.length).toBeGreaterThan(0);
- });
- });
+ const setDefaultButtons = await screen.findAllByTestId('set-default-action');
+ // Should have set default buttons for Dark Theme and Custom Theme (not
Light Theme which is already default)
+ expect(setDefaultButtons.length).toBe(2);
+});
- test('shows bulk select when user has permissions', async () => {
- renderThemesList();
+test('shows unset default action for system default theme', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- // Wait for list to load
- await screen.findByText('Themes');
+ await screen.findByText('Light Theme');
- // Should show bulk select button
- expect(screen.getByText('Bulk select')).toBeInTheDocument();
- });
+ const unsetDefaultButtons = await screen.findAllByTestId(
+ 'unset-default-action',
+ );
+ expect(unsetDefaultButtons.length).toBe(1);
+});
- test('shows create theme button when user has permissions', async () => {
- renderThemesList();
+test('shows set dark action for non-dark themes', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- // Wait for list to load
- await screen.findByText('Themes');
+ await screen.findByText('Custom Theme');
- // Should show theme creation button
- const addButton = screen.getByLabelText('plus');
- expect(addButton).toBeInTheDocument();
- });
+ const setDarkButtons = await screen.findAllByTestId('set-dark-action');
+ // Should have set dark buttons for Light Theme and Custom Theme (not Dark
Theme which is already dark)
+ expect(setDarkButtons.length).toBe(2);
+});
- // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
- describe('Modal.useModal integration', () => {
- beforeEach(() => {
- jest.clearAllMocks();
- });
+test('shows unset dark action for system dark theme', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- test('uses Modal.useModal hook instead of Modal.confirm', () => {
- const useModalSpy = jest.spyOn(Modal, 'useModal');
- renderThemesList();
+ await screen.findByText('Dark Theme');
- // Verify that useModal is called when the component mounts
- expect(useModalSpy).toHaveBeenCalled();
+ const unsetDarkButtons = await screen.findAllByTestId('unset-dark-action');
+ expect(unsetDarkButtons.length).toBe(1);
+});
- useModalSpy.mockRestore();
- });
+test('shows export action for all themes when user has permission', async ()
=> {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- test('renders contextHolder for modal theming', async () => {
- const { container } = renderThemesList();
+ await screen.findByText('Custom Theme');
- // Wait for component to be rendered
- await screen.findByText('Themes');
+ const exportButtons = await screen.findAllByTestId('export-action');
+ expect(exportButtons.length).toBe(3);
+});
- // The contextHolder is rendered but invisible, so we check for its
presence in the DOM
- // Modal.useModal returns elements that get rendered in the component
tree
- const contextHolderExists = container.querySelector('.ant-modal-root');
- expect(contextHolderExists).toBeDefined();
- });
+test('shows edit action for all themes when user has permission', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- test('confirms system theme changes using themed modal', async () => {
- const mockSetSystemDefault = jest.fn().mockResolvedValue({});
- fetchMock.post(
- 'glob:*/api/v1/theme/*/set_system_default',
- mockSetSystemDefault,
- );
+ await screen.findByText('Custom Theme');
- renderThemesList();
+ const editButtons = await screen.findAllByTestId('edit-action');
+ expect(editButtons.length).toBe(3);
+});
- // Wait for list to load
- await screen.findByTestId('themes-list-view');
+test('shows bulk select button when user has permissions', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- // Since the test data doesn't render actual action buttons, we'll verify
- // that the modal system is properly set up by checking the hook was
called
- // This is validated in the "uses Modal.useModal hook" test
- expect(true).toBe(true);
- });
+ await screen.findByText('Themes');
+
+ expect(screen.getByText('Bulk select')).toBeInTheDocument();
+});
+
+test('shows create theme button when user has permissions', async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
+
+ await screen.findByText('Themes');
+
+ const addButton = screen.getByLabelText('plus');
+ expect(addButton).toBeInTheDocument();
+});
+
+test('clicking apply button calls setTemporaryTheme with parsed theme data',
async () => {
+ render(
+ <ThemesList
+ user={mockUser}
+ addDangerToast={jest.fn()}
+ addSuccessToast={jest.fn()}
+ />,
+ {
+ useRedux: true,
+ useRouter: true,
+ useQueryParams: true,
+ useTheme: true,
+ },
+ );
- test('does not use deprecated Modal.confirm directly', () => {
- // Create a spy on the static Modal.confirm method
- const confirmSpy = jest.spyOn(Modal, 'confirm');
+ await screen.findByText('Custom Theme');
- renderThemesList();
+ const applyButtons = await screen.findAllByTestId('apply-action');
- // The component should not call Modal.confirm directly
- expect(confirmSpy).not.toHaveBeenCalled();
+ // Click the first apply button (Light Theme)
+ await userEvent.click(applyButtons[0]);
- confirmSpy.mockRestore();
+ await waitFor(() => {
+ expect(mockSetTemporaryTheme).toHaveBeenCalledWith({
+ colors: { primary: '#ffffff' },
});
});
});
diff --git a/superset-frontend/src/pages/ThemeList/index.test.tsx
b/superset-frontend/src/pages/ThemeList/index.test.tsx
deleted file mode 100644
index 12d6f11360..0000000000
--- a/superset-frontend/src/pages/ThemeList/index.test.tsx
+++ /dev/null
@@ -1,288 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import { render, screen, waitFor } from 'spec/helpers/testing-library';
-import fetchMock from 'fetch-mock';
-import * as hooks from 'src/views/CRUD/hooks';
-import * as themeApi from 'src/features/themes/api';
-import * as getBootstrapData from 'src/utils/getBootstrapData';
-import ThemesList from './index';
-
-// Mock the getBootstrapData function
-jest.mock('src/utils/getBootstrapData', () => ({
- __esModule: true,
- default: () => ({
- common: {
- theme: {
- enableUiThemeAdministration: true,
- },
- },
- user: {
- userId: 1,
- username: 'admin',
- roles: {
- Admin: [['can_write', 'Theme']],
- },
- },
- }),
-}));
-
-// Mock theme API functions
-jest.mock('src/features/themes/api', () => ({
- setSystemDefaultTheme: jest.fn(() => Promise.resolve()),
- setSystemDarkTheme: jest.fn(() => Promise.resolve()),
- unsetSystemDefaultTheme: jest.fn(() => Promise.resolve()),
- unsetSystemDarkTheme: jest.fn(() => Promise.resolve()),
-}));
-
-// Mock the CRUD hooks
-jest.mock('src/views/CRUD/hooks', () => ({
- ...jest.requireActual('src/views/CRUD/hooks'),
- useListViewResource: jest.fn(),
-}));
-
-// Mock the useThemeContext hook
-jest.mock('src/theme/ThemeProvider', () => ({
- ...jest.requireActual('src/theme/ThemeProvider'),
- useThemeContext: jest.fn().mockReturnValue({
- getCurrentCrudThemeId: jest.fn().mockReturnValue('1'),
- appliedTheme: { theme_name: 'Light Theme', id: 1 },
- }),
-}));
-
-const mockThemes = [
- {
- id: 1,
- theme_name: 'Light Theme',
- is_system_default: true,
- is_system_dark: false,
- created_by: { id: 1, first_name: 'Admin', last_name: 'User' },
- changed_on_delta_humanized: '1 day ago',
- },
- {
- id: 2,
- theme_name: 'Dark Theme',
- is_system_default: false,
- is_system_dark: true,
- created_by: { id: 1, first_name: 'Admin', last_name: 'User' },
- changed_on_delta_humanized: '2 days ago',
- },
- {
- id: 3,
- theme_name: 'Custom Theme',
- is_system_default: false,
- is_system_dark: false,
- created_by: { id: 2, first_name: 'Test', last_name: 'User' },
- changed_on_delta_humanized: '3 days ago',
- },
-];
-
-// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from
describe blocks
-describe('ThemesList', () => {
- beforeEach(() => {
- // Mock the useListViewResource hook
- (hooks.useListViewResource as jest.Mock).mockReturnValue({
- state: {
- loading: false,
- resourceCollection: mockThemes,
- resourceCount: 3,
- bulkSelectEnabled: false,
- },
- setResourceCollection: jest.fn(),
- hasPerm: jest.fn().mockReturnValue(true),
- refreshData: jest.fn(),
- fetchData: jest.fn(),
- toggleBulkSelect: jest.fn(),
- });
-
- fetchMock.reset();
- fetchMock.get('glob:*/api/v1/theme/*', {
- count: 3,
- result: mockThemes,
- });
- });
-
- afterEach(() => {
- fetchMock.restore();
- jest.clearAllMocks();
- });
-
- test('renders the themes list with proper structure', async () => {
- render(
- <ThemesList addDangerToast={jest.fn()} addSuccessToast={jest.fn()} />,
- {
- useRedux: true,
- useDnd: true,
- useQueryParams: true,
- },
- );
-
- await waitFor(() => {
- expect(screen.getByText('Light Theme')).toBeInTheDocument();
- expect(screen.getByText('Dark Theme')).toBeInTheDocument();
- expect(screen.getByText('Custom Theme')).toBeInTheDocument();
- });
- });
-
- test('shows system theme badges for default and dark themes', async () => {
- render(
- <ThemesList addDangerToast={jest.fn()} addSuccessToast={jest.fn()} />,
- {
- useRedux: true,
- useDnd: true,
- useQueryParams: true,
- },
- );
-
- await waitFor(() => {
- // Check for system badges
- expect(screen.getByText('Light Theme')).toBeInTheDocument();
- expect(screen.getByText('Dark Theme')).toBeInTheDocument();
- });
-
- // Verify that themes with system flags are marked appropriately
- expect(mockThemes[0].is_system_default).toBe(true);
- expect(mockThemes[1].is_system_dark).toBe(true);
- expect(mockThemes[2].is_system_default).toBe(false);
- expect(mockThemes[2].is_system_dark).toBe(false);
- });
-
- test('uses flat theme structure for enableUiThemeAdministration', () => {
- // Verify the component accesses the correct bootstrap data structure
- const { common } = getBootstrapData.default();
-
- // Should access flat structure
- expect(common.theme.enableUiThemeAdministration).toBe(true);
-
- // Should NOT have nested settings structure
- expect((common.theme as any).settings).toBeUndefined();
- });
-
- test('shows admin controls when user has proper permissions', async () => {
- render(
- <ThemesList addDangerToast={jest.fn()} addSuccessToast={jest.fn()} />,
- {
- useRedux: true,
- useDnd: true,
- useQueryParams: true,
- },
- );
-
- await waitFor(() => {
- expect(screen.getByText('Light Theme')).toBeInTheDocument();
- });
-
- // Admin controls should be visible
- const adminElements = screen.queryAllByRole('button');
- expect(adminElements.length).toBeGreaterThan(0);
- });
-
- test('calls setSystemDefaultTheme API when setting a theme as default',
async () => {
- const { setSystemDefaultTheme } = themeApi;
- const addSuccessToast = jest.fn();
- const refreshData = jest.fn();
-
- (hooks.useListViewResource as jest.Mock).mockReturnValue({
- state: {
- loading: false,
- resourceCollection: mockThemes,
- resourceCount: 3,
- bulkSelectEnabled: false,
- },
- setResourceCollection: jest.fn(),
- hasPerm: jest.fn().mockReturnValue(true),
- refreshData,
- fetchData: jest.fn(),
- toggleBulkSelect: jest.fn(),
- });
-
- render(
- <ThemesList
- addDangerToast={jest.fn()}
- addSuccessToast={addSuccessToast}
- />,
- {
- useRedux: true,
- useDnd: true,
- useQueryParams: true,
- },
- );
-
- await waitFor(() => {
- expect(screen.getByText('Custom Theme')).toBeInTheDocument();
- });
-
- // Since the actual UI interaction would be complex to test,
- // we verify the API function exists and can be called
- expect(setSystemDefaultTheme).toBeDefined();
-
- // Test calling the API directly
- await setSystemDefaultTheme(3);
- expect(setSystemDefaultTheme).toHaveBeenCalledWith(3);
- });
-
- test('configures theme deletion endpoint', async () => {
- const addDangerToast = jest.fn();
- const addSuccessToast = jest.fn();
- const refreshData = jest.fn();
-
- // Setup delete mock before rendering
- fetchMock.delete('glob:*/api/v1/theme/*', 200);
-
- // Mock the useListViewResource hook with refreshData
- (hooks.useListViewResource as jest.Mock).mockReturnValue({
- state: {
- loading: false,
- resourceCollection: mockThemes,
- resourceCount: 3,
- bulkSelectEnabled: false,
- },
- setResourceCollection: jest.fn(),
- hasPerm: jest.fn().mockReturnValue(true),
- refreshData,
- fetchData: jest.fn(),
- toggleBulkSelect: jest.fn(),
- });
-
- render(
- <ThemesList
- addDangerToast={addDangerToast}
- addSuccessToast={addSuccessToast}
- />,
- {
- useRedux: true,
- useDnd: true,
- useQueryParams: true,
- },
- );
-
- await waitFor(() => {
- expect(screen.getByText('Custom Theme')).toBeInTheDocument();
- });
-
- // Verify that themes are rendered correctly
- expect(screen.getByText('Light Theme')).toBeInTheDocument();
- expect(screen.getByText('Dark Theme')).toBeInTheDocument();
- expect(screen.getByText('Custom Theme')).toBeInTheDocument();
-
- // Verify that action buttons are present (which include delete for
non-system themes)
- const actionButtons = screen.getAllByRole('button');
- expect(actionButtons.length).toBeGreaterThan(0);
- });
-});
diff --git a/superset-frontend/src/pages/ThemeList/index.tsx
b/superset-frontend/src/pages/ThemeList/index.tsx
index 75755170b9..adef9153a3 100644
--- a/superset-frontend/src/pages/ThemeList/index.tsx
+++ b/superset-frontend/src/pages/ThemeList/index.tsx
@@ -27,7 +27,6 @@ import {
Alert,
Tooltip,
Space,
- Modal,
} from '@superset-ui/core/components';
import rison from 'rison';
@@ -53,6 +52,7 @@ import ThemeModal from 'src/features/themes/ThemeModal';
import { ThemeObject } from 'src/features/themes/types';
import { QueryObjectColumns } from 'src/views/CRUD/types';
import { Icons } from '@superset-ui/core/components/Icons';
+import { useConfirmModal } from 'src/hooks/useConfirmModal';
import {
setSystemDefaultTheme,
setSystemDarkTheme,
@@ -117,8 +117,7 @@ function ThemesList({
const [importingTheme, showImportModal] = useState<boolean>(false);
const [appliedThemeId, setAppliedThemeId] = useState<number | null>(null);
- // Use Modal.useModal hook to ensure proper theming
- const [modal, contextHolder] = Modal.useModal();
+ const { showConfirm, ConfirmModal } = useConfirmModal();
const canCreate = hasPerm('can_write');
const canEdit = hasPerm('can_write');
@@ -250,83 +249,97 @@ function ThemesList({
addSuccessToast(t('Theme imported'));
};
- // Generic confirmation modal utility to reduce code duplication
- const showThemeConfirmation = (config: {
- title: string;
- content: string;
- onConfirm: () => Promise<any>;
- successMessage: string;
- errorMessage: string;
- }) => {
- modal.confirm({
- title: config.title,
- content: config.content,
- onOk: () => {
- config
- .onConfirm()
- .then(() => {
+ const handleSetSystemDefault = useCallback(
+ (theme: ThemeObject) => {
+ showConfirm({
+ title: t('Set System Default Theme'),
+ body: t(
+ 'Are you sure you want to set "%s" as the system default theme? This
will apply to all users who haven\'t set a personal preference.',
+ theme.theme_name,
+ ),
+ onConfirm: async () => {
+ try {
+ await setSystemDefaultTheme(theme.id!);
refreshData();
- addSuccessToast(config.successMessage);
- })
- .catch(err => {
- addDangerToast(t(config.errorMessage, err.message));
- });
- },
- });
- };
-
- const handleSetSystemDefault = (theme: ThemeObject) => {
- showThemeConfirmation({
- title: t('Set System Default Theme'),
- content: t(
- 'Are you sure you want to set "%s" as the system default theme? This
will apply to all users who haven\'t set a personal preference.',
- theme.theme_name,
- ),
- onConfirm: () => setSystemDefaultTheme(theme.id!),
- successMessage: t(
- '"%s" is now the system default theme',
- theme.theme_name,
- ),
- errorMessage: 'Failed to set system default theme: %s',
- });
- };
+ addSuccessToast(
+ t('"%s" is now the system default theme', theme.theme_name),
+ );
+ } catch (err: any) {
+ addDangerToast(
+ t('Failed to set system default theme: %s', err.message),
+ );
+ }
+ },
+ });
+ },
+ [showConfirm, refreshData, addSuccessToast, addDangerToast],
+ );
- const handleSetSystemDark = (theme: ThemeObject) => {
- showThemeConfirmation({
- title: t('Set System Dark Theme'),
- content: t(
- 'Are you sure you want to set "%s" as the system dark theme? This will
apply to all users who haven\'t set a personal preference.',
- theme.theme_name,
- ),
- onConfirm: () => setSystemDarkTheme(theme.id!),
- successMessage: t('"%s" is now the system dark theme', theme.theme_name),
- errorMessage: 'Failed to set system dark theme: %s',
- });
- };
+ const handleSetSystemDark = useCallback(
+ (theme: ThemeObject) => {
+ showConfirm({
+ title: t('Set System Dark Theme'),
+ body: t(
+ 'Are you sure you want to set "%s" as the system dark theme? This
will apply to all users who haven\'t set a personal preference.',
+ theme.theme_name,
+ ),
+ onConfirm: async () => {
+ try {
+ await setSystemDarkTheme(theme.id!);
+ refreshData();
+ addSuccessToast(
+ t('"%s" is now the system dark theme', theme.theme_name),
+ );
+ } catch (err: any) {
+ addDangerToast(
+ t('Failed to set system dark theme: %s', err.message),
+ );
+ }
+ },
+ });
+ },
+ [showConfirm, refreshData, addSuccessToast, addDangerToast],
+ );
- const handleUnsetSystemDefault = () => {
- showThemeConfirmation({
+ const handleUnsetSystemDefault = useCallback(() => {
+ showConfirm({
title: t('Remove System Default Theme'),
- content: t(
+ body: t(
'Are you sure you want to remove the system default theme? The
application will fall back to the configuration file default.',
),
- onConfirm: () => unsetSystemDefaultTheme(),
- successMessage: t('System default theme removed'),
- errorMessage: 'Failed to remove system default theme: %s',
+ onConfirm: async () => {
+ try {
+ await unsetSystemDefaultTheme();
+ refreshData();
+ addSuccessToast(t('System default theme removed'));
+ } catch (err: any) {
+ addDangerToast(
+ t('Failed to remove system default theme: %s', err.message),
+ );
+ }
+ },
});
- };
+ }, [showConfirm, refreshData, addSuccessToast, addDangerToast]);
- const handleUnsetSystemDark = () => {
- showThemeConfirmation({
+ const handleUnsetSystemDark = useCallback(() => {
+ showConfirm({
title: t('Remove System Dark Theme'),
- content: t(
+ body: t(
'Are you sure you want to remove the system dark theme? The
application will fall back to the configuration file dark theme.',
),
- onConfirm: () => unsetSystemDarkTheme(),
- successMessage: t('System dark theme removed'),
- errorMessage: 'Failed to remove system dark theme: %s',
+ onConfirm: async () => {
+ try {
+ await unsetSystemDarkTheme();
+ refreshData();
+ addSuccessToast(t('System dark theme removed'));
+ } catch (err: any) {
+ addDangerToast(
+ t('Failed to remove system dark theme: %s', err.message),
+ );
+ }
+ },
});
- };
+ }, [showConfirm, refreshData, addSuccessToast, addDangerToast]);
const initialSort = [{ id: 'theme_name', desc: true }];
const columns = useMemo(
@@ -598,7 +611,6 @@ function ThemesList({
return (
<>
- {contextHolder}
<SubMenu {...menuData} />
<ThemeModal
addDangerToast={addDangerToast}
@@ -705,6 +717,7 @@ function ThemesList({
}}
</ConfirmStatusChange>
{preparingExport && <Loading />}
+ {ConfirmModal}
</>
);
}
