You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2024/08/28 17:41:37 UTC
(superset) branch revert-28796-fix-properties-modal-transitions created (now a4bb2b5dba)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch revert-28796-fix-properties-modal-transitions
in repository https://gitbox.apache.org/repos/asf/superset.git
at a4bb2b5dba Revert "fix(list/chart views): Chart Properties modal now has transitions (#2…"
This branch includes the following new commits:
new a4bb2b5dba Revert "fix(list/chart views): Chart Properties modal now has transitions (#2…"
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: Revert "fix(list/chart views): Chart Properties modal now has transitions (#2…"
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch revert-28796-fix-properties-modal-transitions
in repository https://gitbox.apache.org/repos/asf/superset.git
commit a4bb2b5dbacefd8695d44e0ad044bc0541d82faa
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Wed Aug 28 11:41:26 2024 -0600
Revert "fix(list/chart views): Chart Properties modal now has transitions (#2…"
This reverts commit 66eb9593d1807bf44f2c1b9dd46e1ff2013bfb3e.
---
.../explore/components/PropertiesModal/index.tsx | 61 ++++++++++------------
superset-frontend/src/features/home/ChartTable.tsx | 15 +++---
.../src/pages/ChartList/ChartList.test.jsx | 9 +---
superset-frontend/src/pages/ChartList/index.tsx | 14 ++---
4 files changed, 48 insertions(+), 51 deletions(-)
diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx
index 9f502e05f1..9a6831fc59 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<SelectValue | null>(
null,
);
@@ -98,25 +98,23 @@ function PropertiesModal({
const fetchChartOwners = useCallback(
async function fetchChartOwners() {
- 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);
- }
+ 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(
@@ -177,7 +175,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),
});
@@ -186,7 +184,7 @@ function PropertiesModal({
...payload,
...res.json.result,
tags,
- id: slice?.slice_id,
+ id: slice.slice_id,
owners: selectedOwners,
};
onSave(updatedChart);
@@ -208,8 +206,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;
@@ -217,7 +215,7 @@ function PropertiesModal({
fetchTags(
{
objectType: OBJECT_TYPES.CHART,
- objectId: slice?.slice_id,
+ objectId: slice.slice_id,
includeTypes: false,
},
(tags: TagType[]) => setTags(tags),
@@ -228,7 +226,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 => ({
@@ -264,9 +262,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",
)
@@ -286,13 +284,12 @@ 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 46c6bbfc26..30d1d41afa 100644
--- a/superset-frontend/src/features/home/ChartTable.tsx
+++ b/superset-frontend/src/features/home/ChartTable.tsx
@@ -171,12 +171,15 @@ function ChartTable({
if (loading) return <LoadingCards cover={showThumbnails} />;
return (
<ErrorBoundary>
- <PropertiesModal
- onHide={closeChartEditModal}
- onSave={handleChartUpdated}
- show={sliceCurrentlyEditing}
- slice={sliceCurrentlyEditing}
- />
+ {sliceCurrentlyEditing && (
+ <PropertiesModal
+ onHide={closeChartEditModal}
+ onSave={handleChartUpdated}
+ show
+ slice={sliceCurrentlyEditing}
+ />
+ )}
+
<SubMenu
activeChild={activeTab}
tabs={menuTabs}
diff --git a/superset-frontend/src/pages/ChartList/ChartList.test.jsx b/superset-frontend/src/pages/ChartList/ChartList.test.jsx
index 9f9200e1f6..6e1c0728f0 100644
--- a/superset-frontend/src/pages/ChartList/ChartList.test.jsx
+++ b/superset-frontend/src/pages/ChartList/ChartList.test.jsx
@@ -152,11 +152,6 @@ describe('ChartList', () => {
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();
});
@@ -186,10 +181,10 @@ describe('ChartList', () => {
});
it('edits', async () => {
- expect(wrapper.find(PropertiesModal).prop('show')).toBe(false);
+ expect(wrapper.find(PropertiesModal)).not.toExist();
wrapper.find('[data-test="edit-alt"]').first().simulate('click');
await waitForComponentToPaint(wrapper);
- expect(wrapper.find(PropertiesModal).prop('show')).toBe(true);
+ expect(wrapper.find(PropertiesModal)).toExist();
});
it('delete', async () => {
diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx
index ce00d348d0..031722ab22 100644
--- a/superset-frontend/src/pages/ChartList/index.tsx
+++ b/superset-frontend/src/pages/ChartList/index.tsx
@@ -787,12 +787,14 @@ function ChartList(props: ChartListProps) {
return (
<>
<SubMenu name={t('Charts')} buttons={subMenuButtons} />
- <PropertiesModal
- onHide={closeChartEditModal}
- onSave={handleChartUpdated}
- show={!!sliceCurrentlyEditing}
- slice={sliceCurrentlyEditing}
- />
+ {sliceCurrentlyEditing && (
+ <PropertiesModal
+ onHide={closeChartEditModal}
+ onSave={handleChartUpdated}
+ show
+ slice={sliceCurrentlyEditing}
+ />
+ )}
<ConfirmStatusChange
title={t('Please confirm')}
description={t('Are you sure you want to delete the selected charts?')}