Merge pull request #1405 from Shelf-nu/1404-bug-disselecting-category…

…-in-assetedit-doesnt-clear-params-and-doesnt-revalidate-to-show-the-correct-fields fix: asset form category selector
Shelf-nu · Nov 8, 2024 · 293e92d · 293e92d
2 parents ff7f689 + ffe0e84
commit 293e92d
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 28 deletions.
diff --git a/app/components/assets/assets-index/advanced-filters/value-field.tsx b/app/components/assets/assets-index/advanced-filters/value-field.tsx
@@ -601,7 +601,9 @@ function CustodyEnumField({
       placeholder="Select custodian"
       defaultValue={value as string}
       onChange={(selectedId) => {
-        handleChange(selectedId);
+        if (selectedId !== undefined) {
+          handleChange(selectedId);
+        }
       }}
       closeOnSelect={true}
       triggerWrapperClassName="w-full text-gray-700"
@@ -710,7 +712,9 @@ function CategoryEnumField({
       placeholder="Select category"
       defaultValue={value as string}
       onChange={(selectedId) => {
-        handleChange(selectedId);
+        if (selectedId !== undefined) {
+          handleChange(selectedId);
+        }
       }}
       closeOnSelect={true}
       triggerWrapperClassName="w-full text-gray-700"
@@ -811,7 +815,9 @@ function LocationEnumField({
       placeholder="Select location"
       defaultValue={value as string}
       onChange={(selectedId) => {
-        handleChange(selectedId);
+        if (selectedId !== undefined) {
+          handleChange(selectedId);
+        }
       }}
       closeOnSelect={true}
       triggerWrapperClassName="w-full text-gray-700"
@@ -910,7 +916,9 @@ function KitEnumField({
       placeholder="Select kit"
       defaultValue={value as string}
       onChange={(selectedId) => {
-        handleChange(selectedId);
+        if (selectedId !== undefined) {
+          handleChange(selectedId);
+        }
       }}
       closeOnSelect={true}
       triggerWrapperClassName="w-full text-gray-700"

diff --git a/app/components/assets/form.tsx b/app/components/assets/form.tsx
@@ -263,7 +263,7 @@ export const AssetForm = ({
             countKey="totalCategories"
             closeOnSelect
             selectionMode="set"
-            allowClear
+            allowClear={true}
             extraContent={
               <Button
                 to="/categories/new"
@@ -319,6 +319,7 @@ export const AssetForm = ({
           />
           <DynamicSelect
             disabled={disabled}
+            selectionMode="set"
             fieldName="newLocationId"
             defaultValue={location || undefined}
             model={{ name: "location", queryKey: "name" }}

diff --git a/app/components/dynamic-select/dynamic-select.tsx b/app/components/dynamic-select/dynamic-select.tsx
@@ -49,10 +49,10 @@ type Props = ModelFilterProps & {
   placeholder?: string;
   closeOnSelect?: boolean;
   excludeItems?: string[];
-  onChange?: ((value: string) => void) | null;
-  /**
+  /** Allow undefined for deselection cases */
+  onChange?: ((value: string | undefined) => void) | null /**
    * Allow item to unselect on clicking again
-   */
+   */;
   allowClear?: boolean;
   hidden?: boolean;
 };
@@ -110,14 +110,16 @@ export default function DynamicSelect({
   );
 
   function handleItemChange(id: string) {
-    if (allowClear && selectedValue === id) {
-      setSelectedValue(undefined);
-    } else {
-      setSelectedValue(id);
-      handleSelectItemChange(id);
-    }
+    const isDeselecting = allowClear && selectedValue === id;
+
+    // Update local state
+    setSelectedValue(isDeselecting ? undefined : id);
+
+    // Always update URL params and parent state
+    handleSelectItemChange(id);
 
-    onChange && onChange(id);
+    // Notify parent with the new value
+    onChange?.(isDeselecting ? undefined : id);
 
     if (closeOnSelect) {
       setIsPopoverOpen(false);

diff --git a/app/components/forms/categories-input.tsx b/app/components/forms/categories-input.tsx
@@ -38,8 +38,10 @@ export default function CategoriesInput({
             className="flex-1"
             excludeItems={categories}
             onChange={(value) => {
-              categories[i] = value;
-              setCategories([...categories]);
+              if (value !== undefined) {
+                categories[i] = value;
+                setCategories([...categories]);
+              }
             }}
           />
 

diff --git a/app/hooks/use-model-filters.ts b/app/hooks/use-model-filters.ts
@@ -138,6 +138,10 @@ export function useModelFilters({
     hasAllData,
   ]);
 
+  /**
+   * Handles selection/deselection of items and updates URL params according to selectionMode
+   * @param value - The value being selected/deselected
+   */
   const handleSelectItemChange = useCallback(
     (value: string) => {
       if (selectionMode === "none") {
@@ -149,25 +153,44 @@ export function useModelFilters({
         return;
       }
 
-      if (selectedItems.includes(value)) {
-        setSelectedItems((prev) => prev.filter((item) => item !== value));
-        setSearchParams((prev) => {
-          prev.delete(model.name, value);
-          return prev;
-        });
-      } else {
-        setSelectedItems((prev) => [...prev, value]);
+      const isDeselecting = selectedItems.includes(value);
+
+      if (selectionMode === "set") {
+        // In set mode, we either set a new value or remove it completely
+        setSelectedItems(isDeselecting ? [] : [value]);
         setSearchParams(
           (prev) => {
-            if (selectionMode === "append") {
-              prev.append(model.name, value);
+            if (isDeselecting) {
+              prev.delete(model.name);
             } else {
               prev.set(model.name, value);
             }
             return prev;
           },
           { preventScrollReset: true }
         );
+      } else if (selectionMode === "append") {
+        // In append mode, we maintain multiple values
+        setSelectedItems((prev) =>
+          isDeselecting
+            ? prev.filter((item) => item !== value)
+            : [...prev, value]
+        );
+        setSearchParams(
+          (prev) => {
+            if (isDeselecting) {
+              prev.delete(model.name, value);
+            } else {
+              prev.append(model.name, value);
+            }
+            return prev;
+          },
+          { preventScrollReset: true }
+        );
+      }
+
+      if (onSelectionChange) {
+        onSelectionChange(isDeselecting ? [] : [value]);
       }
     },
     [

diff --git a/app/routes/_layout+/assets.new.tsx b/app/routes/_layout+/assets.new.tsx
@@ -224,11 +224,14 @@ export default function NewAssetPage() {
   const title = useAtomValue(dynamicTitleAtom);
   const [searchParams] = useSearchParams();
   const qrId = searchParams.get("qrId");
+
+  // Get category from URL params or use the default passed prop
+  const categoryFromUrl = searchParams.get("category");
   return (
     <>
       <Header title={title ? title : "Untitled Asset"} />
       <div>
-        <AssetForm qrId={qrId} />
+        <AssetForm qrId={qrId} category={categoryFromUrl || undefined} />
       </div>
     </>
   );