From 66eb9593d1807bf44f2c1b9dd46e1ff2013bfb3e Mon Sep 17 00:00:00 2001 From: Evan Rusackas Date: Wed, 31 Jul 2024 17:57:53 -0600 Subject: [PATCH] fix(list/chart views): Chart Properties modal now has transitions (#28796) Co-authored-by: JUST.in DO IT --- .../components/PropertiesModal/index.tsx | 61 ++++++++++--------- .../src/features/home/ChartTable.tsx | 15 ++--- .../src/pages/ChartList/ChartList.test.jsx | 9 ++- .../src/pages/ChartList/index.tsx | 14 ++--- 4 files changed, 51 insertions(+), 48 deletions(-) diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx index 9a6831fc595c4..9f502e05f1e4f 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx @@ -69,7 +69,7 @@ function PropertiesModal({ const [submitting, setSubmitting] = useState(false); const [form] = AntdForm.useForm(); // values of form inputs - const [name, setName] = useState(slice.slice_name || ''); + const [name, setName] = useState(slice?.slice_name || ''); const [selectedOwners, setSelectedOwners] = useState( null, ); @@ -98,23 +98,25 @@ function PropertiesModal({ const fetchChartOwners = useCallback( async function fetchChartOwners() { - try { - const response = await SupersetClient.get({ - endpoint: `/api/v1/chart/${slice.slice_id}`, - }); - const chart = response.json.result; - setSelectedOwners( - chart?.owners?.map((owner: any) => ({ - value: owner.id, - label: `${owner.first_name} ${owner.last_name}`, - })), - ); - } catch (response) { - const clientError = await getClientErrorObject(response); - showError(clientError); + if (slice?.slice_id) { + try { + const response = await SupersetClient.get({ + endpoint: `/api/v1/chart/${slice?.slice_id}`, + }); + const chart = response.json.result; + setSelectedOwners( + chart?.owners?.map((owner: any) => ({ + value: owner.id, + label: `${owner.first_name} ${owner.last_name}`, + })), + ); + } catch (response) { + const clientError = await getClientErrorObject(response); + showError(clientError); + } } }, - [slice.slice_id], + [slice?.slice_id], ); const loadOptions = useMemo( @@ -175,7 +177,7 @@ function PropertiesModal({ try { const res = await SupersetClient.put({ - endpoint: `/api/v1/chart/${slice.slice_id}`, + endpoint: `/api/v1/chart/${slice?.slice_id}`, headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(payload), }); @@ -184,7 +186,7 @@ function PropertiesModal({ ...payload, ...res.json.result, tags, - id: slice.slice_id, + id: slice?.slice_id, owners: selectedOwners, }; onSave(updatedChart); @@ -206,8 +208,8 @@ function PropertiesModal({ // update name after it's changed in another modal useEffect(() => { - setName(slice.slice_name || ''); - }, [slice.slice_name]); + setName(slice?.slice_name || ''); + }, [slice?.slice_name]); useEffect(() => { if (!isFeatureEnabled(FeatureFlag.TaggingSystem)) return; @@ -215,7 +217,7 @@ function PropertiesModal({ fetchTags( { objectType: OBJECT_TYPES.CHART, - objectId: slice.slice_id, + objectId: slice?.slice_id, includeTypes: false, }, (tags: TagType[]) => setTags(tags), @@ -226,7 +228,7 @@ function PropertiesModal({ } catch (error) { showError(error); } - }, [slice.slice_id]); + }, [slice?.slice_id]); const handleChangeTags = (tags: { label: string; value: number }[]) => { const parsedTags: TagType[] = ensureIsArray(tags).map(r => ({ @@ -262,9 +264,9 @@ function PropertiesModal({ buttonSize="small" buttonStyle="primary" onClick={form.submit} - disabled={submitting || !name || slice.is_managed_externally} + disabled={submitting || !name || slice?.is_managed_externally} tooltip={ - slice.is_managed_externally + slice?.is_managed_externally ? t( "This chart is managed externally, and can't be edited in Superset", ) @@ -284,12 +286,13 @@ function PropertiesModal({ onFinish={onSubmit} layout="vertical" initialValues={{ - name: slice.slice_name || '', - description: slice.description || '', - cache_timeout: slice.cache_timeout != null ? slice.cache_timeout : '', - certified_by: slice.certified_by || '', + name: slice?.slice_name || '', + description: slice?.description || '', + cache_timeout: + slice?.cache_timeout != null ? slice.cache_timeout : '', + certified_by: slice?.certified_by || '', certification_details: - slice.certified_by && slice.certification_details + slice?.certified_by && slice?.certification_details ? slice.certification_details : '', }} diff --git a/superset-frontend/src/features/home/ChartTable.tsx b/superset-frontend/src/features/home/ChartTable.tsx index 30d1d41afaa44..46c6bbfc2695a 100644 --- a/superset-frontend/src/features/home/ChartTable.tsx +++ b/superset-frontend/src/features/home/ChartTable.tsx @@ -171,15 +171,12 @@ function ChartTable({ if (loading) return ; return ( - {sliceCurrentlyEditing && ( - - )} - + { 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', () => { expect(wrapper.find(ListView)).toExist(); }); @@ -181,10 +186,10 @@ describe('ChartList', () => { }); 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'); await waitForComponentToPaint(wrapper); - expect(wrapper.find(PropertiesModal)).toExist(); + expect(wrapper.find(PropertiesModal).prop('show')).toBe(true); }); it('delete', async () => { diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx index 6650583534bdf..60c7b5dafeed4 100644 --- a/superset-frontend/src/pages/ChartList/index.tsx +++ b/superset-frontend/src/pages/ChartList/index.tsx @@ -783,14 +783,12 @@ function ChartList(props: ChartListProps) { return ( <> - {sliceCurrentlyEditing && ( - - )} +