[WIP] Outfit-saving UI

The auto-saving frontend! It seems to trigger saves at the right times, but they fail, because the backend doesn't support updates yet!

src/app/WardrobePage/ItemsPanel.js

@@ -17,23 +17,19 @@ import {
   Portal,
   Button,
   useToast,
-  Popover,
+  Spinner,
-  PopoverTrigger,
+  useColorModeValue,
-  PopoverContent,
-  PopoverArrow,
-  PopoverBody,
 } from "@chakra-ui/react";
 import {
   CheckIcon,
   EditIcon,
-  ExternalLinkIcon,
   QuestionIcon,
-  WarningIcon,
+  WarningTwoIcon,
 } from "@chakra-ui/icons";
 import { CSSTransition, TransitionGroup } from "react-transition-group";
 import { useHistory } from "react-router-dom";
 
-import { Delay, Heading1, Heading2 } from "../util";
+import { Delay, Heading1, Heading2, useDebounce } from "../util";
 import Item, { ItemListContainer, ItemListSkeleton } from "./Item";
 import { BiRename } from "react-icons/bi";
 import { IoCloudUploadOutline } from "react-icons/io5";
@@ -267,6 +263,10 @@ function useOutfitSaving(outfitState) {
   const history = useHistory();
   const toast = useToast();
 
+  // There's not a way to reset an Apollo mutation state to clear out the error
+  // when the outfit changes… so we track the error state ourselves!
+  const [saveError, setSaveError] = React.useState(null);
+
   // Whether this outfit is new, i.e. local-only, i.e. has _never_ been saved
   // to the server.
   const isNewOutfit = outfitState.id == null;
@@ -326,15 +326,6 @@ function useOutfitSaving(outfitState) {
       }
     `,
     {
-      variables: {
-        id: outfitState.id, // Optional, is null when saving new outfits
-        name: outfitState.name, // Optional, server may fill in a placeholder
-        speciesId: outfitState.speciesId,
-        colorId: outfitState.colorId,
-        pose: outfitState.pose,
-        wornItemIds: outfitState.wornItemIds,
-        closetedItemIds: outfitState.closetedItemIds,
-      },
       context: { sendAuth: true },
       update: (cache, { data: { outfit } }) => {
         // After save, add this outfit to the current user's outfit list. This
@@ -360,29 +351,99 @@ function useOutfitSaving(outfitState) {
     }
   );
 
-  const saveOutfit = React.useCallback(() => {
+  const saveOutfitFromProvidedState = React.useCallback(
-    sendSaveOutfitMutation()
+    (outfitState) => {
-      .then(({ data: { outfit } }) => {
+      sendSaveOutfitMutation({
-        // Navigate to the new saved outfit URL. Our Apollo cache should pick
+        variables: {
-        // up the data from this mutation response, and combine it with the
+          id: outfitState.id, // Optional, is null when saving new outfits
-        // existing cached data, to make this smooth without any loading UI.
+          name: outfitState.name, // Optional, server may fill in a placeholder
-        history.push(`/outfits/${outfit.id}`);
+          speciesId: outfitState.speciesId,
+          colorId: outfitState.colorId,
+          pose: outfitState.pose,
+          wornItemIds: outfitState.wornItemIds,
+          closetedItemIds: outfitState.closetedItemIds,
+        },
       })
-      .catch((e) => {
+        .then(({ data: { outfit } }) => {
-        console.error(e);
+          // Navigate to the new saved outfit URL. Our Apollo cache should pick
-        toast({
+          // up the data from this mutation response, and combine it with the
-          status: "error",
+          // existing cached data, to make this smooth without any loading UI.
-          title: "Sorry, there was an error saving this outfit!",
+          history.push(`/outfits/${outfit.id}`);
-          description: "Maybe check your connection and try again.",
+        })
+        .catch((e) => {
+          console.error(e);
+          setSaveError(e);
+          toast({
+            status: "error",
+            title: "Sorry, there was an error saving this outfit!",
+            description: "Maybe check your connection and try again.",
+          });
         });
-      });
+    },
-  }, [sendSaveOutfitMutation, history, toast]);
+    // It's important that this callback _doesn't_ change when the outfit
+    // changes, so that the auto-save effect is only responding to the
+    // debounced state!
+    [sendSaveOutfitMutation, history, toast]
+  );
+
+  const saveOutfit = React.useCallback(
+    () => saveOutfitFromProvidedState(outfitState.outfitStateWithoutExtras),
+    [saveOutfitFromProvidedState, outfitState.outfitStateWithoutExtras]
+  );
+
+  // Auto-saving! First, debounce the outfit state. Use `outfitStateWithoutExtras`,
+  // which only contains the basic fields, and will keep a stable object
+  // identity until actual changes occur. Then, save the outfit after the user
+  // has left it alone for long enough, so long as it's actually different
+  // than the saved state.
+  const debouncedOutfitState = useDebounce(
+    outfitState.outfitStateWithoutExtras,
+    2000
+  );
+  // HACK: This prevents us from auto-saving the outfit state that's still
+  //       loading. I worry that this might not catch other loading scenarios
+  //       though, like if the species/color/pose is in the GQL cache, but the
+  //       items are still loading in... not sure where this would happen tho!
+  const debouncedOutfitStateIsSaveable =
+    debouncedOutfitState.speciesId &&
+    debouncedOutfitState.colorId &&
+    debouncedOutfitState.pose;
+  React.useEffect(() => {
+    if (
+      !isNewOutfit &&
+      canSaveOutfit &&
+      debouncedOutfitStateIsSaveable &&
+      !outfitStatesAreEqual(debouncedOutfitState, outfitState.savedOutfitState)
+    ) {
+      console.info(
+        "[useOutfitSaving] Auto-saving outfit from old state to new state",
+        outfitState.savedOutfitState,
+        debouncedOutfitState
+      );
+      saveOutfitFromProvidedState(debouncedOutfitState);
+    }
+  }, [
+    isNewOutfit,
+    canSaveOutfit,
+    debouncedOutfitState,
+    debouncedOutfitStateIsSaveable,
+    outfitState.savedOutfitState,
+    saveOutfitFromProvidedState,
+  ]);
+
+  // When the outfit changes, clear out the error state from previous saves.
+  // We'll send the mutation again after the debounce, and we don't want to
+  // show the error UI in the meantime!
+  React.useEffect(() => {
+    setSaveError(null);
+  }, [outfitState.outfitStateWithoutExtras]);
 
   return {
     canSaveOutfit,
     isNewOutfit,
     isSaving,
     latestVersionIsSaved,
+    saveError,
     saveOutfit,
   };
 }
@@ -397,9 +458,12 @@ function OutfitSavingIndicator({ outfitState }) {
     isNewOutfit,
     isSaving,
     latestVersionIsSaved,
+    saveError,
     saveOutfit,
   } = useOutfitSaving(outfitState);
 
+  const errorTextColor = useColorModeValue("red.600", "red.400");
+
   if (!canSaveOutfit) {
     return null;
   }
@@ -427,6 +491,24 @@ function OutfitSavingIndicator({ outfitState }) {
     );
   }
 
+  if (isSaving) {
+    return (
+      <Flex
+        align="center"
+        fontSize="xs"
+        data-test-id="wardrobe-outfit-is-saving-indicator"
+      >
+        <Spinner
+          size="xs"
+          marginRight="1.5"
+          // HACK: Not sure why my various centering things always feel wrong...
+          marginBottom="-2px"
+        />
+        Saving…
+      </Flex>
+    );
+  }
+
   if (latestVersionIsSaved) {
     return (
       <Flex
@@ -444,37 +526,27 @@ function OutfitSavingIndicator({ outfitState }) {
     );
   }
 
-  return (
+  if (saveError) {
-    <Popover trigger="hover">
+    return (
-      <PopoverTrigger>
+      <Flex
-        <Flex align="center" fontSize="xs" tabIndex="0">
+        align="center"
-          <WarningIcon
+        fontSize="xs"
-            marginRight="1"
+        data-test-id="wardrobe-outfit-save-error-indicator"
-            // HACK: Not sure why my various centering things always feel wrong...
+        color={errorTextColor}
-            marginBottom="-2px"
+      >
-          />
+        <WarningTwoIcon
-          Not saved
+          marginRight="1"
-        </Flex>
+          // HACK: Not sure why my various centering things always feel wrong...
-      </PopoverTrigger>
+          marginBottom="-2px"
-      <PopoverContent>
+        />
-        <PopoverArrow />
+        Error saving
-        <PopoverBody>
+      </Flex>
-          We're still working on this! For now, use{" "}
+    );
-          <Box
+  }
-            as="a"
+
-            href={`https://impress.openneo.net/outfits/${outfitState.id}`}
+  // The most common way we'll hit this null is when the outfit is changing,
-            target="_blank"
+  // but the debouncing isn't done yet, so it's not saving yet.
-          >
+  return null;
-            <Box as="span" textDecoration="underline">
-              Classic DTI
-            </Box>
-            <ExternalLinkIcon marginLeft="1" marginTop="-3px" fontSize="sm" />
-          </Box>{" "}
-          to save existing outfits.
-        </PopoverBody>
-      </PopoverContent>
-    </Popover>
-  );
 }
 
 /**

src/app/WardrobePage/useOutfitState.js

@@ -2,7 +2,7 @@ import React from "react";
 import gql from "graphql-tag";
 import produce, { enableMapSet } from "immer";
 import { useQuery, useApolloClient } from "@apollo/client";
-import { useParams } from "react-router-dom";
+import { useLocation, useParams } from "react-router-dom";
 
 import { itemAppearanceFragment } from "../components/useOutfitAppearance";
 
@@ -70,7 +70,12 @@ function useOutfitState() {
 
   const creator = outfitData?.outfit?.creator;
 
-  const savedOutfitState = getOutfitStateFromOutfitData(outfitData?.outfit);
+  // We memoize this to make `outfitStateWithoutExtras` an even more reliable
+  // stable object!
+  const savedOutfitState = React.useMemo(
+    () => getOutfitStateFromOutfitData(outfitData?.outfit),
+    [outfitData?.outfit]
+  );
 
   // Choose which customization state to use. We want it to match the outfit in
   // the URL immediately, without having to wait for any effects, to avoid race
@@ -92,14 +97,17 @@ function useOutfitState() {
   if (urlOutfitState.id === localOutfitState.id) {
     // Use the reducer state: they're both for the same saved outfit, or both
     // for an unsaved outfit (null === null).
+    console.debug("Choosing local outfit state");
     outfitState = localOutfitState;
   } else if (urlOutfitState.id && urlOutfitState.id === savedOutfitState.id) {
     // Use the saved outfit state: it's for the saved outfit the URL points to.
+    console.debug("Choosing saved outfit state");
     outfitState = savedOutfitState;
   } else {
     // Use the URL state: it's more up-to-date than any of the others. (Worst
     // case, it's empty except for ID, which is fine while the saved outfit
     // data loads!)
+    console.debug("Choosing URL outfit state");
     outfitState = urlOutfitState;
   }
 
@@ -231,6 +239,12 @@ function useOutfitState() {
     pose,
     appearanceId,
     url,
+
+    // We use this plain outfit state objects in `useOutfitSaving`! Unlike the
+    // full `outfitState` object, which we rebuild each render,
+    // `outfitStateWithoutExtras` will mostly only change when there is an
+    // actual change to outfit state.
+    outfitStateWithoutExtras: outfitState,
     savedOutfitState,
   };
 
@@ -248,7 +262,7 @@ function useOutfitState() {
 }
 
 const outfitStateReducer = (apolloClient) => (baseState, action) => {
-  console.info("[Outfit state] Action:", action);
+  console.info("[useOutfitState] Action:", action);
   switch (action.type) {
     case "rename":
       return produce(baseState, (state) => {
@@ -356,29 +370,36 @@ const EMPTY_CUSTOMIZATION_STATE = {
 
 function useParseOutfitUrl() {
   const { id } = useParams();
+  const { search } = useLocation();
 
-  // For the /outfits/:id page, ignore the query string, and just wait for the
+  // We memoize this to make `outfitStateWithoutExtras` an even more reliable
-  // outfit data to load in!
+  // stable object!
-  if (id != null) {
+  const memoizedOutfitState = React.useMemo(() => {
+    // For the /outfits/:id page, ignore the query string, and just wait for the
+    // outfit data to load in!
+    if (id != null) {
+      return {
+        ...EMPTY_CUSTOMIZATION_STATE,
+        id,
+      };
+    }
+
+    // Otherwise, parse the query string, and fill in default values for anything
+    // not specified.
+    const urlParams = new URLSearchParams(search);
     return {
-      ...EMPTY_CUSTOMIZATION_STATE,
+      id: null,
-      id,
+      name: urlParams.get("name"),
+      speciesId: urlParams.get("species") || "1",
+      colorId: urlParams.get("color") || "8",
+      pose: urlParams.get("pose") || "HAPPY_FEM",
+      appearanceId: urlParams.get("state") || null,
+      wornItemIds: new Set(urlParams.getAll("objects[]")),
+      closetedItemIds: new Set(urlParams.getAll("closet[]")),
     };
-  }
+  }, [id, search]);
 
-  // Otherwise, parse the query string, and fill in default values for anything
+  return memoizedOutfitState;
-  // not specified.
-  const urlParams = new URLSearchParams(window.location.search);
-  return {
-    id: null,
-    name: urlParams.get("name"),
-    speciesId: urlParams.get("species") || "1",
-    colorId: urlParams.get("color") || "8",
-    pose: urlParams.get("pose") || "HAPPY_FEM",
-    appearanceId: urlParams.get("state") || null,
-    wornItemIds: new Set(urlParams.getAll("objects[]")),
-    closetedItemIds: new Set(urlParams.getAll("closet[]")),
-  };
 }
 
 function getOutfitStateFromOutfitData(outfit) {
@@ -603,9 +624,9 @@ function buildOutfitQueryString(outfitState) {
 
   const params = new URLSearchParams({
     name: name || "",
-    species: speciesId,
+    species: speciesId || "",
-    color: colorId,
+    color: colorId || "",
-    pose,
+    pose: pose || "",
   });
   for (const itemId of wornItemIds) {
     params.append("objects[]", itemId);