From c60f6f870c85e66499af3c6af53646ea23f35d6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20=C5=BBuraw?= <9116238+krzysztofzuraw@users.noreply.github.com> Date: Tue, 3 Jan 2023 11:04:42 +0100 Subject: [PATCH] Remove `isDevelopment` flag and cleanup a code in Form component (#2912) --- .../Form/ExitFormDialogProvider.tsx | 224 +----------------- src/components/Form/Form.tsx | 2 +- src/components/Form/index.ts | 2 + src/components/Form/types.ts | 27 +++ src/components/Form/useBeforeUnload.ts | 4 +- .../Form/useExitFormDialog.test.tsx | 6 +- src/components/Form/useExitFormDialog.ts | 8 +- .../Form/useExitFormDialogProvider.tsx | 196 +++++++++++++++ src/hooks/useChannels.ts | 3 +- src/hooks/useForm.ts | 6 +- src/hooks/useHandleFormSubmit.ts | 3 +- src/misc.ts | 3 - .../ShippingZoneRatesCreatePage.tsx | 2 +- .../ShippingZoneRatesPage.tsx | 2 +- 14 files changed, 243 insertions(+), 245 deletions(-) create mode 100644 src/components/Form/types.ts create mode 100644 src/components/Form/useExitFormDialogProvider.tsx diff --git a/src/components/Form/ExitFormDialogProvider.tsx b/src/components/Form/ExitFormDialogProvider.tsx index e23b1e2fb..665062391 100644 --- a/src/components/Form/ExitFormDialogProvider.tsx +++ b/src/components/Form/ExitFormDialogProvider.tsx @@ -1,36 +1,9 @@ -import { SubmitPromise } from "@saleor/hooks/useForm"; -import { isInDevelopment } from "@saleor/misc"; -import React, { useEffect, useRef, useState } from "react"; -import { useHistory } from "react-router"; -import useRouter from "use-react-router"; +import React from "react"; import ExitFormDialog from "./ExitFormDialog"; +import { ExitFormDialogData } from "./types"; import useBeforeUnload from "./useBeforeUnload"; - -export interface ExitFormDialogData { - setIsDirty: (id: symbol, isDirty: boolean) => void; - setExitDialogSubmitRef: (id: symbol, submitFn: SubmitFn) => void; - setEnableExitDialog: (value: boolean) => void; - shouldBlockNavigation: () => boolean; - setIsSubmitting: (value: boolean) => void; - leave: () => void; - setIsSubmitDisabled: (value: boolean) => void; -} - -export type SubmitFn = (dataOrEvent?: any) => SubmitPromise; - -export type FormId = symbol; - -type FormsData = Record; - -export interface WithFormId { - formId: FormId; -} - -interface FormData { - isDirty: boolean; - submitFn: SubmitFn | null; -} +import { useExitFormDialogProvider } from "./useExitFormDialogProvider"; // Do not use this context directly in components // use useExitFormDialog hook instead @@ -44,195 +17,6 @@ export const ExitFormDialogContext = React.createContext({ setIsSubmitDisabled: () => undefined, }); -const defaultValues = { - isDirty: false, - showDialog: false, - blockNav: true, - navAction: null, - enableExitDialog: false, - isSubmitting: false, - formsData: {}, -}; - -export function useExitFormDialogProvider() { - const history = useHistory(); - const { history: routerHistory } = useRouter(); - - const [showDialog, setShowDialog] = useState(defaultValues.showDialog); - const isSubmitDisabled = useRef(false); - - const setIsSubmitDisabled = (status: boolean) => { - isSubmitDisabled.current = status; - }; - - const isSubmitting = useRef(defaultValues.isSubmitting); - const formsData = useRef({}); - const blockNav = useRef(defaultValues.blockNav); - const navAction = useRef(defaultValues.navAction); - const enableExitDialog = useRef(defaultValues.enableExitDialog); - const currentLocation = useRef(history.location); - - const setIsSubmitting = (value: boolean) => { - setEnableExitDialog(!value); - isSubmitting.current = value; - }; - - const setEnableExitDialog = (value: boolean) => { - // dialog should never be toggled to enabled during form submission - if (isSubmitting.current) { - return; - } - - enableExitDialog.current = value; - }; - - const setDefaultFormsData = () => { - formsData.current = defaultValues.formsData; - }; - - const setCurrentLocation = (newLocation: typeof history.location) => { - currentLocation.current = newLocation; - }; - - const setFormData = (id: symbol, newData: Partial) => { - const updatedFormData = { ...formsData.current[id], ...newData }; - - formsData.current = { - ...formsData.current, - [id]: updatedFormData, - }; - }; - - // Set either on generic form load or on every custom form data change - // but doesn't cause re-renders - const setSubmitRef = SubmitPromise>( - id: symbol, - submitFn: T, - ) => { - setFormData(id, { submitFn }); - }; - - const setIsDirty = (id: symbol, value: boolean) => { - // in case of race conitions between forms and transitions - if (!formsData.current[id]) { - return; - } - - setFormData(id, { isDirty: value }); - - if (value) { - setEnableExitDialog(true); - } - }; - - const setBlockNav = (value: boolean) => (blockNav.current = value); - - const setDefaultNavAction = () => - (navAction.current = defaultValues.navAction); - - const setStateDefaultValues = () => { - setIsSubmitting(defaultValues.isSubmitting); - setDefaultFormsData(); - setShowDialog(defaultValues.showDialog); - setBlockNav(defaultValues.blockNav); - setEnableExitDialog(defaultValues.enableExitDialog); - setDefaultNavAction(); - }; - - const getFormsDataValuesArray = () => - Object.getOwnPropertySymbols(formsData.current).map( - key => formsData.current[key], - ); - - const hasAnyFormsDirty = () => - getFormsDataValuesArray().some(({ isDirty }) => isDirty); - - const shouldBlockNav = () => { - if (!enableExitDialog.current || !hasAnyFormsDirty()) { - return false; - } - - return blockNav.current; - }; - - const isOnlyQuerying = (transition: typeof history.location) => - // We need to compare to current path and not window location - // so it works with browser back button as well - transition.pathname === currentLocation.current.pathname; - - const handleNavigationBlock = () => { - const unblock = history.block(transition => { - // needs to be done before the shouldBlockNav condition - // so it doesnt trigger setting default values - if (isOnlyQuerying(transition)) { - // ransition type requires this function to return either - // false | void | string where string opens up the browser prompt - // hence we return null - return null; - } - - if (shouldBlockNav()) { - navAction.current = transition; - setShowDialog(true); - return false; - } - - setStateDefaultValues(); - setCurrentLocation(transition); - return null; - }); - - return unblock; - }; - - useEffect(handleNavigationBlock, []); - - const continueNavigation = () => { - setBlockNav(false); - setDefaultFormsData(); - - setCurrentLocation(navAction.current); - // because our useNavigator navigate action may be blocked - // by exit dialog we want to avoid using it doing this transition - if (navAction.current !== null) { - routerHistory.push(navAction.current.pathname + navAction.current.search); - } - setStateDefaultValues(); - }; - - const handleLeave = () => { - continueNavigation(); - }; - - const handleClose = () => { - setDefaultNavAction(); - setShowDialog(false); - }; - - // Used to prevent race conditions from places such as - // create pages with navigation on mutation completed - const shouldBlockNavigation = () => !!navAction.current; - - const providerData: ExitFormDialogData = { - setIsDirty, - shouldBlockNavigation, - setEnableExitDialog, - setExitDialogSubmitRef: setSubmitRef, - setIsSubmitting, - setIsSubmitDisabled, - leave: handleLeave, - }; - - return { - providerData, - showDialog, - handleLeave, - handleClose, - shouldBlockNav, - isSubmitDisabled, - }; -} - const ExitFormDialogProvider = ({ children }) => { const { handleClose, @@ -245,7 +29,7 @@ const ExitFormDialogProvider = ({ children }) => { useBeforeUnload(e => { // If form is dirty and user does a refresh, // the browser will ask about unsaved changes - if (shouldBlockNav() && !isInDevelopment) { + if (shouldBlockNav()) { e.preventDefault(); e.returnValue = ""; } diff --git a/src/components/Form/Form.tsx b/src/components/Form/Form.tsx index 8abaed32b..3bc0acabf 100644 --- a/src/components/Form/Form.tsx +++ b/src/components/Form/Form.tsx @@ -1,7 +1,7 @@ import useForm, { SubmitPromise, UseFormResult } from "@saleor/hooks/useForm"; import React from "react"; -import { FormId } from "./ExitFormDialogProvider"; +import { FormId } from "./types"; export type CheckIfSaveIsDisabledFnType = (data: T) => boolean; diff --git a/src/components/Form/index.ts b/src/components/Form/index.ts index a5c6fdf72..e6fa56515 100644 --- a/src/components/Form/index.ts +++ b/src/components/Form/index.ts @@ -1,2 +1,4 @@ export * from "./Form"; export { default } from "./Form"; +export * from "./types"; +export * from "./useExitFormDialog"; diff --git a/src/components/Form/types.ts b/src/components/Form/types.ts new file mode 100644 index 000000000..2efa6fb17 --- /dev/null +++ b/src/components/Form/types.ts @@ -0,0 +1,27 @@ +import { SubmitPromise } from "@saleor/hooks/useForm"; +import React from "react"; + +export type SubmitFn = (event?: React.FormEvent) => SubmitPromise; + +export type FormId = symbol; + +export type FormsData = Record; + +export interface WithFormId { + formId: FormId; +} + +export interface ExitFormDialogData { + setIsDirty: (id: symbol, isDirty: boolean) => void; + setExitDialogSubmitRef: (id: symbol, submitFn: SubmitFn) => void; + setEnableExitDialog: (value: boolean) => void; + shouldBlockNavigation: () => boolean; + setIsSubmitting: (value: boolean) => void; + leave: () => void; + setIsSubmitDisabled: (value: boolean) => void; +} + +export interface FormData { + isDirty: boolean; + submitFn: SubmitFn | null; +} diff --git a/src/components/Form/useBeforeUnload.ts b/src/components/Form/useBeforeUnload.ts index c3a8d6fdf..cf54e8f0d 100644 --- a/src/components/Form/useBeforeUnload.ts +++ b/src/components/Form/useBeforeUnload.ts @@ -1,6 +1,6 @@ import { useEffect, useRef } from "react"; -const useBeforeUnload = fn => { +const useBeforeUnload = (fn: (event: BeforeUnloadEvent) => void) => { const cb = useRef(fn); useEffect(() => { @@ -8,7 +8,7 @@ const useBeforeUnload = fn => { }, [fn]); useEffect(() => { - const onBeforeUnload = (...args) => cb.current?.(...args); + const onBeforeUnload = (event: BeforeUnloadEvent) => cb.current?.(event); window.addEventListener("beforeunload", onBeforeUnload); diff --git a/src/components/Form/useExitFormDialog.test.tsx b/src/components/Form/useExitFormDialog.test.tsx index fd147238a..9af3486d3 100644 --- a/src/components/Form/useExitFormDialog.test.tsx +++ b/src/components/Form/useExitFormDialog.test.tsx @@ -4,11 +4,9 @@ import React from "react"; import { useHistory } from "react-router"; import { MemoryRouter } from "react-router-dom"; -import { - ExitFormDialogContext, - useExitFormDialogProvider, -} from "./ExitFormDialogProvider"; +import { ExitFormDialogContext } from "./ExitFormDialogProvider"; import { useExitFormDialog } from "./useExitFormDialog"; +import { useExitFormDialogProvider } from "./useExitFormDialogProvider"; jest.mock("../../hooks/useNotifier", () => undefined); diff --git a/src/components/Form/useExitFormDialog.ts b/src/components/Form/useExitFormDialog.ts index 7478642fc..9fac51fb5 100644 --- a/src/components/Form/useExitFormDialog.ts +++ b/src/components/Form/useExitFormDialog.ts @@ -1,11 +1,7 @@ import React, { useContext, useRef } from "react"; -import { - ExitFormDialogContext, - ExitFormDialogData, - SubmitFn, - WithFormId, -} from "./ExitFormDialogProvider"; +import { ExitFormDialogContext } from "./ExitFormDialogProvider"; +import { ExitFormDialogData, SubmitFn, WithFormId } from "./types"; export interface UseExitFormDialogResult extends Omit, diff --git a/src/components/Form/useExitFormDialogProvider.tsx b/src/components/Form/useExitFormDialogProvider.tsx new file mode 100644 index 000000000..571231b2e --- /dev/null +++ b/src/components/Form/useExitFormDialogProvider.tsx @@ -0,0 +1,196 @@ +import { SubmitPromise } from "@saleor/hooks/useForm"; +import { useEffect, useRef, useState } from "react"; +import { useHistory } from "react-router"; +import useRouter from "use-react-router"; + +import { ExitFormDialogData, FormData, FormsData } from "./types"; + +const defaultValues = { + isDirty: false, + showDialog: false, + blockNav: true, + navAction: null, + enableExitDialog: false, + isSubmitting: false, + formsData: {}, +}; + +export function useExitFormDialogProvider() { + const history = useHistory(); + const { history: routerHistory } = useRouter(); + + const [showDialog, setShowDialog] = useState(defaultValues.showDialog); + const isSubmitDisabled = useRef(false); + + const setIsSubmitDisabled = (status: boolean) => { + isSubmitDisabled.current = status; + }; + + const isSubmitting = useRef(defaultValues.isSubmitting); + const formsData = useRef({}); + const blockNav = useRef(defaultValues.blockNav); + const navAction = useRef(defaultValues.navAction); + const enableExitDialog = useRef(defaultValues.enableExitDialog); + const currentLocation = useRef(history.location); + + const setIsSubmitting = (value: boolean) => { + setEnableExitDialog(!value); + isSubmitting.current = value; + }; + + const setEnableExitDialog = (value: boolean) => { + // dialog should never be toggled to enabled during form submission + if (isSubmitting.current) { + return; + } + + enableExitDialog.current = value; + }; + + const setDefaultFormsData = () => { + formsData.current = defaultValues.formsData; + }; + + const setCurrentLocation = (newLocation: typeof history.location) => { + currentLocation.current = newLocation; + }; + + const setFormData = (id: symbol, newData: Partial) => { + const updatedFormData = { ...formsData.current[id], ...newData }; + + formsData.current = { + ...formsData.current, + [id]: updatedFormData, + }; + }; + + // Set either on generic form load or on every custom form data change + // but doesn't cause re-renders + const setSubmitRef = SubmitPromise>( + id: symbol, + submitFn: T, + ) => { + setFormData(id, { submitFn }); + }; + + const setIsDirty = (id: symbol, value: boolean) => { + // in case of race conitions between forms and transitions + if (!formsData.current[id]) { + return; + } + + setFormData(id, { isDirty: value }); + + if (value) { + setEnableExitDialog(true); + } + }; + + const setBlockNav = (value: boolean) => (blockNav.current = value); + + const setDefaultNavAction = () => + (navAction.current = defaultValues.navAction); + + const setStateDefaultValues = () => { + setIsSubmitting(defaultValues.isSubmitting); + setDefaultFormsData(); + setShowDialog(defaultValues.showDialog); + setBlockNav(defaultValues.blockNav); + setEnableExitDialog(defaultValues.enableExitDialog); + setDefaultNavAction(); + }; + + const getFormsDataValuesArray = () => + Object.getOwnPropertySymbols(formsData.current).map( + key => formsData.current[key], + ); + + const hasAnyFormsDirty = () => + getFormsDataValuesArray().some(({ isDirty }) => isDirty); + + const shouldBlockNav = () => { + if (!enableExitDialog.current || !hasAnyFormsDirty()) { + return false; + } + + return blockNav.current; + }; + + const isOnlyQuerying = (transition: typeof history.location) => + // We need to compare to current path and not window location + // so it works with browser back button as well + transition.pathname === currentLocation.current.pathname; + + const handleNavigationBlock = () => { + // This callback blocks only navigation between internal dashboard pages + // https://github.com/remix-run/history/blob/main/docs/blocking-transitions.md#caveats + const unblock = history.block(transition => { + // needs to be done before the shouldBlockNav condition + // so it doesn't trigger setting default values + if (isOnlyQuerying(transition)) { + // transition type requires this function to return either + // false | void | string where string opens up the browser prompt + // hence we return null + return null; + } + if (shouldBlockNav()) { + navAction.current = transition; + setShowDialog(true); + return false; + } + + setStateDefaultValues(); + setCurrentLocation(transition); + return null; + }); + + return unblock; + }; + + useEffect(handleNavigationBlock, []); + + const continueNavigation = () => { + setBlockNav(false); + setDefaultFormsData(); + + setCurrentLocation(navAction.current); + // because our useNavigator navigate action may be blocked + // by exit dialog we want to avoid using it doing this transition + if (navAction.current !== null) { + routerHistory.push(navAction.current.pathname + navAction.current.search); + } + setStateDefaultValues(); + }; + + const handleLeave = () => { + continueNavigation(); + }; + + const handleClose = () => { + setDefaultNavAction(); + setShowDialog(false); + }; + + // Used to prevent race conditions from places such as + // create pages with navigation on mutation completed + const shouldBlockNavigation = () => !!navAction.current; + + const providerData: ExitFormDialogData = { + setIsDirty, + shouldBlockNavigation, + setEnableExitDialog, + setExitDialogSubmitRef: setSubmitRef, + setIsSubmitting, + setIsSubmitDisabled, + leave: handleLeave, + }; + + return { + providerData, + showDialog, + handleLeave, + handleClose, + shouldBlockNav, + isSubmitDisabled, + }; +} diff --git a/src/hooks/useChannels.ts b/src/hooks/useChannels.ts index 4adffa8ef..7366e8b08 100644 --- a/src/hooks/useChannels.ts +++ b/src/hooks/useChannels.ts @@ -1,7 +1,6 @@ import { ChannelsAction } from "@saleor/channels/urls"; import { Channel } from "@saleor/channels/utils"; -import { WithFormId } from "@saleor/components/Form/ExitFormDialogProvider"; -import { useExitFormDialog } from "@saleor/components/Form/useExitFormDialog"; +import { useExitFormDialog, WithFormId } from "@saleor/components/Form"; import useListActions from "@saleor/hooks/useListActions"; import useStateFromProps from "@saleor/hooks/useStateFromProps"; diff --git a/src/hooks/useForm.ts b/src/hooks/useForm.ts index 32d55bb31..e37b0de49 100644 --- a/src/hooks/useForm.ts +++ b/src/hooks/useForm.ts @@ -1,9 +1,9 @@ -import { CheckIfSaveIsDisabledFnType } from "@saleor/components/Form"; -import { FormId } from "@saleor/components/Form/ExitFormDialogProvider"; import { + CheckIfSaveIsDisabledFnType, + FormId, useExitFormDialog, UseExitFormDialogResult, -} from "@saleor/components/Form/useExitFormDialog"; +} from "@saleor/components/Form"; import useHandleFormSubmit from "@saleor/hooks/useHandleFormSubmit"; import { toggle } from "@saleor/utils/lists"; import isEqual from "lodash/isEqual"; diff --git a/src/hooks/useHandleFormSubmit.ts b/src/hooks/useHandleFormSubmit.ts index 6db20ab78..c75349b0b 100644 --- a/src/hooks/useHandleFormSubmit.ts +++ b/src/hooks/useHandleFormSubmit.ts @@ -1,5 +1,4 @@ -import { FormId } from "@saleor/components/Form/ExitFormDialogProvider"; -import { useExitFormDialog } from "@saleor/components/Form/useExitFormDialog"; +import { FormId, useExitFormDialog } from "@saleor/components/Form"; import { MessageContext } from "@saleor/components/messages"; import { SubmitPromise } from "@saleor/hooks/useForm"; import { useContext } from "react"; diff --git a/src/misc.ts b/src/misc.ts index 7e4ae0713..5903d5315 100644 --- a/src/misc.ts +++ b/src/misc.ts @@ -536,9 +536,6 @@ export const combinedMultiAutocompleteChoices = ( choices: MultiAutocompleteChoiceType[], ) => uniqBy([...selected, ...choices], "value"); -export const isInDevelopment = - !process.env.NODE_ENV || process.env.NODE_ENV === "development"; - export type WithOptional = Omit & Partial>; diff --git a/src/shipping/components/ShippingZoneRatesCreatePage/ShippingZoneRatesCreatePage.tsx b/src/shipping/components/ShippingZoneRatesCreatePage/ShippingZoneRatesCreatePage.tsx index e4dcd9865..5a9f5e305 100644 --- a/src/shipping/components/ShippingZoneRatesCreatePage/ShippingZoneRatesCreatePage.tsx +++ b/src/shipping/components/ShippingZoneRatesCreatePage/ShippingZoneRatesCreatePage.tsx @@ -3,7 +3,7 @@ import { Backlink } from "@saleor/components/Backlink"; import CardSpacer from "@saleor/components/CardSpacer"; import ChannelsAvailabilityCard from "@saleor/components/ChannelsAvailabilityCard"; import Container from "@saleor/components/Container"; -import { WithFormId } from "@saleor/components/Form/ExitFormDialogProvider"; +import { WithFormId } from "@saleor/components/Form"; import Grid from "@saleor/components/Grid"; import PageHeader from "@saleor/components/PageHeader"; import Savebar from "@saleor/components/Savebar"; diff --git a/src/shipping/components/ShippingZoneRatesPage/ShippingZoneRatesPage.tsx b/src/shipping/components/ShippingZoneRatesPage/ShippingZoneRatesPage.tsx index daa81943e..339d76d11 100644 --- a/src/shipping/components/ShippingZoneRatesPage/ShippingZoneRatesPage.tsx +++ b/src/shipping/components/ShippingZoneRatesPage/ShippingZoneRatesPage.tsx @@ -3,7 +3,7 @@ import { Backlink } from "@saleor/components/Backlink"; import CardSpacer from "@saleor/components/CardSpacer"; import ChannelsAvailabilityCard from "@saleor/components/ChannelsAvailabilityCard"; import Container from "@saleor/components/Container"; -import { WithFormId } from "@saleor/components/Form/ExitFormDialogProvider"; +import { WithFormId } from "@saleor/components/Form"; import Grid from "@saleor/components/Grid"; import Metadata from "@saleor/components/Metadata/Metadata"; import PageHeader from "@saleor/components/PageHeader";