This is an automated email from the ASF dual-hosted git repository.
msyavuz 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 cb88d886c7 fix(PropertiesModal): do not show validation errors while
loading (#35215)
cb88d886c7 is described below
commit cb88d886c71066e4fdbb09c0aeaa9fbe1378d9fd
Author: Mehmet Salih Yavuz <[email protected]>
AuthorDate: Wed Sep 24 10:52:16 2025 +0300
fix(PropertiesModal): do not show validation errors while loading (#35215)
---
.../src/components/Modal/StandardModal.tsx | 14 +++-
.../PropertiesModal/PropertiesModal.test.tsx | 24 +++++-
.../dashboard/components/PropertiesModal/index.tsx | 18 ++--
.../sections/BasicInfoSection.test.tsx | 15 +---
.../PropertiesModal/sections/BasicInfoSection.tsx | 97 +++++++++++-----------
5 files changed, 91 insertions(+), 77 deletions(-)
diff --git a/superset-frontend/src/components/Modal/StandardModal.tsx
b/superset-frontend/src/components/Modal/StandardModal.tsx
index 43522baf22..49e0d31433 100644
--- a/superset-frontend/src/components/Modal/StandardModal.tsx
+++ b/superset-frontend/src/components/Modal/StandardModal.tsx
@@ -18,7 +18,7 @@
*/
import { ReactNode } from 'react';
import { styled, t } from '@superset-ui/core';
-import { Modal } from '@superset-ui/core/components';
+import { Modal, Loading, Flex } from '@superset-ui/core/components';
import { ModalTitleWithIcon } from 'src/components/ModalTitleWithIcon';
interface StandardModalProps {
@@ -39,6 +39,7 @@ interface StandardModalProps {
destroyOnClose?: boolean;
maskClosable?: boolean;
wrapProps?: object;
+ contentLoading?: boolean;
}
// Standard modal widths
@@ -113,12 +114,13 @@ export function StandardModal({
destroyOnClose = true,
maskClosable = false,
wrapProps,
+ contentLoading = false,
}: StandardModalProps) {
const primaryButtonName = saveText || (isEditMode ? t('Save') : t('Add'));
return (
<StyledModal
- disablePrimaryButton={saveDisabled || saveLoading}
+ disablePrimaryButton={saveDisabled || saveLoading || contentLoading}
primaryButtonLoading={saveLoading}
primaryTooltipMessage={errorTooltip}
onHandledPrimaryAction={onSave}
@@ -139,7 +141,13 @@ export function StandardModal({
)
}
>
- {children}
+ {contentLoading ? (
+ <Flex justify="center" align="center" style={{ minHeight: 200 }}>
+ <Loading />
+ </Flex>
+ ) : (
+ children
+ )}
</StyledModal>
);
}
diff --git
a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
index 52ead26cb3..9298c6b09e 100644
---
a/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
+++
b/superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
@@ -354,9 +354,19 @@ describe('PropertiesModal', () => {
mockedIsFeatureEnabled.mockReturnValue(false);
const props = createProps();
props.onlyApply = false;
- render(<PropertiesModal {...props} />, {
+ // Pass dashboardInfo to avoid loading state
+ const propsWithDashboardInfo = {
+ ...props,
+ dashboardInfo: {
+ ...dashboardInfo,
+ json_metadata: mockedJsonMetadata,
+ },
+ };
+ render(<PropertiesModal {...propsWithDashboardInfo} />, {
useRedux: true,
});
+
+ // Wait for the form to be visible
expect(
await screen.findByTestId('dashboard-edit-properties-form'),
).toBeInTheDocument();
@@ -379,9 +389,19 @@ describe('PropertiesModal', () => {
mockedIsFeatureEnabled.mockReturnValue(false);
const props = createProps();
props.onlyApply = true;
- render(<PropertiesModal {...props} />, {
+ // Pass dashboardInfo to avoid loading state
+ const propsWithDashboardInfo = {
+ ...props,
+ dashboardInfo: {
+ ...dashboardInfo,
+ json_metadata: mockedJsonMetadata,
+ },
+ };
+ render(<PropertiesModal {...propsWithDashboardInfo} />, {
useRedux: true,
});
+
+ // Wait for the form to be visible
expect(
await screen.findByTestId('dashboard-edit-properties-form'),
).toBeInTheDocument();
diff --git
a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
index 040bc6ee2e..614f042ba5 100644
--- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
+++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
@@ -112,7 +112,7 @@ const PropertiesModal = ({
const dispatch = useDispatch();
const [form] = Form.useForm();
- const [isLoading, setIsLoading] = useState(false);
+ const [isLoading, setIsLoading] = useState(true);
const [isApplying, setIsApplying] = useState(false);
const [colorScheme, setCurrentColorScheme] = useState(currentColorScheme);
const [jsonMetadata, setJsonMetadata] = useState('');
@@ -207,7 +207,6 @@ const PropertiesModal = ({
);
const fetchDashboardDetails = useCallback(() => {
- setIsLoading(true);
// We fetch the dashboard details because not all code
// that renders this component have all the values we need.
// At some point when we have a more consistent frontend
@@ -382,10 +381,6 @@ const PropertiesModal = ({
if (onlyApply) {
setIsApplying(true);
try {
- console.log('Apply CSS debug:', {
- css_being_sent: customCss,
- onSubmitProps_css: onSubmitProps.css,
- });
onSubmit(onSubmitProps);
onHide();
addSuccessToast(t('Dashboard properties updated'));
@@ -422,10 +417,15 @@ const PropertiesModal = ({
useEffect(() => {
if (show) {
+ // Reset loading state when modal opens
+ setIsLoading(true);
+
if (!currentDashboardInfo) {
fetchDashboardDetails();
} else {
handleDashboardData(currentDashboardInfo);
+ // Data is immediately available, so we can stop loading
+ setIsLoading(false);
}
// Fetch themes (excluding system themes)
@@ -621,10 +621,9 @@ const PropertiesModal = ({
}}
title={t('Dashboard properties')}
isEditMode
- saveDisabled={
- isLoading || dashboardInfo?.isManagedExternally || hasErrors
- }
+ saveDisabled={dashboardInfo?.isManagedExternally || hasErrors}
saveLoading={isApplying}
+ contentLoading={isLoading}
errorTooltip={
dashboardInfo?.isManagedExternally
? t(
@@ -665,7 +664,6 @@ const PropertiesModal = ({
children: (
<BasicInfoSection
form={form}
- isLoading={isLoading}
validationStatus={validationStatus}
/>
),
diff --git
a/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.test.tsx
b/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.test.tsx
index 6f8052c9c5..df3c8f322d 100644
---
a/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.test.tsx
+++
b/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.test.tsx
@@ -21,8 +21,9 @@ import { Form } from '@superset-ui/core/components';
import BasicInfoSection from './BasicInfoSection';
const defaultProps = {
- form: {} as any,
- isLoading: false,
+ form: {
+ getFieldValue: jest.fn(() => 'Test Dashboard'),
+ } as any,
validationStatus: {
basic: { hasErrors: false, errors: [], name: 'Basic' },
},
@@ -50,16 +51,6 @@ test('shows required asterisk for name field', () => {
expect(screen.getByText('*')).toBeInTheDocument();
});
-test('disables inputs when loading', () => {
- render(
- <Form>
- <BasicInfoSection {...defaultProps} isLoading />
- </Form>,
- );
-
- expect(screen.getByTestId('dashboard-title-input')).toBeDisabled();
-});
-
test('shows error message when name is empty and has validation errors', () =>
{
const mockForm = {
getFieldValue: jest.fn(field => (field === 'title' ? '' : 'test')),
diff --git
a/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.tsx
b/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.tsx
index cd11e71647..43047725be 100644
---
a/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.tsx
+++
b/superset-frontend/src/dashboard/components/PropertiesModal/sections/BasicInfoSection.tsx
@@ -23,62 +23,59 @@ import { ValidationObject } from
'src/components/Modal/useModalValidation';
interface BasicInfoSectionProps {
form: FormInstance;
- isLoading: boolean;
validationStatus: ValidationObject;
}
const BasicInfoSection = ({
form,
- isLoading,
validationStatus,
-}: BasicInfoSectionProps) => (
- <>
- <ModalFormField
- label={t('Name')}
- required
- testId="dashboard-name-field"
- error={
- validationStatus.basic?.hasErrors &&
- (!form.getFieldValue('title') ||
- form.getFieldValue('title').trim().length === 0)
- ? t('Dashboard name is required')
- : undefined
- }
- >
- <FormItem
- name="title"
- noStyle
- rules={[
- {
- required: true,
- message: t('Dashboard name is required'),
- whitespace: true,
- },
- ]}
+}: BasicInfoSectionProps) => {
+ const titleValue = form.getFieldValue('title');
+ const hasError =
+ validationStatus.basic?.hasErrors &&
+ (!titleValue || titleValue.trim().length === 0);
+
+ return (
+ <>
+ <ModalFormField
+ label={t('Name')}
+ required
+ testId="dashboard-name-field"
+ error={hasError ? t('Dashboard name is required') : undefined}
+ >
+ <FormItem
+ name="title"
+ noStyle
+ rules={[
+ {
+ required: true,
+ message: t('Dashboard name is required'),
+ whitespace: true,
+ },
+ ]}
+ >
+ <Input
+ placeholder={t('The display name of your dashboard')}
+ data-test="dashboard-title-input"
+ type="text"
+ />
+ </FormItem>
+ </ModalFormField>
+ <ModalFormField
+ label={t('URL Slug')}
+ testId="dashboard-slug-field"
+ bottomSpacing={false}
>
- <Input
- placeholder={t('The display name of your dashboard')}
- data-test="dashboard-title-input"
- type="text"
- disabled={isLoading}
- />
- </FormItem>
- </ModalFormField>
- <ModalFormField
- label={t('URL Slug')}
- testId="dashboard-slug-field"
- bottomSpacing={false}
- >
- <FormItem name="slug" noStyle>
- <Input
- placeholder={t('A readable URL for your dashboard')}
- data-test="dashboard-slug-input"
- type="text"
- disabled={isLoading}
- />
- </FormItem>
- </ModalFormField>
- </>
-);
+ <FormItem name="slug" noStyle>
+ <Input
+ placeholder={t('A readable URL for your dashboard')}
+ data-test="dashboard-slug-input"
+ type="text"
+ />
+ </FormItem>
+ </ModalFormField>
+ </>
+ );
+};
export default BasicInfoSection;
