From 62629865d86eb8fa1e77c239432e9d9222b148dc Mon Sep 17 00:00:00 2001
From: Matchu <matchu1993@gmail.com>
Date: Wed, 2 Sep 2020 00:53:35 -0700
Subject: [PATCH] stop removing items after you try something on

I was getting annoyed by how, when you're using search, trying on an item will remove conflicting stuff, and then if you decide you don't like what you tried the old stuff _doesn't come back_

As of this change, it does! When you start a new search, we save the outfit state, and then whenever you change the items we ask "hey can these old ones safely be re-worn again?" and re-wear them if so.
---
 src/app/WardrobePage/Item.js           |  7 ++---
 src/app/WardrobePage/ItemsPanel.js     |  4 ++-
 src/app/WardrobePage/SearchPanel.js    | 36 +++++++++++++++++++++++---
 src/app/WardrobePage/useOutfitState.js | 34 +++++++++++++++++++++---
 4 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/src/app/WardrobePage/Item.js b/src/app/WardrobePage/Item.js
index 93f800e..ab8f55d 100644
--- a/src/app/WardrobePage/Item.js
+++ b/src/app/WardrobePage/Item.js
@@ -36,7 +36,8 @@ const LoadableItemSupportDrawer = loadable(() =>
  *
  * In fact, this component can't trigger wear or unwear events! When you click
  * it in the app, you're actually clicking a <label> that wraps the radio or
- * checkbox. We _do_ control the Remove button in here, though!
+ * checkbox. Similarly, the parent provides the `onRemove` callback for the
+ * Remove button.
  *
  * NOTE: This component is memoized with React.memo. It's surpisingly expensive
  *       to re-render, because Chakra components are a lil bit expensive from
@@ -50,7 +51,7 @@ function Item({
   itemNameId,
   isWorn,
   isInOutfit,
-  dispatchToOutfit,
+  onRemove,
   isDisabled = false,
 }) {
   const [supportDrawerIsOpen, setSupportDrawerIsOpen] = React.useState(false);
@@ -114,7 +115,7 @@ function Item({
               icon={<DeleteIcon />}
               label="Remove"
               onClick={(e) => {
-                dispatchToOutfit({ type: "removeItem", itemId: item.id });
+                onRemove();
                 e.preventDefault();
               }}
             />
diff --git a/src/app/WardrobePage/ItemsPanel.js b/src/app/WardrobePage/ItemsPanel.js
index 666c605..3a1c265 100644
--- a/src/app/WardrobePage/ItemsPanel.js
+++ b/src/app/WardrobePage/ItemsPanel.js
@@ -136,7 +136,9 @@ function ItemZoneGroup({
                   !isDisabled && outfitState.wornItemIds.includes(item.id)
                 }
                 isInOutfit={outfitState.allItemIds.includes(item.id)}
-                dispatchToOutfit={dispatchToOutfit}
+                onRemove={() =>
+                  dispatchToOutfit({ type: "removeItem", itemId: item.id })
+                }
                 isDisabled={isDisabled}
               />
             );
diff --git a/src/app/WardrobePage/SearchPanel.js b/src/app/WardrobePage/SearchPanel.js
index acb1856..5df3685 100644
--- a/src/app/WardrobePage/SearchPanel.js
+++ b/src/app/WardrobePage/SearchPanel.js
@@ -48,6 +48,16 @@ function SearchPanel({
       }}
     >
       <SearchResults
+        // When the query changes, replace the SearchResults component with a
+        // new instance, to reset `itemIdsToReconsider`. That way, if you find
+        // an item you like in one search, then immediately do a second search
+        // and try a conflicting item, we'll restore the item you liked from
+        // your first search!
+        //
+        // NOTE: I wonder how this affects things like state. This component
+        //       also tries to gracefully handle changes in the query, but tbh
+        //       I wonder whether that's still necessary...
+        key={serializeQuery(query)}
         query={query}
         outfitState={outfitState}
         dispatchToOutfit={dispatchToOutfit}
@@ -81,14 +91,20 @@ function SearchResults({
   );
   useScrollTracker(scrollContainerRef, 300, fetchMore);
 
+  // This will save the `wornItemIds` when the SearchResults first mounts, and
+  // keep it saved even after the outfit changes. We use this to try to restore
+  // these items after the user makes changes, e.g., after they try on another
+  // Background we want to restore the previous one!
+  const [itemIdsToReconsider] = React.useState(outfitState.wornItemIds);
+
   // When the checkbox changes, we should wear/unwear the item!
   const onChange = (e) => {
     const itemId = e.target.value;
     const willBeWorn = e.target.checked;
     if (willBeWorn) {
-      dispatchToOutfit({ type: "wearItem", itemId });
+      dispatchToOutfit({ type: "wearItem", itemId, itemIdsToReconsider });
     } else {
-      dispatchToOutfit({ type: "unwearItem", itemId });
+      dispatchToOutfit({ type: "unwearItem", itemId, itemIdsToReconsider });
     }
   };
 
@@ -173,7 +189,13 @@ function SearchResults({
               item={item}
               isWorn={outfitState.wornItemIds.includes(item.id)}
               isInOutfit={outfitState.allItemIds.includes(item.id)}
-              dispatchToOutfit={dispatchToOutfit}
+              onRemove={() =>
+                dispatchToOutfit({
+                  type: "removeItem",
+                  itemId: item.id,
+                  itemIdsToReconsider,
+                })
+              }
             />
           </label>
         ))}
@@ -405,4 +427,12 @@ function useScrollTracker(scrollContainerRef, threshold, onScrolledToBottom) {
   }, [onScroll, scrollContainerRef]);
 }
 
+/**
+ * serializeQuery stably converts a search query object to a string, for easier
+ * JS comparison.
+ */
+function serializeQuery(query) {
+  return `${JSON.stringify([query.value, query.filterToZoneLabel])}`;
+}
+
 export default SearchPanel;
diff --git a/src/app/WardrobePage/useOutfitState.js b/src/app/WardrobePage/useOutfitState.js
index 867b5db..1882202 100644
--- a/src/app/WardrobePage/useOutfitState.js
+++ b/src/app/WardrobePage/useOutfitState.js
@@ -124,7 +124,7 @@ const outfitStateReducer = (apolloClient) => (baseState, action) => {
     case "wearItem":
       return produce(baseState, (state) => {
         const { wornItemIds, closetedItemIds } = state;
-        const { itemId } = action;
+        const { itemId, itemIdsToReconsider = [] } = action;
 
         // Move conflicting items to the closet.
         //
@@ -153,24 +153,30 @@ const outfitStateReducer = (apolloClient) => (baseState, action) => {
         // Move this item from the closet to the worn set.
         closetedItemIds.delete(itemId);
         wornItemIds.add(itemId);
+
+        reconsiderItems(itemIdsToReconsider, state, apolloClient);
       });
     case "unwearItem":
       return produce(baseState, (state) => {
         const { wornItemIds, closetedItemIds } = state;
-        const { itemId } = action;
+        const { itemId, itemIdsToReconsider = [] } = action;
 
         // Move this item from the worn set to the closet.
         wornItemIds.delete(itemId);
         closetedItemIds.add(itemId);
+
+        reconsiderItems(itemIdsToReconsider, state, apolloClient);
       });
     case "removeItem":
       return produce(baseState, (state) => {
         const { wornItemIds, closetedItemIds } = state;
-        const { itemId } = action;
+        const { itemId, itemIdsToReconsider = [] } = action;
 
         // Remove this item from both the worn set and the closet.
         wornItemIds.delete(itemId);
         closetedItemIds.delete(itemId);
+
+        reconsiderItems(itemIdsToReconsider, state, apolloClient);
       });
     case "setPose":
       return {
@@ -305,6 +311,28 @@ function setsIntersect(a, b) {
   return false;
 }
 
+/**
+ * Try to add these items back to the outfit, if there would be no conflicts.
+ * We use this in Search to try to restore these items after the user makes
+ * changes, e.g., after they try on another Background we want to restore the
+ * previous one!
+ *
+ * This mutates state.wornItemIds directly, on the assumption that we're in an
+ * immer block, in which case mutation is the simplest API!
+ */
+function reconsiderItems(itemIdsToReconsider, state, apolloClient) {
+  for (const itemIdToReconsider of itemIdsToReconsider) {
+    const conflictingIds = findItemConflicts(
+      itemIdToReconsider,
+      state,
+      apolloClient
+    );
+    if (conflictingIds.length === 0) {
+      state.wornItemIds.add(itemIdToReconsider);
+    }
+  }
+}
+
 // TODO: Get this out of here, tbh...
 function getZonesAndItems(itemsById, wornItemIds, closetedItemIds) {
   const wornItems = wornItemIds.map((id) => itemsById[id]).filter((i) => i);