fix(Combobox): make value as string of integer work (#2095)

resolves #2081 resolves #2106 by setting the overscan to 7. Seems like 1 was too low, and 7 seems to work fine.
digdir · Jun 7, 2024 · 0a687f2 · 0a687f2
1 parent f1747b0
commit 0a687f2
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 29 deletions.
diff --git a/packages/react/src/components/form/Combobox/Combobox.stories.tsx b/packages/react/src/components/form/Combobox/Combobox.stories.tsx
@@ -673,3 +673,24 @@ export const RemoveAllOptions: StoryFn<typeof Combobox> = (args) => {
     </>
   );
 };
+
+export const WithNumberValues: StoryFn<typeof Combobox> = () => {
+  return (
+    <Combobox initialValue={['2000']}>
+      <Combobox.Option
+        id={'3000'}
+        key={'3000'}
+        value={'3000'}
+      >
+        some value
+      </Combobox.Option>
+      <Combobox.Option
+        id={'2000'}
+        key={'2000'}
+        value={'2000'}
+      >
+        some other value
+      </Combobox.Option>
+    </Combobox>
+  );
+};
diff --git a/packages/react/src/components/form/Combobox/Combobox.tsx b/packages/react/src/components/form/Combobox/Combobox.tsx
@@ -14,7 +14,7 @@ import { Spinner } from '../../Spinner';
 import { getSize } from '../../../utilities/getSize';
 
 import type { Option } from './useCombobox';
-import useCombobox from './useCombobox';
+import useCombobox, { prefix, removePrefix } from './useCombobox';
 import ComboboxInput from './internal/ComboboxInput';
 import ComboboxLabel from './internal/ComboboxLabel';
 import ComboboxError from './internal/ComboboxError';
