diff --git a/frontend/src/metabase-types/store/undo.ts b/frontend/src/metabase-types/store/undo.ts index 75f4450e26869b3235b4960261053076bdd30c94..d1c4a67892975ac6eb367eb427f5dd558bf7ae1b 100644 --- a/frontend/src/metabase-types/store/undo.ts +++ b/frontend/src/metabase-types/store/undo.ts @@ -1,28 +1,31 @@ +import type { IconName } from "metabase/ui"; import type { DashCardId, DashboardTabId } from "metabase-types/api"; export interface Undo { id: string | number; type?: string; - action?: () => void; - message?: string; + action?: (() => void) | null; + message?: string | ((undo: Undo) => string); timeout?: number; + initialTimeout?: number; actions?: (() => void)[]; showProgress?: boolean; - icon?: string | null; + icon?: IconName | null; toastColor?: string; actionLabel?: string; canDismiss?: boolean; startedAt?: number; - pausedAt?: number; + pausedAt?: number | null; dismissIconColor?: string; extraInfo?: { dashcardIds?: DashCardId[]; tabId?: DashboardTabId } & Record< string, unknown >; _domId?: string | number; - timeoutId?: number; + timeoutId: number | null; count?: number; + verb?: string; + subject?: string; } -// TODO: convert redux/undo and UndoListing.jsx to TS and update type export type UndoState = Undo[]; diff --git a/frontend/src/metabase/admin/performance/components/ModelPersistenceConfiguration.tsx b/frontend/src/metabase/admin/performance/components/ModelPersistenceConfiguration.tsx index 09ac46ee66e3eee0b451315cf514fbd7ba8740b3..26fd9ca2438f2d875d60241b23d28554a63d7852 100644 --- a/frontend/src/metabase/admin/performance/components/ModelPersistenceConfiguration.tsx +++ b/frontend/src/metabase/admin/performance/components/ModelPersistenceConfiguration.tsx @@ -80,7 +80,7 @@ export const ModelPersistenceConfiguration = () => { const dismissLoadingToast = useCallback( (toastId: number) => { - dispatch(dismissUndo(toastId)); + dispatch(dismissUndo({ undoId: toastId })); }, [dispatch], ); diff --git a/frontend/src/metabase/components/SaveStatus/SaveStatus.jsx b/frontend/src/metabase/components/SaveStatus/SaveStatus.jsx index 73bf5245746a83a22f0090f0c868529452a2c3de..b735569e59b2194a3db2b1d909b5ddd640c653b7 100644 --- a/frontend/src/metabase/components/SaveStatus/SaveStatus.jsx +++ b/frontend/src/metabase/components/SaveStatus/SaveStatus.jsx @@ -55,7 +55,7 @@ class SaveStatus extends Component { const mapDispatchToProps = dispatch => ({ notify: undo => dispatch(addUndo(undo)), - unnotify: undoId => dispatch(dismissUndo(undoId)), + unnotify: undoId => dispatch(dismissUndo({ undoId })), }); export default _.compose( diff --git a/frontend/src/metabase/containers/UndoListing.styled.tsx b/frontend/src/metabase/containers/UndoListing.styled.tsx index 7c6cb953069b06366cf50018a7582887dd1eae5c..8ffc4f0b464b26e6ee986d76f6974cfa480f2298 100644 --- a/frontend/src/metabase/containers/UndoListing.styled.tsx +++ b/frontend/src/metabase/containers/UndoListing.styled.tsx @@ -22,7 +22,7 @@ export const UndoList = styled.ul` `; export const ToastCard = styled(Card)<{ - translateY: number; + translateY?: number; color?: string; noBorder?: boolean; }>` @@ -31,7 +31,8 @@ export const ToastCard = styled(Card)<{ min-width: 310px; max-width: calc(100vw - 2 * ${LIST_H_MARGINS}); position: relative; - transform: ${props => `translateY(${props.translateY}px)`}; + ${props => + props.translateY ? `transform: translateY(${props.translateY}px)` : ""} ${props => (props.color ? `background-color: ${color(props.color)}` : "")}; ${({ noBorder }) => noBorder && diff --git a/frontend/src/metabase/containers/UndoListing.jsx b/frontend/src/metabase/containers/UndoListing.tsx similarity index 90% rename from frontend/src/metabase/containers/UndoListing.jsx rename to frontend/src/metabase/containers/UndoListing.tsx index 6692dc4f3ac9d4e4e36be25c24476e7f4e963198..61ceac9210f448f68c3c3c5a53b65a3ccf1b5176 100644 --- a/frontend/src/metabase/containers/UndoListing.jsx +++ b/frontend/src/metabase/containers/UndoListing.tsx @@ -14,6 +14,7 @@ import { resumeUndo, } from "metabase/redux/undo"; import { Progress, Transition } from "metabase/ui"; +import type { Undo } from "metabase-types/store/undo"; import CS from "./UndoListing.module.css"; import { @@ -28,12 +29,10 @@ import { UndoList, } from "./UndoListing.styled"; -DefaultMessage.propTypes = { - undo: PropTypes.object.isRequired, -}; - function DefaultMessage({ undo: { verb = t`modified`, count = 1, subject = t`item` }, +}: { + undo: Undo; }) { return ( <DefaultText> @@ -44,7 +43,7 @@ function DefaultMessage({ ); } -function renderMessage(undo) { +function renderMessage(undo: Undo) { const { message } = undo; if (!message) { return <DefaultMessage undo={undo || {}} />; @@ -67,7 +66,13 @@ const slideIn = { const TOAST_TRANSITION_DURATION = 300; -function UndoToast({ undo, onUndo, onDismiss }) { +interface UndoToastProps { + undo: Undo; + onUndo: () => void; + onDismiss: () => void; +} + +function UndoToast({ undo, onUndo, onDismiss }: UndoToastProps) { const dispatch = useDispatch(); const [mounted, setMounted] = useState(false); const [paused, setPaused] = useState(false); @@ -137,8 +142,8 @@ function UndoToast({ undo, onUndo, onDismiss }) { </Ellipsified> </CardContentSide> <ControlsCardContent> - {undo.actions?.length > 0 && ( - <UndoButton role="button" onClick={onUndo}> + {undo.actions && undo.actions.length > 0 && ( + <UndoButton role="button" onClick={onUndo} to=""> {undo.actionLabel ?? t`Undo`} </UndoButton> )} @@ -167,7 +172,7 @@ function UndoListingInner() { key={undo._domId} undo={undo} onUndo={() => dispatch(performUndo(undo.id))} - onDismiss={() => dispatch(dismissUndo(undo.id))} + onDismiss={() => dispatch(dismissUndo({ undoId: undo.id }))} /> ))} </UndoList> diff --git a/frontend/src/metabase/containers/UndoListing.unit.spec.tsx b/frontend/src/metabase/containers/UndoListing.unit.spec.tsx index 08ab7535b4fcdc22197c26aa9f251b96943c0a9f..e5a0d02475f8e64e02a86895abf501d48e7628b0 100644 --- a/frontend/src/metabase/containers/UndoListing.unit.spec.tsx +++ b/frontend/src/metabase/containers/UndoListing.unit.spec.tsx @@ -1,14 +1,15 @@ import { renderWithProviders, screen } from "__support__/ui"; -import type { UndoState } from "metabase-types/store/undo"; +import type { Undo } from "metabase-types/store/undo"; import { UndoListing } from "./UndoListing"; -const AUTO_CONNECT_UNDO: UndoState[number] = { +const AUTO_CONNECT_UNDO: Undo = { icon: null, message: "Auto-connect this filter to all questions containing “Product.Titleâ€, in the current tab?", actionLabel: "Auto-connect", timeout: 12000, + timeoutId: null, id: 0, _domId: 1, canDismiss: true, diff --git a/frontend/src/metabase/dashboard/actions/auto-wire-parameters/toasts.ts b/frontend/src/metabase/dashboard/actions/auto-wire-parameters/toasts.ts index cda2ca467844cfa22988d8b7f7f376d6048bd5ec..e66af6c37a4433ff7689353d782ac7b096954be9 100644 --- a/frontend/src/metabase/dashboard/actions/auto-wire-parameters/toasts.ts +++ b/frontend/src/metabase/dashboard/actions/auto-wire-parameters/toasts.ts @@ -169,7 +169,7 @@ export const showAddedCardAutoWireParametersToast = export const closeAutoWireParameterToast = (toastId: string = AUTO_WIRE_TOAST_ID) => (dispatch: Dispatch) => { - dispatch(dismissUndo(toastId, false)); + dispatch(dismissUndo({ undoId: toastId, track: false })); }; const autoWireToastTypes = ["filterAutoConnect", "filterAutoConnectDone"]; @@ -179,7 +179,7 @@ export const closeAddCardAutoWireToasts = for (const undo of undos) { if (undo.type && autoWireToastTypes.includes(undo.type)) { - dispatch(dismissUndo(undo.id, false)); + dispatch(dismissUndo({ undoId: undo.id, track: false })); } } }; diff --git a/frontend/src/metabase/dashboard/actions/parameters.ts b/frontend/src/metabase/dashboard/actions/parameters.ts index 11dd3fd3d3b16762d130865f9554f089c31e3f0e..18c220c3093be606cef173835e16340324e61255 100644 --- a/frontend/src/metabase/dashboard/actions/parameters.ts +++ b/frontend/src/metabase/dashboard/actions/parameters.ts @@ -664,7 +664,7 @@ export const closeAutoApplyFiltersToast = createThunkAction( () => (dispatch, getState) => { const toastId = getAutoApplyFiltersToastId(getState()); if (toastId) { - dispatch(dismissUndo(toastId, false)); + dispatch(dismissUndo({ undoId: toastId, track: false })); } }, ); diff --git a/frontend/src/metabase/dashboard/components/DashCard/DashCardParameterMapper/DashCardCardParameterMapper.tsx b/frontend/src/metabase/dashboard/components/DashCard/DashCardParameterMapper/DashCardCardParameterMapper.tsx index dfeb390782dfc9c587ed7e399c57843327d4f0b7..4196fd57713cdb779b222913bbd5530da28574f4 100644 --- a/frontend/src/metabase/dashboard/components/DashCard/DashCardParameterMapper/DashCardCardParameterMapper.tsx +++ b/frontend/src/metabase/dashboard/components/DashCard/DashCardParameterMapper/DashCardCardParameterMapper.tsx @@ -95,7 +95,6 @@ const mapStateToProps = ( mappingOptions: getDashcardParameterMappingOptions(state, props), isRecentlyAutoConnected: getIsRecentlyAutoConnectedDashcard( state, - // @ts-expect-error redux/undo is not in TS props, editingParameter?.id, ), diff --git a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx index a9a9d9069e622bf8b1c7924a6afe584e9318a676..726998ab449857a28fe8b92603be3152da3bac17 100644 --- a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx +++ b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.tsx @@ -137,7 +137,7 @@ const DashboardApp = (props: DashboardAppProps) => { } return () => { - dispatch(dismissUndo(slowToastId)); + dispatch(dismissUndo({ undoId: slowToastId })); }; }, [ dashboard?.name, @@ -149,7 +149,7 @@ const DashboardApp = (props: DashboardAppProps) => { const onConfirmToast = useCallback(async () => { await requestPermission(); - dispatch(dismissUndo(slowToastId)); + dispatch(dismissUndo({ undoId: slowToastId })); }, [dispatch, requestPermission, slowToastId]); const onTimeout = useCallback(() => { diff --git a/frontend/src/metabase/home/components/CustomHomePageModal/CustomHomePageModal.tsx b/frontend/src/metabase/home/components/CustomHomePageModal/CustomHomePageModal.tsx index cb62f6808e596ca62efbf80264d95276addc828a..f21fafcb27f0b9cfe6476c9f2fbcb3fca7ef652f 100644 --- a/frontend/src/metabase/home/components/CustomHomePageModal/CustomHomePageModal.tsx +++ b/frontend/src/metabase/home/components/CustomHomePageModal/CustomHomePageModal.tsx @@ -59,7 +59,7 @@ export const CustomHomePageModal = ({ icon: "info", timeout: 10000, id, - actions: [dismissUndo(id)], + actions: [dismissUndo({ undoId: id })], actionLabel: "Got it", canDismiss: false, }), diff --git a/frontend/src/metabase/reducers-common.ts b/frontend/src/metabase/reducers-common.ts index 768155cb08f3b15d9ac3fc608c79c371308cd6a2..6694e1c064af851820b47098e5b3d93d95eb46fc 100644 --- a/frontend/src/metabase/reducers-common.ts +++ b/frontend/src/metabase/reducers-common.ts @@ -12,7 +12,7 @@ import entities, { enhanceRequestsReducer } from "metabase/redux/entities"; import requests from "metabase/redux/requests"; import { settings } from "metabase/redux/settings"; import { modal } from "metabase/redux/ui"; -import undo from "metabase/redux/undo"; +import { undoReducer as undo } from "metabase/redux/undo"; import upload from "metabase/redux/uploads"; import { currentUser } from "metabase/redux/user"; diff --git a/frontend/src/metabase/redux/undo.js b/frontend/src/metabase/redux/undo.ts similarity index 68% rename from frontend/src/metabase/redux/undo.js rename to frontend/src/metabase/redux/undo.ts index a5422096b209deb5ce2110c80ed05c11922d6244..2b7c7f64a619c0302dbe39d5ec4c26b666130cf9 100644 --- a/frontend/src/metabase/redux/undo.js +++ b/frontend/src/metabase/redux/undo.ts @@ -1,8 +1,11 @@ import { createSelector } from "@reduxjs/toolkit"; +import type { Action } from "redux-actions"; import _ from "underscore"; import * as MetabaseAnalytics from "metabase/lib/analytics"; import { createAction, createThunkAction } from "metabase/lib/redux"; +import type { State } from "metabase-types/store"; +import type { Undo } from "metabase-types/store/undo"; const ADD_UNDO = "metabase/questions/ADD_UNDO"; const DISMISS_UNDO = "metabase/questions/DISMISS_UNDO"; @@ -17,11 +20,17 @@ export const addUndo = createThunkAction(ADD_UNDO, undo => { const id = undo.id ?? nextUndoId++; // if we're overwriting an existing undo, clear its timeout const currentUndo = getUndo(getState(), id); - clearTimeoutForUndo(currentUndo); + + if (currentUndo) { + clearTimeoutForUndo(currentUndo); + } let timeoutId = null; if (timeout) { - timeoutId = setTimeout(() => dispatch(dismissUndo(id, false)), timeout); + timeoutId = setTimeout( + () => dispatch(dismissUndo({ undoId: id, track: false })), + timeout, + ); } return { ...undo, @@ -36,8 +45,8 @@ export const addUndo = createThunkAction(ADD_UNDO, undo => { }); const PAUSE_UNDO = "metabase/questions/PAUSE_UNDO"; -export const pauseUndo = createAction(PAUSE_UNDO, undo => { - clearTimeout(undo.timeoutId); +export const pauseUndo = createAction(PAUSE_UNDO, (undo: Undo) => { + clearTimeoutForUndo(undo); return { ...undo, pausedAt: Date.now(), timeoutId: null }; }); @@ -50,7 +59,7 @@ export const resumeUndo = createThunkAction(RESUME_UNDO, undo => { return { ...undo, timeoutId: setTimeout( - () => dispatch(dismissUndo(undo.id, false)), + () => dispatch(dismissUndo({ undoId: undo.id, track: false })), restTime, ), timeout: restTime, @@ -58,19 +67,16 @@ export const resumeUndo = createThunkAction(RESUME_UNDO, undo => { }; }); -/** - * - * @param {import("metabase-types/store").State} state - * @param {*} undoId - * @returns - */ -function getUndo(state, undoId) { +function getUndo(state: State, undoId: Undo["id"]) { return _.findWhere(state.undo, { id: undoId }); } -const getAutoConnectedUndos = createSelector([state => state.undo], undos => { - return undos.filter(undo => undo.type === "filterAutoConnectDone"); -}); +const getAutoConnectedUndos = createSelector( + [(state: State) => state.undo], + undos => { + return undos.filter(undo => undo.type === "filterAutoConnectDone"); + }, +); export const getIsRecentlyAutoConnectedDashcard = createSelector( [ @@ -93,15 +99,13 @@ export const getIsRecentlyAutoConnectedDashcard = createSelector( }, ); -export const dismissUndo = createThunkAction( +export const dismissUndo = createAction( DISMISS_UNDO, - (undoId, track = true) => { - return () => { - if (track) { - MetabaseAnalytics.trackStructEvent("Undo", "Dismiss Undo"); - } - return undoId; - }; + ({ undoId, track = true }: { undoId: Undo["id"]; track?: boolean }) => { + if (track) { + MetabaseAnalytics.trackStructEvent("Undo", "Dismiss Undo"); + } + return undoId; }, ); @@ -110,18 +114,25 @@ export const dismissAllUndo = createAction(DISMISS_ALL_UNDO); export const performUndo = createThunkAction(PERFORM_UNDO, undoId => { return (dispatch, getState) => { const undo = getUndo(getState(), undoId); - if (!undo.actionLabel) { + if (!undo?.actionLabel) { MetabaseAnalytics.trackStructEvent("Undo", "Perform Undo"); } if (undo) { - undo.actions.map(action => dispatch(action)); - dispatch(dismissUndo(undoId, false)); + undo.actions?.forEach(action => dispatch(action)); + dispatch(dismissUndo({ undoId, track: false })); } }; }); -export default function (state = [], { type, payload, error }) { +export function undoReducer( + state: Undo[] = [], + { type, payload, error }: Action<Undo | Undo["id"]>, +) { if (type === ADD_UNDO) { + if (isUndoId(payload)) { + return state; + } + if (error) { console.warn("ADD_UNDO", payload); return state; @@ -137,7 +148,7 @@ export default function (state = [], { type, payload, error }) { count: payload.count || 1, }; - const previous = state[state.length - 1]; + const previous: Undo = state[state.length - 1]; // if last undo was same verb then merge them if (previous && undo.verb != null && undo.verb === previous.verb) { return state.slice(0, -1).concat({ @@ -146,11 +157,11 @@ export default function (state = [], { type, payload, error }) { // merge the verb, count, and subject appropriately verb: previous.verb, - count: previous.count + undo.count, + count: (previous.count ?? 0) + undo.count, subject: previous.subject === undo.subject ? undo.subject : "item", // merge items - actions: [...previous.actions, ...(payload.actions ?? [])], + actions: [...(previous.actions ?? []), ...(payload.actions ?? [])], _domId: previous._domId, // use original _domId so we don't get funky animations swapping for the new one }); @@ -158,20 +169,31 @@ export default function (state = [], { type, payload, error }) { return state.concat(undo); } } else if (type === DISMISS_UNDO) { - const dismissedUndo = getUndo({ undo: state }, payload); + if (!isUndoId(payload)) { + return state; + } + + const undoId = payload; + const dismissedUndo = state.find(undo => undo.id === undoId); - clearTimeoutForUndo(dismissedUndo); + if (dismissedUndo) { + clearTimeoutForUndo(dismissedUndo); + } if (error) { console.warn("DISMISS_UNDO", payload); return state; } - return state.filter(undo => undo.id !== payload); + return state.filter(undo => undo.id !== undoId); } else if (type === DISMISS_ALL_UNDO) { for (const undo of state) { clearTimeoutForUndo(undo); } return []; } else if (type === PAUSE_UNDO) { + if (isUndoId(payload)) { + return state; + } + return state.map(undo => { if (undo.id === payload.id) { return { @@ -184,6 +206,10 @@ export default function (state = [], { type, payload, error }) { return undo; }); } else if (type === RESUME_UNDO) { + if (isUndoId(payload)) { + return state; + } + return state.map(undo => { if (undo.id === payload.id) { return { @@ -202,8 +228,12 @@ export default function (state = [], { type, payload, error }) { return state; } -const clearTimeoutForUndo = undo => { - if (undo?.timeoutId) { +const clearTimeoutForUndo = (undo: Undo) => { + if (undo.timeoutId) { clearTimeout(undo.timeoutId); } }; + +function isUndoId(payload: unknown): payload is Undo["id"] { + return typeof payload !== "object"; +} diff --git a/frontend/src/metabase/redux/undo.unit.spec.ts b/frontend/src/metabase/redux/undo.unit.spec.ts index abfeb61fa4dbba14b079bdc546e4dca1cb13a2e4..297b3de3ce156f7fd2fc32d3769b3ab7e46af8c1 100644 --- a/frontend/src/metabase/redux/undo.unit.spec.ts +++ b/frontend/src/metabase/redux/undo.unit.spec.ts @@ -3,7 +3,8 @@ import { act } from "@testing-library/react"; import type { Dispatch } from "metabase-types/store"; -import undoReducer, { +import { + undoReducer, addUndo, dismissAllUndo, dismissUndo, @@ -60,7 +61,7 @@ describe("metabase/redux/undo", () => { await store.dispatch(addUndo({ id: MOCK_ID })); - await store.dispatch(dismissUndo(MOCK_ID)); + await store.dispatch(dismissUndo({ undoId: MOCK_ID })); expect(clearTimeoutSpy).toHaveBeenCalledTimes(1); }); @@ -96,7 +97,6 @@ describe("metabase/redux/undo", () => { }); // pause undo (e.g. when mouse is over toast) - // @ts-expect-error undo is still not converted to TS store.dispatch(pauseUndo(store.getState().undo[0])); await act(async () => { @@ -132,16 +132,24 @@ describe("metabase/redux/undo", () => { // await act is required to simulate store update on the next tick await act(async () => { - jest.advanceTimersByTime(timeout); + jest.advanceTimersByTime(timeout - 100); }); + // verify undo is there + expect(store.getState().undo.length).toBe(1); + + await act(async () => { + jest.advanceTimersByTime(100); + }); + + // verify undo is dismissed expect(store.getState().undo.length).toBe(0); }); }); const createMockStore = () => { const store = configureStore({ - // @ts-expect-error undo is still not converted to TS + // @ts-expect-error rework undo reducer to RTK reducer: { undo: undoReducer }, }); return store as typeof store & { dispatch: Dispatch };