Skip to content

Commit

Permalink
feat(explore): more toast feedback on user actions in Explore (#18108)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
kgabryje authored Jan 24, 2022
1 parent d9eef8e commit e632193
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type ExploreActionButtonsProps = {
queriesResponse: {};
slice: { slice_name: string };
addDangerToast: Function;
addSuccessToast: Function;
};

const VIZ_TYPES_PIVOTABLE = ['pivot_table', 'pivot_table_v2'];
Expand Down Expand Up @@ -98,6 +99,7 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
latestQueryFormData,
slice,
addDangerToast,
addSuccessToast,
} = props;

const copyTooltipText = t('Copy chart URL to clipboard');
Expand All @@ -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.'));
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -68,6 +68,7 @@ const createProps = () => ({
show: true,
onHide: jest.fn(),
onSave: jest.fn(),
addSuccessToast: jest.fn(),
});

fetchMock.get('glob:*/api/v1/chart/318', {
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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' }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -308,3 +312,5 @@ export default function PropertiesModal({
</Modal>
);
}

export default withToasts(PropertiesModal);
8 changes: 4 additions & 4 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down

0 comments on commit e632193

Please sign in to comment.