fix: Input arrow up and down pulls up autocomplete now instead of incrementing (#4291)

## Description

closes #4287

- [x] Up/Down now uses `onChange` instead of `onChangeComplete` (this
should be faster)
- [x] Up/Down in the Space control now works with modifiers (it seems
this never worked before)
- [x] Up/Down no longer opens the combobox if the input is
numeric-like​⬤

## Steps for reproduction

1. click button
2. expect xyz

## Code Review

- [ ] hi @kof, I need you to do
  - conceptual review (architecture, feature-correctness)
  - detailed review (read every line)
  - test it on preview

## Before requesting a review

- [ ] made a self-review
- [ ] added inline comments where things may be not obvious (the "why",
not "what")

## Before merging

- [ ] tested locally and on preview environment (preview dev login:
5de6)
- [ ] updated [test
cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md)
document
- [ ] added tests
- [ ] if any new env variables are added, added them to `.env` file

Author: istarkov

apps/builder/app/builder/features/style-panel/shared/css-value-input/css-value-input.tsx

@@ -218,63 +218,6 @@ const useScrub = ({
 export const isNumericString = (input: string) =>
   String(input).trim().length !== 0 && Number.isNaN(Number(input)) === false;
 
-const useHandleKeyDown =
-  ({
-    ignoreEnter,
-    ignoreUpDownNumeric,
-    value,
-    onChange,
-    onKeyDown,
-  }: {
-    ignoreEnter: boolean;
-    ignoreUpDownNumeric: boolean;
-    value: CssValueInputValue;
-    onChange: (event: {
-      value: CssValueInputValue;
-      altKey: boolean;
-      shiftKey: boolean;
-    }) => void;
-    onKeyDown: KeyboardEventHandler<HTMLInputElement>;
-  }) =>
-  (event: KeyboardEvent<HTMLInputElement>) => {
-    if (event.defaultPrevented) {
-      // Underlying select like `unitSelect` can already prevent an event like up/down buttons
-      return;
-    }
-    const meta = { altKey: event.altKey, shiftKey: event.shiftKey };
-
-    // Do not prevent downshift behaviour on item select
-    if (ignoreEnter === false) {
-      if (event.key === "Enter") {
-        onChange({ value, ...meta });
-      }
-    }
-
-    if (
-      ignoreUpDownNumeric === false &&
-      (value.type === "unit" ||
-        (value.type === "intermediate" && isNumericString(value.value))) &&
-      value.unit !== undefined &&
-      (event.key === "ArrowUp" || event.key === "ArrowDown")
-    ) {
-      const inputValue =
-        value.type === "unit" ? value.value : Number(value.value.trim());
-
-      onChange({
-        value: {
-          type: "unit",
-          value: handleNumericInputArrowKeys(inputValue, event),
-          unit: value.unit,
-        },
-        ...meta,
-      });
-      // Prevent Downshift from opening menu on arrow up/down
-      return;
-    }
-
-    onKeyDown(event);
-  };
-
 export type IntermediateStyleValue = {
   type: "intermediate";
   value: string;
@@ -289,7 +232,13 @@ type Modifiers = {
 };
 
 type ChangeCompleteEvent = {
-  type: "enter" | "blur" | "scrub-end" | "unit-select" | "keyword-select";
+  type:
+    | "enter"
+    | "blur"
+    | "scrub-end"
+    | "unit-select"
+    | "keyword-select"
+    | "delta";
   value: StyleValue;
 } & Modifiers;
 
@@ -355,6 +304,7 @@ const Description = styled(Box, { width: theme.spacing[27] });
  *   - shift key modifier increases/decreases value by 10
  *   - option/alt key modifier increases/decreases value by 0.1
  *   - no modifier increases/decreases value by 1
+ *   - does not open the combobox when the input is a number (CSS root variables can include numbers in their names)
  * - Scrub interaction
  * - Click outside, unit selection or escape when list is open should unfocus the unit select trigger
  *
@@ -612,18 +562,6 @@ export const CssValueInput = ({
     onChangeComplete({ value, type: "blur" });
   };
 
-  const handleKeyDown = useHandleKeyDown({
-    // In case of the menu is really open and the selection is inside it
-    // we do not prevent the default downshift Enter key behavior
-    ignoreEnter:
-      isUnitsOpen || (isOpen && !menuProps.empty && highlightedIndex !== -1),
-    // Do not change the number value on the arrow up/down if any menu is opened
-    ignoreUpDownNumeric: isUnitsOpen || isOpen,
-    onChange: (event) => onChangeComplete({ ...event, type: "enter" }),
-    value,
-    onKeyDown: inputProps.onKeyDown,
-  });
-
   const finalPrefix =
     prefix ||
     (icon && (
@@ -692,9 +630,67 @@ export const CssValueInput = ({
     .filter(Boolean)
     .map((descr) => <Description>{descr}</Description>);
 
+  const handleUpDownNumeric = (event: KeyboardEvent<HTMLInputElement>) => {
+    const isComboOpen = isOpen && !menuProps.empty;
+
+    if (isUnitsOpen || isComboOpen) {
+      return;
+    }
+
+    if (
+      (value.type === "unit" ||
+        (value.type === "intermediate" && isNumericString(value.value))) &&
+      value.unit !== undefined &&
+      (event.key === "ArrowUp" || event.key === "ArrowDown")
+    ) {
+      const inputValue =
+        value.type === "unit" ? value.value : Number(value.value.trim());
+
+      const meta = { altKey: event.altKey, shiftKey: event.shiftKey };
+      const hasMeta = meta.altKey || meta.shiftKey;
+
+      if (hasMeta) {
+        // @todo switch to using props.onChange instead of props.onChangeComplete
+        // this will require modifying input-popover.tsx
+        const newValue = {
+          type: "unit" as const,
+          value: handleNumericInputArrowKeys(inputValue, event),
+          unit: value.unit,
+        };
+
+        onChangeComplete({ value: newValue, ...meta, type: "delta" });
+      } else {
+        props.onChange({
+          type: "unit",
+          value: handleNumericInputArrowKeys(inputValue, event),
+          unit: value.unit,
+        });
+      }
+      event.preventDefault();
+    }
+  };
+
+  const handleMetaEnter = (event: KeyboardEvent<HTMLInputElement>) => {
+    if (
+      isUnitsOpen ||
+      (isOpen && !menuProps.empty && highlightedIndex !== -1)
+    ) {
+      return;
+    }
+
+    const meta = { altKey: event.altKey, shiftKey: event.shiftKey };
+
+    if (event.key === "Enter") {
+      onChangeComplete({ type: "enter", value, ...meta });
+    }
+  };
+
   const inputPropsHandleKeyDown = composeEventHandlers(
-    inputProps.onKeyDown,
-    handleKeyDown
+    composeEventHandlers(handleUpDownNumeric, inputProps.onKeyDown, {
+      // Pass prevented events to the combobox (e.g., the Escape key doesn't work otherwise, as it's blocked by Radix)
+      checkForDefaultPrevented: false,
+    }),
+    handleMetaEnter
   );
 
   return (

packages/design-system/src/components/combobox.tsx

@@ -255,6 +255,9 @@ type UseComboboxProps<Item> = Omit<UseDownshiftComboboxProps<Item>, "items"> & {
 
 export const comboboxStateChangeTypes = useDownshiftCombobox.stateChangeTypes;
 
+const isNumericString = (input: string) =>
+  String(input).trim().length !== 0 && Number.isNaN(Number(input)) === false;
+
 export const useCombobox = <Item,>({
   getItems,
   value,
@@ -281,7 +284,20 @@ export const useCombobox = <Item,>({
     selectedItem: selectedItem ?? null, // Prevent downshift warning about switching controlled mode
     isOpen,
 
-    onIsOpenChange({ isOpen, inputValue }) {
+    onIsOpenChange(state) {
+      const { type, isOpen, inputValue } = state;
+
+      // Don't open the combobox if the input is a number and the user is using the arrow keys.
+      // This prevents the combobox from opening when the user is trying to increment or decrement a number.
+      if (
+        (type === comboboxStateChangeTypes.InputKeyDownArrowDown ||
+          type === comboboxStateChangeTypes.InputKeyDownArrowUp) &&
+        inputValue !== undefined &&
+        isNumericString(inputValue)
+      ) {
+        return;
+      }
+
       if (isOpen) {
         itemsCache.current = getItems();
         const matchedItems = match(
@@ -303,11 +319,16 @@ export const useCombobox = <Item,>({
     stateReducer,
     itemToString,
     inputValue: value ? itemToString(value) : "",
-    onInputValueChange({ inputValue, type }) {
+    onInputValueChange(state) {
+      const { inputValue, type } = state;
       if (type === comboboxStateChangeTypes.InputChange) {
-        setMatchedItems(
-          match(inputValue ?? "", itemsCache.current, itemToString)
+        const matchedItems = match(
+          inputValue ?? "",
+          itemsCache.current,
+          itemToString
         );
+        setIsOpen(matchedItems.length > 0);
+        setMatchedItems(matchedItems);
       }
     },
     onSelectedItemChange({ selectedItem, type }) {