Skip to content

Commit cbb2b2f

Browse files
authored
feat(themes): add JSON formatting to theme modal editor (#38739)
1 parent 82a74c8 commit cbb2b2f

File tree

2 files changed

+196
-22
lines changed

2 files changed

+196
-22
lines changed

superset-frontend/src/features/themes/ThemeModal.test.tsx

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,138 @@ test('applies theme locally when clicking Apply button', async () => {
712712
expect(mockThemeContext.setTemporaryTheme).toHaveBeenCalled();
713713
});
714714

715+
test('shows Format button when modal is in edit mode', () => {
716+
render(
717+
<ThemeModal
718+
addDangerToast={jest.fn()}
719+
addSuccessToast={jest.fn()}
720+
onThemeAdd={jest.fn()}
721+
onHide={jest.fn()}
722+
show
723+
canDevelop={false}
724+
/>,
725+
{ useRedux: true, useRouter: true },
726+
);
727+
728+
expect(screen.getByRole('button', { name: /format/i })).toBeInTheDocument();
729+
});
730+
731+
test('does not show Format button for read-only system themes', async () => {
732+
render(
733+
<ThemeModal
734+
addDangerToast={jest.fn()}
735+
addSuccessToast={jest.fn()}
736+
onThemeAdd={jest.fn()}
737+
onHide={jest.fn()}
738+
show
739+
canDevelop={false}
740+
theme={mockSystemTheme}
741+
/>,
742+
{ useRedux: true, useRouter: true },
743+
);
744+
745+
await screen.findByText('System Theme - Read Only');
746+
747+
expect(
748+
screen.queryByRole('button', { name: /format/i }),
749+
).not.toBeInTheDocument();
750+
});
751+
752+
test('disables Format button when JSON is invalid', async () => {
753+
render(
754+
<ThemeModal
755+
addDangerToast={jest.fn()}
756+
addSuccessToast={jest.fn()}
757+
onThemeAdd={jest.fn()}
758+
onHide={jest.fn()}
759+
show
760+
canDevelop={false}
761+
/>,
762+
{ useRedux: true, useRouter: true },
763+
);
764+
765+
const jsonEditor = screen.getByTestId('json-editor');
766+
userEvent.clear(jsonEditor);
767+
userEvent.type(jsonEditor, '{invalid json');
768+
769+
await waitFor(() => {
770+
expect(screen.getByRole('button', { name: /format/i })).toBeDisabled();
771+
});
772+
});
773+
774+
test('enables Format button when JSON is valid', async () => {
775+
render(
776+
<ThemeModal
777+
addDangerToast={jest.fn()}
778+
addSuccessToast={jest.fn()}
779+
onThemeAdd={jest.fn()}
780+
onHide={jest.fn()}
781+
show
782+
canDevelop={false}
783+
/>,
784+
{ useRedux: true, useRouter: true },
785+
);
786+
787+
await addValidJsonData();
788+
789+
await waitFor(() => {
790+
expect(screen.getByRole('button', { name: /format/i })).toBeEnabled();
791+
});
792+
});
793+
794+
test('Format button pretty-prints minified JSON', async () => {
795+
render(
796+
<ThemeModal
797+
addDangerToast={jest.fn()}
798+
addSuccessToast={jest.fn()}
799+
onThemeAdd={jest.fn()}
800+
onHide={jest.fn()}
801+
show
802+
canDevelop={false}
803+
/>,
804+
{ useRedux: true, useRouter: true },
805+
);
806+
807+
const minifiedJson = '{"token":{"colorPrimary":"#1890ff"}}';
808+
const jsonEditor = screen.getByTestId('json-editor');
809+
userEvent.clear(jsonEditor);
810+
userEvent.type(jsonEditor, minifiedJson);
811+
812+
const formatButton = screen.getByRole('button', { name: /format/i });
813+
userEvent.click(formatButton);
814+
815+
const expectedFormatted = JSON.stringify(
816+
{ token: { colorPrimary: '#1890ff' } },
817+
null,
818+
2,
819+
);
820+
await waitFor(() => {
821+
expect(jsonEditor).toHaveValue(expectedFormatted);
822+
});
823+
});
824+
825+
test('Format button is disabled when JSON editor is empty', async () => {
826+
render(
827+
<ThemeModal
828+
addDangerToast={jest.fn()}
829+
addSuccessToast={jest.fn()}
830+
onThemeAdd={jest.fn()}
831+
onHide={jest.fn()}
832+
show
833+
canDevelop={false}
834+
/>,
835+
{ useRedux: true, useRouter: true },
836+
);
837+
838+
// The editor initializes with `{}` — clear it to reach the empty state
839+
const jsonEditor = screen.getByTestId('json-editor');
840+
userEvent.clear(jsonEditor);
841+
842+
await waitFor(() => {
843+
expect(screen.getByRole('button', { name: /format/i })).toBeDisabled();
844+
});
845+
});
846+
715847
test('disables Apply button when JSON configuration is invalid', async () => {
716848
fetchMock.clearHistory().removeRoutes();
717849
fetchMock.get('glob:*/api/v1/theme/*', {

superset-frontend/src/features/themes/ThemeModal.tsx

Lines changed: 64 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,15 @@ const toEditorAnnotations = (
7171
message: ann.text,
7272
}));
7373

74+
const formatJsonData = (jsonData?: string): string | undefined => {
75+
if (!jsonData) return jsonData;
76+
try {
77+
return JSON.stringify(JSON.parse(jsonData), null, 2);
78+
} catch {
79+
return jsonData;
80+
}
81+
};
82+
7483
interface ThemeModalProps {
7584
addDangerToast: (msg: string) => void;
7685
addSuccessToast?: (msg: string) => void;
@@ -316,6 +325,15 @@ const ThemeModal: FunctionComponent<ThemeModalProps> = ({
316325
[currentTheme],
317326
);
318327

328+
const onFormat = useCallback(() => {
329+
if (currentTheme?.json_data) {
330+
const formatted = formatJsonData(currentTheme.json_data);
331+
if (formatted !== currentTheme.json_data) {
332+
onJsonDataChange(formatted || '');
333+
}
334+
}
335+
}, [currentTheme?.json_data, onJsonDataChange]);
336+
319337
const validate = () => {
320338
if (isReadOnly || !currentTheme) {
321339
setDisableSave(true);
@@ -357,8 +375,12 @@ const ThemeModal: FunctionComponent<ThemeModalProps> = ({
357375

358376
useEffect(() => {
359377
if (resource) {
360-
setCurrentTheme(resource);
361-
setInitialTheme(resource);
378+
const formatted = {
379+
...resource,
380+
json_data: formatJsonData(resource.json_data),
381+
};
382+
setCurrentTheme(formatted);
383+
setInitialTheme(formatted);
362384
}
363385
}, [resource]);
364386

@@ -522,27 +544,47 @@ const ThemeModal: FunctionComponent<ThemeModalProps> = ({
522544
annotations={toEditorAnnotations(validation.annotations)}
523545
/>
524546
</StyledEditorWrapper>
525-
{canDevelopThemes && (
526-
<div className="apply-button-container">
527-
<Tooltip
528-
title={t('Set local theme for testing (preview only)')}
529-
placement="top"
530-
>
531-
<Button
532-
icon={<Icons.ThunderboltOutlined />}
533-
onClick={onApply}
534-
disabled={
535-
!currentTheme?.json_data ||
536-
!isValidJson(currentTheme.json_data) ||
537-
validation.hasErrors
538-
}
539-
buttonStyle="secondary"
547+
<div className="apply-button-container">
548+
<Space>
549+
{!isReadOnly && (
550+
<Tooltip
551+
title={t('Format JSON configuration')}
552+
placement="top"
540553
>
541-
{t('Apply')}
542-
</Button>
543-
</Tooltip>
544-
</div>
545-
)}
554+
<Button
555+
icon={<Icons.AlignLeftOutlined />}
556+
buttonStyle="secondary"
557+
onClick={onFormat}
558+
disabled={
559+
!currentTheme?.json_data ||
560+
!isValidJson(currentTheme.json_data)
561+
}
562+
>
563+
{t('Format')}
564+
</Button>
565+
</Tooltip>
566+
)}
567+
{canDevelopThemes && (
568+
<Tooltip
569+
title={t('Set local theme for testing (preview only)')}
570+
placement="top"
571+
>
572+
<Button
573+
icon={<Icons.ThunderboltOutlined />}
574+
onClick={onApply}
575+
disabled={
576+
!currentTheme?.json_data ||
577+
!isValidJson(currentTheme.json_data) ||
578+
validation.hasErrors
579+
}
580+
buttonStyle="secondary"
581+
>
582+
{t('Apply')}
583+
</Button>
584+
</Tooltip>
585+
)}
586+
</Space>
587+
</div>
546588
</Form.Item>
547589
</Form>
548590
</StyledFormWrapper>

0 commit comments

Comments
 (0)