feat(explore): more toast feedback on user actions in Explore (#18108)

* feat(explore): add toasts feedback when user copies chart url

* Show toast message when updating chart properties

* Change toast type to success when saving chart

* Use success toast from props

* Fix tests

* Use withToasts instead of dispatch

* Use PropertiesModalProps instead of any
This commit is contained in:
Kamil Gabryjelski 2022-01-24 11:42:24 +01:00 committed by GitHub
parent d9eef8ec2f
commit e632193eb0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 30 additions and 16 deletions

View File

@ -48,6 +48,7 @@ type ExploreActionButtonsProps = {
queriesResponse: {};
slice: { slice_name: string };
addDangerToast: Function;
addSuccessToast: Function;
};
const VIZ_TYPES_PIVOTABLE = ['pivot_table', 'pivot_table_v2'];
@ -98,6 +99,7 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
latestQueryFormData,
slice,
addDangerToast,
addSuccessToast,
} = props;
const copyTooltipText = t('Copy chart URL to clipboard');
@ -111,8 +113,10 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
const shortUrl = await getShortUrl();
await copyTextToClipboard(shortUrl);
setCopyTooltip(t('Copied to clipboard!'));
addSuccessToast(t('Copied to clipboard!'));
} catch (error) {
setCopyTooltip(t('Sorry, your browser does not support copying.'));
addDangerToast(t('Sorry, your browser does not support copying.'));
}
};

View File

@ -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,10 +161,13 @@ afterAll(() => {
fetchMock.resetBehavior();
});
const renderModal = (props: PropertiesModalProps) =>
render(<PropertiesModal {...props} />, { useRedux: true });
test('Should render null when show:false', async () => {
const props = createProps();
props.show = false;
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(
@ -174,7 +178,7 @@ test('Should render null when show:false', async () => {
test('Should render when show:true', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(
@ -185,7 +189,7 @@ test('Should render when show:true', async () => {
test('Should have modal header', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(screen.getByText('Edit Chart Properties')).toBeVisible();
@ -196,7 +200,7 @@ test('Should have modal header', async () => {
test('"Close" button should call "onHide"', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(props.onHide).toBeCalledTimes(0);
@ -212,7 +216,7 @@ test('"Close" button should call "onHide"', async () => {
test('Should render all elements inside modal', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(screen.getAllByRole('textbox')).toHaveLength(5);
expect(screen.getByRole('combobox')).toBeInTheDocument();
@ -240,7 +244,7 @@ test('Should render all elements inside modal', async () => {
test('Should have modal footer', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(screen.getByText('Cancel')).toBeVisible();
@ -254,7 +258,7 @@ test('Should have modal footer', async () => {
test('"Cancel" button should call "onHide"', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(props.onHide).toBeCalledTimes(0);
@ -270,7 +274,7 @@ test('"Cancel" button should call "onHide"', async () => {
test('"Save" button should call only "onSave"', async () => {
const props = createProps();
render(<PropertiesModal {...props} />);
renderModal(props);
await waitFor(() => {
expect(props.onSave).toBeCalledTimes(0);
expect(props.onHide).toBeCalledTimes(0);
@ -294,7 +298,7 @@ test('Empty "Certified by" should clear "Certification details"', async () => {
certified_by: '',
},
};
render(<PropertiesModal {...noCertifiedByProps} />);
renderModal(noCertifiedByProps);
expect(
screen.getByRole('textbox', { name: 'Certification details' }),

View File

@ -26,14 +26,16 @@ 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 withToasts from 'src/components/MessageToasts/withToasts';
type PropertiesModalProps = {
export type PropertiesModalProps = {
slice: Slice;
show: boolean;
onHide: () => void;
onSave: (chart: Chart) => void;
permissionsError?: string;
existingOwners?: SelectValue;
addSuccessToast: (msg: string) => void;
};
const FormItem = Form.Item;
@ -46,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();
@ -157,6 +160,7 @@ export default function PropertiesModal({
id: slice.slice_id,
};
onSave(updatedChart);
addSuccessToast(t('Chart properties updated'));
onHide();
} catch (res) {
const clientError = await getClientErrorObject(res);
@ -308,3 +312,5 @@ export default function PropertiesModal({
</Modal>
);
}
export default withToasts(PropertiesModal);

View File

@ -975,11 +975,11 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
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 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
_("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 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
_(
"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: