COM-134: Improve Save Conflict Dialog (vivid-planet#1366)

## Changes: - extend the text in the save conflicts 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 (I will also show the other user (who made the conflicting change) in the dialog. I'll do that in a follow-up PR) ## New dialog: <img width="1075" alt="Bildschirmfoto 2023-11-06 um 09 32 27" src="https://github.com/vivid-planet/comet/assets/13380047/49ee1ea5-e4f5-4ed5-ad91-9c695e5ea975"> ## Screencast: https://github.com/vivid-planet/comet/assets/13380047/51c18896-7e02-4373-a7e1-1fae10faebfc --------- Co-authored-by: Ricky James Smith <jamesricky@me.com>
VP-DS · Jan 12, 2024 · 209688d · 209688d
1 parent 2c5b097
commit 209688d
Show file tree

Hide file tree

Showing 17 changed files with 257 additions and 73 deletions.
diff --git a/.changeset/beige-ghosts-invent.md 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
@@ -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
@@ -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
@@ -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
@@ -164,7 +164,7 @@ export function NewsForm({ id }: FormProps): React.ReactElement {
                         </ToolbarTitleItem>
                         <ToolbarFillSpace />
                         <ToolbarActions>
-                            <FinalFormSaveSplitButton />
+                            <FinalFormSaveSplitButton hasConflict={saveConflict.hasConflict} />
                         </ToolbarActions>
                     </Toolbar>
                     <MainContent>

diff --git a/demo/admin/src/products/ProductForm.tsx b/demo/admin/src/products/ProductForm.tsx
@@ -186,7 +186,7 @@ function ProductForm({ id }: FormProps): React.ReactElement {
                         </ToolbarTitleItem>
                         <ToolbarFillSpace />
                         <ToolbarActions>
-                            <FinalFormSaveSplitButton />
+                            <FinalFormSaveSplitButton hasConflict={saveConflict.hasConflict} />
                         </ToolbarActions>
                     </Toolbar>
                     <MainContent>

diff --git 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 {
                         </ToolbarTitleItem>
                         <ToolbarFillSpace />
                         <ToolbarActions>
-                            <FinalFormSaveSplitButton />
+                            <FinalFormSaveSplitButton hasConflict={saveConflict.hasConflict} />
                         </ToolbarActions>
                     </Toolbar>
                     <MainContent>

diff --git 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 {
                         </ToolbarTitleItem>
                         <ToolbarFillSpace />
                         <ToolbarActions>
-                            <FinalFormSaveSplitButton />
+                            <FinalFormSaveSplitButton hasConflict={saveConflict.hasConflict} />
                         </ToolbarActions>
                     </Toolbar>
                     <MainContent>

diff --git 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 {
                         </ToolbarTitleItem>
                         <ToolbarFillSpace />
                         <ToolbarActions>
-                            <FinalFormSaveSplitButton />
+                            <FinalFormSaveSplitButton hasConflict={saveConflict.hasConflict} />
                         </ToolbarActions>
                     </Toolbar>
                     <MainContent>

diff --git a/packages/admin/admin/src/FinalFormSaveButton.tsx b/packages/admin/admin/src/FinalFormSaveButton.tsx
@@ -7,16 +7,18 @@ import { messages } from "./messages";
 
 interface FinalFormSaveButtonProps {
     message?: React.ReactNode;
+    hasConflict?: boolean;
 }
 
-export const FinalFormSaveButton = ({ message = <FormattedMessage {...messages.save} /> }: FinalFormSaveButtonProps) => {
+export const FinalFormSaveButton = ({ message = <FormattedMessage {...messages.save} />, hasConflict }: FinalFormSaveButtonProps) => {
     const form = useForm();
     const { pristine, hasValidationErrors, submitting, hasSubmitErrors } = useFormState();
 
     const isDisabled = pristine || hasValidationErrors || submitting;
 
     return (
         <SaveButton
+            hasConflict={hasConflict}
             disabled={isDisabled}
             color="primary"
             variant="contained"

diff --git a/packages/admin/admin/src/FinalFormSaveSplitButton.tsx b/packages/admin/admin/src/FinalFormSaveSplitButton.tsx
@@ -1,19 +1,21 @@
+import { ChevronDown } from "@comet/admin-icons";
 import * as React from "react";
 import { PropsWithChildren } from "react";
 import { useForm, useFormState } from "react-final-form";
 import { FormattedMessage } from "react-intl";
 
 import { SaveButton } from "./common/buttons/save/SaveButton";
-import { SplitButton } from "./common/buttons/split/SplitButton";
+import { SplitButton, SplitButtonProps } from "./common/buttons/split/SplitButton";
 import { FinalFormSubmitEvent } from "./FinalForm";
 import { messages } from "./messages";
 import { useStackApi } from "./stack/Api";
 
 export interface FormSaveButtonProps {
     localStorageKey?: string;
+    hasConflict?: boolean;
 }
 
-export const FinalFormSaveSplitButton = ({ localStorageKey = "SaveSplitButton" }: PropsWithChildren<FormSaveButtonProps>) => {
+export const FinalFormSaveSplitButton = ({ localStorageKey = "SaveSplitButton", hasConflict = false }: PropsWithChildren<FormSaveButtonProps>) => {
     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<SplitButtonProps> = {};
+    if (hasConflict) {
+        splitButtonProps.selectIcon = <ChevronDown sx={{ color: (theme) => theme.palette.error.contrastText }} />;
+    }
+
     return (
-        <SplitButton disabled={pristine || hasValidationErrors || submitting} localStorageKey={localStorageKey}>
+        <SplitButton {...splitButtonProps} disabled={pristine || hasValidationErrors || submitting} localStorageKey={localStorageKey}>
             <SaveButton
-                color="primary"
+                // setting the color to "error" is only necessary for the SplitButton and doesn't affect the SaveButton
+                color={hasConflict ? "error" : "primary"}
                 variant="contained"
                 saving={submitting}
                 hasErrors={hasSubmitErrors}
+                hasConflict={hasConflict}
                 onClick={async (clickEvent) => {
                     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
@@ -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<SaveButtonClassKey, SaveButtonProps>({
@@ -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
@@ -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 = <FormattedMessage {...messages.save} />,
     savingItem = <FormattedMessage id="comet.saveButton.savingItem.title" defaultMessage="Saving" />,
     successItem = <FormattedMessage id="comet.saveButton.successItem.title" defaultMessage="Successfully Saved" />,
     errorItem = <FormattedMessage id="comet.saveButton.errorItem.title" defaultMessage="Save Error" />,
+    conflictItem = <FormattedMessage {...messages.saveConflict} />,
     saveIcon = <Save />,
     savingIcon = <ThreeDotSaving />,
     successIcon = <Check />,
     errorIcon = <Error />,
+    conflictIcon = <ErrorIcon />,
     variant = "contained",
     color = "primary",
     classes,
@@ -51,16 +57,22 @@ const SaveBtn = ({
             return successIcon;
         } else if (displayState === "error") {
             return errorIcon;
+        } else if (displayState === "conflict") {
+            return conflictIcon;
         }
         return saveIcon;
     };
 
     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}
         </Button>
     );
 };
@@ -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
@@ -293,7 +293,7 @@ export async function writeCrudForm(generatorConfig: CrudGeneratorConfig, schema
                             </ToolbarTitleItem>
                             <ToolbarFillSpace />
                             <ToolbarActions>
-                                <FinalFormSaveSplitButton />
+                                <FinalFormSaveSplitButton hasConflict={saveConflict.hasConflict} />
                             </ToolbarActions>
                         </Toolbar>
                         <MainContent>