diff --git a/.changeset/beige-ghosts-invent.md b/.changeset/beige-ghosts-invent.md new file mode 100644 index 0000000000..86a83bbf93 --- /dev/null +++ b/.changeset/beige-ghosts-invent.md @@ -0,0 +1,15 @@ +--- +"@comet/cms-admin": minor +--- + +Improve the `SaveConflictDialog` + +- extend the text in the dialog to explain + - what happened + - what the next steps are + - what can be done to avoid conflicts +- make the button labels more precise +- once the save dialog is closed + - stop polling + - mark the save button red and with an error icon + diff --git a/.changeset/big-socks-whisper.md b/.changeset/big-socks-whisper.md new file mode 100644 index 0000000000..ecfa43c891 --- /dev/null +++ b/.changeset/big-socks-whisper.md @@ -0,0 +1,8 @@ +--- +"@comet/cms-admin": minor +--- + +`useSaveConflict()`, `useSaveConflictQuery()` and `useFormSaveConflict()` now return a `hasConflict` prop + +If `hasConflict` is true, a save conflict has been detected. +You should pass `hasConflict` on to `SaveButton`, `FinalFormSaveButton` or `FinalFormSaveSplitButton`. The button will then display a "conflict" state. diff --git a/.changeset/fuzzy-doors-tie.md b/.changeset/fuzzy-doors-tie.md new file mode 100644 index 0000000000..a158f35946 --- /dev/null +++ b/.changeset/fuzzy-doors-tie.md @@ -0,0 +1,5 @@ +--- +"@comet/cms-admin": minor +--- + +Admin Generator: In the generated form, the `hasConflict` prop is passed from the `useFormSaveConflict()` hook to the `FinalFormSaveSplitButton` diff --git a/.changeset/little-hounds-promise.md b/.changeset/little-hounds-promise.md new file mode 100644 index 0000000000..57504a6898 --- /dev/null +++ b/.changeset/little-hounds-promise.md @@ -0,0 +1,8 @@ +--- +"@comet/admin": minor +--- + +Add optional `hasConflict` prop to `SaveButton`, `FinalFormSaveButton` and `FinalFormSaveSplitButton` + +If set to `true`, a new "conflict" display state is triggered. +You should pass the `hasConflict` prop returned by `useSaveConflict()`, `useSaveConflictQuery()` and `useFormSaveConflict()`. diff --git a/demo/admin/src/news/generated/NewsForm.tsx b/demo/admin/src/news/generated/NewsForm.tsx index 6279b4ddf9..7139e4d4e5 100644 --- a/demo/admin/src/news/generated/NewsForm.tsx +++ b/demo/admin/src/news/generated/NewsForm.tsx @@ -164,7 +164,7 @@ export function NewsForm({ id }: FormProps): React.ReactElement { - + diff --git a/demo/admin/src/products/ProductForm.tsx b/demo/admin/src/products/ProductForm.tsx index 6f94e2b1f6..b2db47e7a3 100644 --- a/demo/admin/src/products/ProductForm.tsx +++ b/demo/admin/src/products/ProductForm.tsx @@ -186,7 +186,7 @@ function ProductForm({ id }: FormProps): React.ReactElement { - + diff --git a/demo/admin/src/products/categories/ProductCategoryForm.tsx b/demo/admin/src/products/categories/ProductCategoryForm.tsx index 2913b33904..e93d9bf893 100644 --- a/demo/admin/src/products/categories/ProductCategoryForm.tsx +++ b/demo/admin/src/products/categories/ProductCategoryForm.tsx @@ -157,7 +157,7 @@ function ProductCategoryForm({ id }: FormProps): React.ReactElement { - + diff --git a/demo/admin/src/products/generated/ProductForm.tsx b/demo/admin/src/products/generated/ProductForm.tsx index 8dccbd0ce3..7c4b9cd8f6 100644 --- a/demo/admin/src/products/generated/ProductForm.tsx +++ b/demo/admin/src/products/generated/ProductForm.tsx @@ -158,7 +158,7 @@ export function ProductForm({ id }: FormProps): React.ReactElement { - + diff --git a/demo/admin/src/products/tags/ProductTagForm.tsx b/demo/admin/src/products/tags/ProductTagForm.tsx index 21704a7e61..42d4243c25 100644 --- a/demo/admin/src/products/tags/ProductTagForm.tsx +++ b/demo/admin/src/products/tags/ProductTagForm.tsx @@ -154,7 +154,7 @@ function ProductTagForm({ id }: FormProps): React.ReactElement { - + diff --git a/packages/admin/admin/src/FinalFormSaveButton.tsx b/packages/admin/admin/src/FinalFormSaveButton.tsx index 1e98cd48fa..d6ee3dc3ea 100644 --- a/packages/admin/admin/src/FinalFormSaveButton.tsx +++ b/packages/admin/admin/src/FinalFormSaveButton.tsx @@ -7,9 +7,10 @@ import { messages } from "./messages"; interface FinalFormSaveButtonProps { message?: React.ReactNode; + hasConflict?: boolean; } -export const FinalFormSaveButton = ({ message = }: FinalFormSaveButtonProps) => { +export const FinalFormSaveButton = ({ message = , hasConflict }: FinalFormSaveButtonProps) => { const form = useForm(); const { pristine, hasValidationErrors, submitting, hasSubmitErrors } = useFormState(); @@ -17,6 +18,7 @@ export const FinalFormSaveButton = ({ message = ) => { +export const FinalFormSaveSplitButton = ({ localStorageKey = "SaveSplitButton", hasConflict = false }: PropsWithChildren) => { const stackApi = useStackApi(); const form = useForm(); const { pristine, hasValidationErrors, submitting, hasSubmitErrors } = useFormState(); @@ -25,13 +27,20 @@ export const FinalFormSaveSplitButton = ({ localStorageKey = "SaveSplitButton" } console.warn(`Can't set submitEvent, as the setSubmitEvent mutator is missing. Did you forget to add the mutator to the form?`); }; + const splitButtonProps: Partial = {}; + if (hasConflict) { + splitButtonProps.selectIcon = theme.palette.error.contrastText }} />; + } + return ( - + { const event = new FinalFormSubmitEvent("submit"); event.navigatingBack = false; diff --git a/packages/admin/admin/src/common/buttons/save/SaveButton.styles.ts b/packages/admin/admin/src/common/buttons/save/SaveButton.styles.ts index caa1aa7281..035909a8b0 100644 --- a/packages/admin/admin/src/common/buttons/save/SaveButton.styles.ts +++ b/packages/admin/admin/src/common/buttons/save/SaveButton.styles.ts @@ -1,10 +1,10 @@ -import { ButtonClassKey } from "@mui/material"; +import { ButtonClassKey, buttonGroupClasses } from "@mui/material"; import { Theme } from "@mui/material/styles"; import { createStyles } from "@mui/styles"; import { SaveButtonProps } from "./SaveButton"; -export type SaveButtonClassKey = "saving" | "error" | "success" | ButtonClassKey; +export type SaveButtonClassKey = "saving" | "error" | "success" | "conflict" | ButtonClassKey; export const styles = (theme: Theme) => { return createStyles({ @@ -65,6 +65,16 @@ export const styles = (theme: Theme) => { backgroundColor: theme.palette.success.light, }, }, + conflict: { + color: theme.palette.error.contrastText, + backgroundColor: theme.palette.error.main, + "&:hover": { + backgroundColor: theme.palette.error.dark, + }, + [`&.${buttonGroupClasses.grouped}:not(:last-child)`]: { + borderRightColor: theme.palette.error.dark, + }, + }, textError: {}, textInfo: {}, textSuccess: {}, diff --git a/packages/admin/admin/src/common/buttons/save/SaveButton.tsx b/packages/admin/admin/src/common/buttons/save/SaveButton.tsx index b807b48741..9c237dae61 100644 --- a/packages/admin/admin/src/common/buttons/save/SaveButton.tsx +++ b/packages/admin/admin/src/common/buttons/save/SaveButton.tsx @@ -1,4 +1,4 @@ -import { Check, Error, Save, ThreeDotSaving } from "@comet/admin-icons"; +import { Check, Error, Error as ErrorIcon, Save, ThreeDotSaving } from "@comet/admin-icons"; import { Button, ButtonClassKey, ButtonProps, ComponentsOverrides, Theme } from "@mui/material"; import { WithStyles, withStyles } from "@mui/styles"; import { ClassKeyOfStyles } from "@mui/styles/withStyles"; @@ -13,28 +13,34 @@ import { SaveButtonClassKey, styles } from "./SaveButton.styles"; export interface SaveButtonProps extends ButtonProps { saving?: boolean; hasErrors?: boolean; + hasConflict?: boolean; savingItem?: React.ReactNode; successItem?: React.ReactNode; errorItem?: React.ReactNode; + conflictItem?: React.ReactNode; saveIcon?: React.ReactNode; savingIcon?: React.ReactNode; successIcon?: React.ReactNode; errorIcon?: React.ReactNode; + conflictIcon?: React.ReactNode; } -export type SaveButtonDisplayState = "idle" | "saving" | "success" | "error"; +export type SaveButtonDisplayState = "idle" | "saving" | "success" | "error" | "conflict"; const SaveBtn = ({ saving = false, hasErrors = false, + hasConflict = false, children = , savingItem = , successItem = , errorItem = , + conflictItem = , saveIcon = , savingIcon = , successIcon = , errorIcon = , + conflictIcon = , variant = "contained", color = "primary", classes, @@ -51,6 +57,8 @@ const SaveBtn = ({ return successIcon; } else if (displayState === "error") { return errorIcon; + } else if (displayState === "conflict") { + return conflictIcon; } return saveIcon; }; @@ -58,9 +66,13 @@ const SaveBtn = ({ React.useEffect(() => { let timeoutId: number | undefined; - if (displayState === "idle" && saving) { + if ((displayState === "idle" || displayState === "conflict") && saving) { setDisplayState("saving"); } + // Display Conflict + else if (displayState === "idle" && hasConflict) { + setDisplayState("conflict"); + } // Display Error else if (displayState === "saving" && hasErrors === true) { timeoutId = window.setTimeout(() => { @@ -91,7 +103,7 @@ const SaveBtn = ({ window.clearTimeout(timeoutId); } }; - }, [displayState, saving, hasErrors]); + }, [displayState, saving, hasErrors, hasConflict]); React.useEffect(() => { if (displayState === "idle") { @@ -102,6 +114,8 @@ const SaveBtn = ({ saveSplitButton?.setShowSelectButton(false); } else if (displayState === "error") { saveSplitButton?.setShowSelectButton(false); + } else if (displayState === "conflict") { + saveSplitButton?.setShowSelectButton(undefined); } }, [displayState, saveSplitButton]); @@ -112,12 +126,13 @@ const SaveBtn = ({ startIcon={resolveIconForDisplayState(displayState)} variant={variant} color={color} - disabled={disabled || displayState != "idle"} + disabled={disabled || (displayState != "idle" && displayState != "conflict")} > {displayState === "idle" && children} {displayState === "saving" && savingItem} {displayState === "success" && successItem} {displayState === "error" && errorItem} + {displayState === "conflict" && conflictItem} ); }; @@ -134,6 +149,8 @@ const resolveClassForDisplayState = ( buttonClasses.root += ` ${classes.saving}`; } else if (displayState === "error") { buttonClasses.root += ` ${classes.error}`; + } else if (displayState === "conflict") { + buttonClasses.root += ` ${classes.conflict}`; } return buttonClasses; diff --git a/packages/admin/cms-admin/src/generator/generateForm.ts b/packages/admin/cms-admin/src/generator/generateForm.ts index fea0bbc0ca..60e0ab2846 100644 --- a/packages/admin/cms-admin/src/generator/generateForm.ts +++ b/packages/admin/cms-admin/src/generator/generateForm.ts @@ -293,7 +293,7 @@ export async function writeCrudForm(generatorConfig: CrudGeneratorConfig, schema - + diff --git a/packages/admin/cms-admin/src/pages/SaveConflictDialog.tsx b/packages/admin/cms-admin/src/pages/SaveConflictDialog.tsx index 06a83d8137..5f42f4014b 100644 --- a/packages/admin/cms-admin/src/pages/SaveConflictDialog.tsx +++ b/packages/admin/cms-admin/src/pages/SaveConflictDialog.tsx @@ -1,7 +1,8 @@ import { messages } from "@comet/admin"; import { Clear, Delete, OpenNewTab, Warning } from "@comet/admin-icons"; import { fontWeights } from "@comet/admin-theme"; -import { Alert, Button, Dialog, DialogActions, DialogContent, DialogTitle, Typography } from "@mui/material"; +import { Alert, Box, Button, Dialog, DialogActions, DialogContent, DialogTitle, List, ListItem, Stack, Typography } from "@mui/material"; +import { styled } from "@mui/material/styles"; import makeStyles from "@mui/styles/makeStyles"; import * as React from "react"; import { FormattedMessage } from "react-intl"; @@ -39,7 +40,7 @@ function SaveConflictDialog({ open, onClosePressed, onDiscardChangesPressed }: S const styles = useStyles(); return ( - + @@ -49,16 +50,82 @@ function SaveConflictDialog({ open, onClosePressed, onDiscardChangesPressed }: S } classes={{ root: styles.errorAlert }}> - - - + + + + + + + + {chunks} }} + /> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
@@ -92,4 +159,14 @@ function SaveConflictDialog({ open, onClosePressed, onDiscardChangesPressed }: S ); } +const StyledList = styled(List)` + list-style-type: disc; + padding-inline-start: ${({ theme }) => theme.spacing(6)}; +`; + +const StyledListItem = styled(ListItem)` + display: list-item; + padding-left: 0; +`; + export { SaveConflictDialog }; diff --git a/packages/admin/cms-admin/src/pages/createUsePage.tsx b/packages/admin/cms-admin/src/pages/createUsePage.tsx index b131ab3f8e..a343ea6d91 100644 --- a/packages/admin/cms-admin/src/pages/createUsePage.tsx +++ b/packages/admin/cms-admin/src/pages/createUsePage.tsx @@ -1,5 +1,6 @@ import { ApolloError, gql, TypedDocumentNode, useApolloClient, useQuery } from "@apollo/client"; -import { messages, SaveButton, SaveButtonProps, SplitButton, useStackApi } from "@comet/admin"; +import { messages, SaveButton, SaveButtonProps, SplitButton, SplitButtonProps, useStackApi } from "@comet/admin"; +import { ChevronDown } from "@comet/admin-icons"; import { BindBlockAdminComponent, BlockInterface, @@ -208,10 +209,11 @@ export const createUsePage: CreateUsePage = await refetch(); }; - const { dialogs, checkForConflicts: checkForSaveConflict } = useSaveConflictQuery< - GQLCheckForChangesQuery, - GQLCheckForChangesQueryVariables - >( + const { + dialogs, + checkForConflicts: checkForSaveConflict, + hasConflict, + } = useSaveConflictQuery( checkForChangesQuery, { variables: { @@ -398,8 +400,16 @@ export const createUsePage: CreateUsePage = }, [pageState, createHandleUpdate, pageId]); const pageSaveButton = React.useMemo( - () => , - [hasChanges, handleSavePage, saveError, saving], + () => ( + + ), + [hasConflict, hasChanges, handleSavePage, saveError, saving], ); return { @@ -433,10 +443,11 @@ const checkForChangesQuery = gql` interface PageSaveButtonProps { handleSavePage: () => Promise; hasChanges?: boolean; + hasConflict: boolean; saving: boolean; saveError: "invalid" | "conflict" | "error" | undefined; } -function PageSaveButton({ handleSavePage, hasChanges, saving, saveError }: PageSaveButtonProps): JSX.Element { +function PageSaveButton({ handleSavePage, hasChanges, hasConflict, saving, saveError }: PageSaveButtonProps): JSX.Element { const stackApi = useStackApi(); const saveButtonProps: Omit = { @@ -444,6 +455,7 @@ function PageSaveButton({ handleSavePage, hasChanges, saving, saveError }: PageS variant: "contained", saving, hasErrors: !!saveError, + hasConflict, errorItem: saveError == "invalid" ? ( @@ -452,8 +464,15 @@ function PageSaveButton({ handleSavePage, hasChanges, saving, saveError }: PageS ) : undefined, }; + const splitButtonProps: Partial = {}; + if (hasConflict) { + // setting the color to "error" is only necessary for the SplitButton and doesn't affect the SaveButton + saveButtonProps.color = "error"; + splitButtonProps.selectIcon = theme.palette.error.contrastText }} />; + } + return ( - + diff --git a/packages/admin/cms-admin/src/pages/useSaveConflict.tsx b/packages/admin/cms-admin/src/pages/useSaveConflict.tsx index eca42e8d3e..84013675f2 100644 --- a/packages/admin/cms-admin/src/pages/useSaveConflict.tsx +++ b/packages/admin/cms-admin/src/pages/useSaveConflict.tsx @@ -15,55 +15,64 @@ export interface SaveConflictOptions { export interface SaveConflictHookReturn { dialogs: React.ReactNode; checkForConflicts: () => Promise; + hasConflict: boolean; } export function useSaveConflict(options: SaveConflictOptions): SaveConflictHookReturn { const { checkConflict, hasChanges, loadLatestVersion, onDiscardButtonPressed } = options; const snackbarApi = useSnackbarApi(); + const pollingIntervalId = React.useRef(); const [showDialog, setShowDialog] = React.useState(false); + const [hasConflict, setHasConflict] = React.useState(false); - React.useEffect(() => { - const checkChanges = async () => { - const newHasConflict = await checkConflict(); - //we don't need setHasConflict as that is used during save only - if (newHasConflict) { - if (!hasChanges()) { - // No local changes, server changes available - await loadLatestVersion(); - snackbarApi.showSnackbar( - - - - - , - ); - } else { - // local changes, and server changes available: ask user what to do + const checkChanges = React.useCallback(async () => { + const newHasConflict = await checkConflict(); + //we don't need setHasConflict as that is used during save only + if (newHasConflict) { + if (!hasChanges()) { + // No local changes, server changes available + await loadLatestVersion(); + snackbarApi.showSnackbar( + + + + + , + ); + } else { + // local changes, and server changes available: ask user what to do + if (!hasConflict) { setShowDialog(true); } - } else { - setShowDialog(false); + setHasConflict(true); } - }; - - let intervalId: number | undefined; + } else { + setShowDialog(false); + setHasConflict(false); + } + }, [checkConflict, hasChanges, hasConflict, loadLatestVersion, snackbarApi]); - const startPolling = () => { - window.clearInterval(intervalId); - intervalId = window.setInterval(checkChanges, 10000); - }; + const startPolling = React.useCallback(() => { + if (!hasConflict) { + window.clearInterval(pollingIntervalId.current); + pollingIntervalId.current = window.setInterval(checkChanges, 10000); + } + }, [checkChanges, hasConflict]); - const stopPolling = () => { - window.clearInterval(intervalId); - intervalId = undefined; - }; + const stopPolling = React.useCallback(() => { + window.clearInterval(pollingIntervalId.current); + pollingIntervalId.current = undefined; + }, []); + React.useEffect(() => { const handleFocus = () => { - checkChanges(); + if (!hasConflict) { + checkChanges(); + } startPolling(); }; @@ -84,25 +93,30 @@ export function useSaveConflict(options: SaveConflictOptions): SaveConflictHookR stopPolling(); }; - }, [checkConflict, snackbarApi, loadLatestVersion, hasChanges]); + }, [checkConflict, snackbarApi, loadLatestVersion, hasChanges, checkChanges, startPolling, stopPolling, hasConflict]); const checkForConflicts = React.useCallback(async () => { const newHasConflict = await checkConflict(); if (newHasConflict) { setShowDialog(true); + setHasConflict(true); } return newHasConflict; }, [checkConflict]); + return { checkForConflicts, + hasConflict, dialogs: ( <> { + stopPolling(); setShowDialog(false); }} onDiscardChangesPressed={() => { + setHasConflict(false); setShowDialog(false); onDiscardButtonPressed(); }}