iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(list/chart views): Chart Properties modal now has transitions (#28796) 

Co-authored-by: JUST.in DO IT <justin.park@airbnb.com>
This commit is contained in:
Evan Rusackas 2024-07-31 17:57:53 -06:00 committed by GitHub
parent 59e366ce90
commit 66eb9593d1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 51 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
superset-frontend/src/explore/components/PropertiesModal/index.tsx
View File

@ -69,7 +69,7 @@ function PropertiesModal({
const [submitting, setSubmitting] = useState(false); const [submitting, setSubmitting] = useState(false);
const [form] = AntdForm.useForm(); const [form] = AntdForm.useForm();
// values of form inputs // values of form inputs
const [name, setName] = useState(slice.slice_name || ''); const [name, setName] = useState(slice?.slice_name || '');
const [selectedOwners, setSelectedOwners] = useState<SelectValue | null>( const [selectedOwners, setSelectedOwners] = useState<SelectValue | null>(
null, null,
); );
@ -98,23 +98,25 @@ function PropertiesModal({
const fetchChartOwners = useCallback( const fetchChartOwners = useCallback(
async function fetchChartOwners() { async function fetchChartOwners() {
try { if (slice?.slice_id) {
const response = await SupersetClient.get({ try {
endpoint: `/api/v1/chart/${slice.slice_id}`, const response = await SupersetClient.get({
}); endpoint: `/api/v1/chart/${slice?.slice_id}`,
const chart = response.json.result; });
setSelectedOwners( const chart = response.json.result;
chart?.owners?.map((owner: any) => ({ setSelectedOwners(
value: owner.id, chart?.owners?.map((owner: any) => ({
label: `${owner.first_name} ${owner.last_name}`, value: owner.id,
})), label: `${owner.first_name} ${owner.last_name}`,
); })),
} catch (response) { );
const clientError = await getClientErrorObject(response); } catch (response) {
showError(clientError); const clientError = await getClientErrorObject(response);
showError(clientError);
}
} }
}, },
[slice.slice_id], [slice?.slice_id],
); );
const loadOptions = useMemo( const loadOptions = useMemo(
@ -175,7 +177,7 @@ function PropertiesModal({
try { try {
const res = await SupersetClient.put({ const res = await SupersetClient.put({
endpoint: `/api/v1/chart/${slice.slice_id}`, endpoint: `/api/v1/chart/${slice?.slice_id}`,
headers: { 'Content-Type': 'application/json' }, headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(payload), body: JSON.stringify(payload),
}); });
@ -184,7 +186,7 @@ function PropertiesModal({
...payload, ...payload,
...res.json.result, ...res.json.result,
tags, tags,
id: slice.slice_id, id: slice?.slice_id,
owners: selectedOwners, owners: selectedOwners,
}; };
onSave(updatedChart); onSave(updatedChart);
@ -206,8 +208,8 @@ function PropertiesModal({
// update name after it's changed in another modal // update name after it's changed in another modal
useEffect(() => { useEffect(() => {
setName(slice.slice_name || ''); setName(slice?.slice_name || '');
}, [slice.slice_name]); }, [slice?.slice_name]);
useEffect(() => { useEffect(() => {
if (!isFeatureEnabled(FeatureFlag.TaggingSystem)) return; if (!isFeatureEnabled(FeatureFlag.TaggingSystem)) return;
@ -215,7 +217,7 @@ function PropertiesModal({
fetchTags( fetchTags(
{ {
objectType: OBJECT_TYPES.CHART, objectType: OBJECT_TYPES.CHART,
objectId: slice.slice_id, objectId: slice?.slice_id,
includeTypes: false, includeTypes: false,
}, },
(tags: TagType[]) => setTags(tags), (tags: TagType[]) => setTags(tags),
@ -226,7 +228,7 @@ function PropertiesModal({
} catch (error) { } catch (error) {
showError(error); showError(error);
} }
}, [slice.slice_id]); }, [slice?.slice_id]);
const handleChangeTags = (tags: { label: string; value: number }[]) => { const handleChangeTags = (tags: { label: string; value: number }[]) => {
const parsedTags: TagType[] = ensureIsArray(tags).map(r => ({ const parsedTags: TagType[] = ensureIsArray(tags).map(r => ({
@ -262,9 +264,9 @@ function PropertiesModal({
buttonSize="small" buttonSize="small"
buttonStyle="primary" buttonStyle="primary"
onClick={form.submit} onClick={form.submit}
disabled={submitting || !name || slice.is_managed_externally} disabled={submitting || !name || slice?.is_managed_externally}
tooltip={ tooltip={
slice.is_managed_externally slice?.is_managed_externally
? t( ? t(
"This chart is managed externally, and can't be edited in Superset", "This chart is managed externally, and can't be edited in Superset",
) )
@ -284,12 +286,13 @@ function PropertiesModal({
onFinish={onSubmit} onFinish={onSubmit}
layout="vertical" layout="vertical"
initialValues={{ initialValues={{
name: slice.slice_name || '', name: slice?.slice_name || '',
description: slice.description || '', description: slice?.description || '',
cache_timeout: slice.cache_timeout != null ? slice.cache_timeout : '', cache_timeout:
certified_by: slice.certified_by || '', slice?.cache_timeout != null ? slice.cache_timeout : '',
certified_by: slice?.certified_by || '',
certification_details: certification_details:
slice.certified_by && slice.certification_details slice?.certified_by && slice?.certification_details
? slice.certification_details ? slice.certification_details
: '', : '',
}} }}

15
superset-frontend/src/features/home/ChartTable.tsx
View File

@ -171,15 +171,12 @@ function ChartTable({
if (loading) return <LoadingCards cover={showThumbnails} />; if (loading) return <LoadingCards cover={showThumbnails} />;
return ( return (
<ErrorBoundary> <ErrorBoundary>
{sliceCurrentlyEditing && ( <PropertiesModal
<PropertiesModal onHide={closeChartEditModal}
onHide={closeChartEditModal} onSave={handleChartUpdated}
onSave={handleChartUpdated} show={sliceCurrentlyEditing}
show slice={sliceCurrentlyEditing}
slice={sliceCurrentlyEditing} />
/>
)}
<SubMenu <SubMenu
activeChild={activeTab} activeChild={activeTab}
tabs={menuTabs} tabs={menuTabs}

9
superset-frontend/src/pages/ChartList/ChartList.test.jsx
View File

@ -152,6 +152,11 @@ describe('ChartList', () => {
expect(wrapper.find(ChartList)).toExist(); expect(wrapper.find(ChartList)).toExist();
}); });
it('renders, but PropertiesModal initially hidden', () => {
expect(wrapper.find(PropertiesModal).exists()).toBe(true);
expect(wrapper.find(PropertiesModal).prop('show')).toBe(false);
});
it('renders a ListView', () => { it('renders a ListView', () => {
expect(wrapper.find(ListView)).toExist(); expect(wrapper.find(ListView)).toExist();
}); });
@ -181,10 +186,10 @@ describe('ChartList', () => {
}); });
it('edits', async () => { it('edits', async () => {
expect(wrapper.find(PropertiesModal)).not.toExist(); expect(wrapper.find(PropertiesModal).prop('show')).toBe(false);
wrapper.find('[data-test="edit-alt"]').first().simulate('click'); wrapper.find('[data-test="edit-alt"]').first().simulate('click');
await waitForComponentToPaint(wrapper); await waitForComponentToPaint(wrapper);
expect(wrapper.find(PropertiesModal)).toExist(); expect(wrapper.find(PropertiesModal).prop('show')).toBe(true);
}); });
it('delete', async () => { it('delete', async () => {

14
superset-frontend/src/pages/ChartList/index.tsx
View File

@ -783,14 +783,12 @@ function ChartList(props: ChartListProps) {
return ( return (
<> <>
<SubMenu name={t('Charts')} buttons={subMenuButtons} /> <SubMenu name={t('Charts')} buttons={subMenuButtons} />
{sliceCurrentlyEditing && ( <PropertiesModal
<PropertiesModal onHide={closeChartEditModal}
onHide={closeChartEditModal} onSave={handleChartUpdated}
onSave={handleChartUpdated} show={!!sliceCurrentlyEditing}
show slice={sliceCurrentlyEditing}
slice={sliceCurrentlyEditing} />
/>
)}
<ConfirmStatusChange <ConfirmStatusChange
title={t('Please confirm')} title={t('Please confirm')}
description={t('Are you sure you want to delete the selected charts?')} description={t('Are you sure you want to delete the selected charts?')}