From 9479a639ef0535c8847d7fdba0250edfbc5224c2 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 20 Jan 2022 12:42:52 +0100 Subject: [PATCH 1/7] feat(explore): add toasts feedback when user copies chart url --- .../src/explore/components/ExploreActionButtons.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/superset-frontend/src/explore/components/ExploreActionButtons.tsx b/superset-frontend/src/explore/components/ExploreActionButtons.tsx index 8e2738d3bfe48..621fee2da0261 100644 --- a/superset-frontend/src/explore/components/ExploreActionButtons.tsx +++ b/superset-frontend/src/explore/components/ExploreActionButtons.tsx @@ -18,11 +18,13 @@ */ import React, { ReactElement, useState } from 'react'; import cx from 'classnames'; +import { useDispatch } from 'react-redux'; import { QueryFormData, t } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import { Tooltip } from 'src/components/Tooltip'; import copyTextToClipboard from 'src/utils/copy'; import withToasts from 'src/components/MessageToasts/withToasts'; +import { addSuccessToast } from 'src/components/MessageToasts/actions'; import { useUrlShortener } from 'src/hooks/useUrlShortener'; import EmbedCodeButton from './EmbedCodeButton'; import { exportChart, getExploreLongUrl } from '../exploreUtils'; @@ -100,6 +102,7 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => { addDangerToast, } = props; + const dispatch = useDispatch(); const copyTooltipText = t('Copy chart URL to clipboard'); const [copyTooltip, setCopyTooltip] = useState(copyTooltipText); const longUrl = getExploreLongUrl(latestQueryFormData); @@ -111,8 +114,14 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => { const shortUrl = await getShortUrl(); await copyTextToClipboard(shortUrl); setCopyTooltip(t('Copied to clipboard!')); + dispatch(addSuccessToast(t('Copied to clipboard!'))); } catch (error) { setCopyTooltip(t('Sorry, your browser does not support copying.')); + dispatch( + addDangerToast(t('Sorry, your browser does not support copying.'), { + noDuplicate: true, + }), + ); } }; From c4e0017520158cd7f24f37c000f84a1b23af33f8 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 20 Jan 2022 13:16:07 +0100 Subject: [PATCH 2/7] Show toast message when updating chart properties --- .../src/explore/components/PropertiesModal/index.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx index 2b2e9b56ea512..1c69251fadd3b 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx @@ -20,12 +20,14 @@ import React, { useMemo, useState, useCallback, useEffect } from 'react'; import Modal from 'src/components/Modal'; import { Form, Row, Col, Input, TextArea } from 'src/common/components'; import Button from 'src/components/Button'; +import { addSuccessToast } from 'src/components/MessageToasts/actions'; import { Select } from 'src/components'; import { SelectValue } from 'antd/lib/select'; import rison from 'rison'; import { t, SupersetClient, styled } from '@superset-ui/core'; import Chart, { Slice } from 'src/types/Chart'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import { useDispatch } from 'react-redux'; type PropertiesModalProps = { slice: Slice; @@ -59,6 +61,7 @@ export default function PropertiesModal({ const [selectedOwners, setSelectedOwners] = useState( null, ); + const dispatch = useDispatch(); function showError({ error, statusText, message }: any) { let errorText = error || statusText || t('An error has occurred'); @@ -157,6 +160,7 @@ export default function PropertiesModal({ id: slice.slice_id, }; onSave(updatedChart); + dispatch(addSuccessToast(t('Chart properties updated'))); onHide(); } catch (res) { const clientError = await getClientErrorObject(res); From 11ba0b2f942cd749d8c1bd8f03c4d6dc6be31e23 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 20 Jan 2022 13:17:33 +0100 Subject: [PATCH 3/7] Change toast type to success when saving chart --- superset/views/core.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index ca7dae3ee51e7..8e115a97ce5de 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -975,11 +975,11 @@ def save_or_overwrite_slice( if action == "saveas" and slice_add_perm: ChartDAO.save(slc) msg = _("Chart [{}] has been saved").format(slc.slice_name) - flash(msg, "info") + flash(msg, "success") elif action == "overwrite" and slice_overwrite_perm: ChartDAO.overwrite(slc) msg = _("Chart [{}] has been overwritten").format(slc.slice_name) - flash(msg, "info") + flash(msg, "success") # Adding slice to a dashboard if requested dash: Optional[Dashboard] = None @@ -1008,7 +1008,7 @@ def save_or_overwrite_slice( _("Chart [{}] was added to dashboard [{}]").format( slc.slice_name, dash.dashboard_title ), - "info", + "success", ) elif new_dashboard_name: # Creating and adding to a new dashboard @@ -1030,7 +1030,7 @@ def save_or_overwrite_slice( _( "Dashboard [{}] just got created and chart [{}] was added " "to it" ).format(dash.dashboard_title, slc.slice_name), - "info", + "success", ) if dash and slc not in dash.slices: From 97e74974682625a5333f24788a49dc044044b9f0 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 20 Jan 2022 13:46:10 +0100 Subject: [PATCH 4/7] Use success toast from props --- .../src/explore/components/ExploreActionButtons.tsx | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/explore/components/ExploreActionButtons.tsx b/superset-frontend/src/explore/components/ExploreActionButtons.tsx index 621fee2da0261..5d4ac7b5cc4be 100644 --- a/superset-frontend/src/explore/components/ExploreActionButtons.tsx +++ b/superset-frontend/src/explore/components/ExploreActionButtons.tsx @@ -18,13 +18,11 @@ */ import React, { ReactElement, useState } from 'react'; import cx from 'classnames'; -import { useDispatch } from 'react-redux'; import { QueryFormData, t } from '@superset-ui/core'; import Icons from 'src/components/Icons'; import { Tooltip } from 'src/components/Tooltip'; import copyTextToClipboard from 'src/utils/copy'; import withToasts from 'src/components/MessageToasts/withToasts'; -import { addSuccessToast } from 'src/components/MessageToasts/actions'; import { useUrlShortener } from 'src/hooks/useUrlShortener'; import EmbedCodeButton from './EmbedCodeButton'; import { exportChart, getExploreLongUrl } from '../exploreUtils'; @@ -50,6 +48,7 @@ type ExploreActionButtonsProps = { queriesResponse: {}; slice: { slice_name: string }; addDangerToast: Function; + addSuccessToast: Function; }; const VIZ_TYPES_PIVOTABLE = ['pivot_table', 'pivot_table_v2']; @@ -100,9 +99,9 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => { latestQueryFormData, slice, addDangerToast, + addSuccessToast, } = props; - const dispatch = useDispatch(); const copyTooltipText = t('Copy chart URL to clipboard'); const [copyTooltip, setCopyTooltip] = useState(copyTooltipText); const longUrl = getExploreLongUrl(latestQueryFormData); @@ -114,14 +113,10 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => { const shortUrl = await getShortUrl(); await copyTextToClipboard(shortUrl); setCopyTooltip(t('Copied to clipboard!')); - dispatch(addSuccessToast(t('Copied to clipboard!'))); + addSuccessToast(t('Copied to clipboard!')); } catch (error) { setCopyTooltip(t('Sorry, your browser does not support copying.')); - dispatch( - addDangerToast(t('Sorry, your browser does not support copying.'), { - noDuplicate: true, - }), - ); + addDangerToast(t('Sorry, your browser does not support copying.')); } }; From 693545d7f50f2f5b777aee003f6b1e22f8158f05 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 20 Jan 2022 15:44:48 +0100 Subject: [PATCH 5/7] Fix tests --- .../PropertiesModal/PropertiesModal.test.tsx | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx index 8dba7fbb40c1b..79f3452dc0873 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx @@ -160,10 +160,13 @@ afterAll(() => { fetchMock.resetBehavior(); }); +const renderModal = (props: any) => + render(, { useRedux: true }); + test('Should render null when show:false', async () => { const props = createProps(); props.show = false; - render(); + renderModal(props); await waitFor(() => { expect( @@ -174,7 +177,7 @@ test('Should render null when show:false', async () => { test('Should render when show:true', async () => { const props = createProps(); - render(); + renderModal(props); await waitFor(() => { expect( @@ -185,7 +188,7 @@ test('Should render when show:true', async () => { test('Should have modal header', async () => { const props = createProps(); - render(); + renderModal(props); await waitFor(() => { expect(screen.getByText('Edit Chart Properties')).toBeVisible(); @@ -196,7 +199,7 @@ test('Should have modal header', async () => { test('"Close" button should call "onHide"', async () => { const props = createProps(); - render(); + renderModal(props); await waitFor(() => { expect(props.onHide).toBeCalledTimes(0); @@ -212,7 +215,7 @@ test('"Close" button should call "onHide"', async () => { test('Should render all elements inside modal', async () => { const props = createProps(); - render(); + renderModal(props); await waitFor(() => { expect(screen.getAllByRole('textbox')).toHaveLength(5); expect(screen.getByRole('combobox')).toBeInTheDocument(); @@ -240,7 +243,7 @@ test('Should render all elements inside modal', async () => { test('Should have modal footer', async () => { const props = createProps(); - render(); + renderModal(props); await waitFor(() => { expect(screen.getByText('Cancel')).toBeVisible(); @@ -254,7 +257,7 @@ test('Should have modal footer', async () => { test('"Cancel" button should call "onHide"', async () => { const props = createProps(); - render(); + renderModal(props); await waitFor(() => { expect(props.onHide).toBeCalledTimes(0); @@ -270,7 +273,7 @@ test('"Cancel" button should call "onHide"', async () => { test('"Save" button should call only "onSave"', async () => { const props = createProps(); - render(); + renderModal(props); await waitFor(() => { expect(props.onSave).toBeCalledTimes(0); expect(props.onHide).toBeCalledTimes(0); @@ -294,7 +297,7 @@ test('Empty "Certified by" should clear "Certification details"', async () => { certified_by: '', }, }; - render(); + renderModal(noCertifiedByProps); expect( screen.getByRole('textbox', { name: 'Certification details' }), From e410592382c8ed55e6106f4cff0ff3fff747a8b4 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 20 Jan 2022 16:29:11 +0100 Subject: [PATCH 6/7] Use withToasts instead of dispatch --- .../src/explore/components/PropertiesModal/index.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx index 1c69251fadd3b..22df03cd0d86f 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx @@ -20,14 +20,13 @@ import React, { useMemo, useState, useCallback, useEffect } from 'react'; import Modal from 'src/components/Modal'; import { Form, Row, Col, Input, TextArea } from 'src/common/components'; import Button from 'src/components/Button'; -import { addSuccessToast } from 'src/components/MessageToasts/actions'; import { Select } from 'src/components'; import { SelectValue } from 'antd/lib/select'; import rison from 'rison'; import { t, SupersetClient, styled } from '@superset-ui/core'; import Chart, { Slice } from 'src/types/Chart'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; -import { useDispatch } from 'react-redux'; +import withToasts from 'src/components/MessageToasts/withToasts'; type PropertiesModalProps = { slice: Slice; @@ -36,6 +35,7 @@ type PropertiesModalProps = { onSave: (chart: Chart) => void; permissionsError?: string; existingOwners?: SelectValue; + addSuccessToast: (msg: string) => void; }; const FormItem = Form.Item; @@ -48,11 +48,12 @@ const StyledHelpBlock = styled.span` margin-bottom: 0; `; -export default function PropertiesModal({ +function PropertiesModal({ slice, onHide, onSave, show, + addSuccessToast, }: PropertiesModalProps) { const [submitting, setSubmitting] = useState(false); const [form] = Form.useForm(); @@ -61,7 +62,6 @@ export default function PropertiesModal({ const [selectedOwners, setSelectedOwners] = useState( null, ); - const dispatch = useDispatch(); function showError({ error, statusText, message }: any) { let errorText = error || statusText || t('An error has occurred'); @@ -160,7 +160,7 @@ export default function PropertiesModal({ id: slice.slice_id, }; onSave(updatedChart); - dispatch(addSuccessToast(t('Chart properties updated'))); + addSuccessToast(t('Chart properties updated')); onHide(); } catch (res) { const clientError = await getClientErrorObject(res); @@ -312,3 +312,5 @@ export default function PropertiesModal({ ); } + +export default withToasts(PropertiesModal); From c1d0ec93c0dace29d193a50dda857b33524a3766 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 21 Jan 2022 17:26:19 +0100 Subject: [PATCH 7/7] Use PropertiesModalProps instead of any --- .../components/PropertiesModal/PropertiesModal.test.tsx | 5 +++-- .../src/explore/components/PropertiesModal/index.tsx | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx index 79f3452dc0873..6fc4bc88a43bd 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx @@ -22,7 +22,7 @@ import { Slice } from 'src/types/Chart'; import { render, screen, waitFor } from 'spec/helpers/testing-library'; import fetchMock from 'fetch-mock'; import userEvent from '@testing-library/user-event'; -import PropertiesModal from '.'; +import PropertiesModal, { PropertiesModalProps } from '.'; const createProps = () => ({ slice: { @@ -68,6 +68,7 @@ const createProps = () => ({ show: true, onHide: jest.fn(), onSave: jest.fn(), + addSuccessToast: jest.fn(), }); fetchMock.get('glob:*/api/v1/chart/318', { @@ -160,7 +161,7 @@ afterAll(() => { fetchMock.resetBehavior(); }); -const renderModal = (props: any) => +const renderModal = (props: PropertiesModalProps) => render(, { useRedux: true }); test('Should render null when show:false', async () => { diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx index 22df03cd0d86f..022b74f02907d 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx @@ -28,7 +28,7 @@ import Chart, { Slice } from 'src/types/Chart'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import withToasts from 'src/components/MessageToasts/withToasts'; -type PropertiesModalProps = { +export type PropertiesModalProps = { slice: Slice; show: boolean; onHide: () => void;