@@ -209,23 +209,23 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
     // if value is set, set input value to the label of the value
     useEffect(() => {
       if (value && value.length > 0 && !multiple) {
-        const option = options[value[0]];
+        const option = options[prefix(value[0])];
         setInputValue(option?.label || '');
       }
     }, [multiple, value, options]);
 
     useEffect(() => {
       if (value && Object.keys(options).length >= 0) {
         const updatedSelectedOptions = value.map((option) => {
-          const value = options[option];
+          const value = options[prefix(option)];
           return value;
         });
 
         setSelectedOptions(
           updatedSelectedOptions.reduce<{
             [key: string]: Option;
           }>((acc, value) => {
-            acc[value.value] = value;
+            acc[prefix(value.value)] = value;
             return acc;
           }, {}),
         );
@@ -250,19 +250,21 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
 
       if (remove) {
         const newSelectedOptions = { ...selectedOptions };
-        delete newSelectedOptions[option.value];
+        delete newSelectedOptions[prefix(option.value)];
         setSelectedOptions(newSelectedOptions);
-        onValueChange?.(Object.keys(newSelectedOptions));
+        onValueChange?.(
+          Object.keys(newSelectedOptions).map((key) => removePrefix(key)),
+        );
         return;
       }
 
       const newSelectedOptions = { ...selectedOptions };
 
       if (multiple) {
-        if (newSelectedOptions[option.value]) {
-          delete newSelectedOptions[option.value];
+        if (newSelectedOptions[prefix(option.value)]) {
+          delete newSelectedOptions[prefix(option.value)];
         } else {
-          newSelectedOptions[option.value] = option;
+          newSelectedOptions[prefix(option.value)] = option;
         }
         setInputValue('');
         inputRef.current?.focus();
@@ -271,7 +273,7 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
         Object.keys(newSelectedOptions).forEach((key) => {
           delete newSelectedOptions[key];
         });
-        newSelectedOptions[option.value] = option;
+        newSelectedOptions[prefix(option.value)] = option;
         setInputValue(option?.label || '');
         // move cursor to the end of the input
         setTimeout(() => {
@@ -283,7 +285,9 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
       }
 
       setSelectedOptions(newSelectedOptions);
-      onValueChange?.(Object.keys(newSelectedOptions));
+      onValueChange?.(
+        Object.keys(newSelectedOptions).map((key) => removePrefix(key)),
+      );
 
       !multiple && setOpen(false);
       refs.domReference.current?.focus();
@@ -313,7 +317,7 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
       measureElement: (elem) => {
         return elem.getBoundingClientRect().height;
       },
-      overscan: 1,
+      overscan: 7,
     });
 
     return (
@@ -342,7 +346,7 @@ export const ComboboxComponent = forwardRef<HTMLInputElement, ComboboxProps>(
           onOptionClick: (value: string) => {
             if (readOnly) return;
             if (disabled) return;
-            const option = options[value];
+            const option = options[prefix(value)];
             debouncedHandleSelectOption({ option: option });
           },
           handleSelectOption: debouncedHandleSelectOption,

diff --git a/packages/react/src/components/form/Combobox/Option/useComboboxOption.tsx b/packages/react/src/components/form/Combobox/Option/useComboboxOption.tsx
@@ -4,6 +4,7 @@ import { useMergeRefs } from '@floating-ui/react';
 import { ComboboxContext } from '../ComboboxContext';
 import useDebounce from '../../../../utilities/useDebounce';
 import { useComboboxId, useComboboxIdDispatch } from '../ComboboxIdContext';
+import { prefix } from '../useCombobox';
 
 type UseComboboxOptionProps = {
   id?: string;
@@ -34,7 +35,7 @@ export default function useComboboxOption({
   } = context;
 
   const index = useMemo(
-    () => filteredOptions.indexOf(value) + customIds.length,
+    () => filteredOptions.indexOf(prefix(String(value))) + customIds.length,
     [customIds.length, filteredOptions, value],
   );
 
@@ -49,7 +50,7 @@ export default function useComboboxOption({
     throw new Error('Internal error: ComboboxOption did not find index');
   }
 
-  const selected = selectedOptions[value];
+  const selected = selectedOptions[prefix(value)];
   const active = activeIndex === index;
 
   useEffect(() => {

diff --git a/packages/react/src/components/form/Combobox/internal/ComboboxInput.tsx b/packages/react/src/components/form/Combobox/internal/ComboboxInput.tsx
@@ -9,6 +9,7 @@ import { Box } from '../../../Box';
 import { omit } from '../../../../utilities';
 import { useComboboxIdDispatch } from '../ComboboxIdContext';
 import type { ComboboxProps } from '../Combobox';
+import { prefix } from '../useCombobox';
 
 import ComboboxChips from './ComboboxChips';
 import ComboboxClearButton from './ComboboxClearButton';
@@ -68,9 +69,9 @@ export const ComboboxInput = ({
     setActiveIndex(0);
 
     // check if input value is the same as a label, if so, select it
-    const option = options[value.toLowerCase()];
+    const option = options[prefix(value.toLowerCase())];
     if (!option) return;
-    if (selectedOptions[option.value]) return;
+    if (selectedOptions[prefix(option.value)]) return;
 
     handleSelectOption({ option: option });
   };

diff --git a/packages/react/src/components/form/Combobox/internal/ComboboxNative.tsx b/packages/react/src/components/form/Combobox/internal/ComboboxNative.tsx
@@ -1,5 +1,6 @@
 import type { Option } from '../useCombobox';
 import type { ComboboxProps } from '../Combobox';
+import { removePrefix } from '../useCombobox';
 
 type ComboboxNativeProps = {
   selectedOptions: {
@@ -14,19 +15,17 @@ export const ComboboxNative = ({
   multiple,
   name,
 }: ComboboxNativeProps) => {
+  const VALUE = Object.keys(selectedOptions).map((key) => removePrefix(key));
+
   return (
     <select
       name={name}
       multiple={multiple}
       style={{ display: 'none' }}
-      value={
-        multiple
-          ? Object.keys(selectedOptions)
-          : Object.keys(selectedOptions)[0]
-      }
+      value={multiple ? VALUE : VALUE[0]}
       onChange={() => {}}
     >
-      {Object.keys(selectedOptions).map((value) => (
+      {VALUE.map((value) => (
         <option
           key={value}
           value={value}

diff --git a/packages/react/src/components/form/Combobox/useCombobox.tsx b/packages/react/src/components/form/Combobox/useCombobox.tsx
@@ -42,6 +42,21 @@ export function isInteractiveComboboxCustom(
   return isComboboxCustom(child) && child.props.interactive === true;
 }
 
+const INTERNAL_OPTION_PREFIX = 'internal-option-';
+
+/**
+ * We use this function to prefix the value of the options so we can make sure numbers as strings are not parsed as numbers in objects
+ * @param value
+ * @returns
+ */
+export const prefix = (value?: string): string => {
+  return INTERNAL_OPTION_PREFIX + value;
+};
+
+export const removePrefix = (value: string): string => {
+  return value.slice(INTERNAL_OPTION_PREFIX.length);
+};
+
 export default function useCombobox({
   children,
   inputValue,
@@ -115,8 +130,8 @@ export default function useCombobox({
         label = childrenLabel;
       }
 
-      allOptions[props.value] = {
-        value: props.value,
+      allOptions[prefix(String(props.value))] = {
+        value: String(props.value),
         label,
         displayValue: props.displayValue,
         description: props.description,
@@ -128,7 +143,11 @@ export default function useCombobox({
 
   const preSelectedOptions = useMemo(
     () =>
-      (initialValue || []).reduce<{
+      (
+        initialValue?.map((key) => {
+          return prefix(key);
+        }) || []
+      ).reduce<{
         [key: string]: Option;
       }>((acc, value) => {
         const option = options[value];
@@ -151,16 +170,17 @@ export default function useCombobox({
       (option, index) => {
         /* If we have a selected value in single mode and the input matches an option, return all children */
         if (!multiple && Object.keys(selectedOptions).length === 1) {
-          filteredOptions.push(options[option].value);
+          filteredOptions.push(option);
           return optionsChildren[index];
         }
 
         if (multiple && selectedOptions[option]) {
-          filteredOptions.push(options[option].value);
+          filteredOptions.push(option);
           return optionsChildren[index];
         }
+
         if (filter(inputValue, options[option])) {
-          filteredOptions.push(options[option].value);
+          filteredOptions.push(option);
           return optionsChildren[index];
         }
       },