Merge branch 'fix/binding-value-undesirely-expands-combobox' of https…

…://github.com/dlacaille/reach-ui into dlacaille-fix/binding-value-undesirely-expands-combobox
reach · Oct 17, 2021 · 8bd5b5c · 8bd5b5c
2 parents 63b4299 + 3112a81
commit 8bd5b5c
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 18 deletions.
diff --git a/packages/combobox/examples/simulated-change.example.js b/packages/combobox/examples/simulated-change.example.js
@@ -0,0 +1,91 @@
+import * as React from "react";
+import {
+  Combobox,
+  ComboboxInput,
+  ComboboxList,
+  ComboboxOption,
+  ComboboxPopover,
+} from "@reach/combobox";
+import { useCityMatch } from "./utils";
+import "@reach/combobox/styles.css";
+
+let name = "Simulated Change";
+
+function Example() {
+  let [term, setTerm] = React.useState("Detroit");
+  let [selection, setSelection] = React.useState("");
+  let results = useCityMatch(term);
+  let ref = React.useRef();
+
+  const handleChange = (event) => {
+    setTerm(event.target.value);
+  };
+
+  const handleSelect = (value) => {
+    setSelection(value);
+    setTerm("");
+  };
+
+  const handleSimulateChange = () => {
+    setTerm("New York");
+  };
+
+  return (
+    <div>
+      <h2>Clientside Search</h2>
+      <p>
+        This example tests that changes to the controlled value of Combobox
+        don't expand it unless we are actually typing. The initial value and
+        programmatically set value here shouldn't open the Popover.
+      </p>
+      <p>Selection: {selection}</p>
+      <p>Term: {term}</p>
+      <p>
+        <button onClick={handleSimulateChange}>
+          Set value programmatically
+        </button>
+      </p>
+      <Combobox onSelect={handleSelect} aria-label="choose a city">
+        <ComboboxInput
+          ref={ref}
+          value={term}
+          onChange={handleChange}
+          autocomplete={false}
+          style={{ width: 400 }}
+        />
+        {results && (
+          <ComboboxPopover>
+            {results.length === 0 && (
+              <p>
+                No Results{" "}
+                <button
+                  onClick={() => {
+                    setTerm("");
+                    ref.current.focus();
+                  }}
+                >
+                  clear
+                </button>
+              </p>
+            )}
+            <ComboboxList>
+              {results.slice(0, 10).map((result, index) => (
+                <ComboboxOption
+                  key={index}
+                  value={`${result.city}, ${result.state}`}
+                />
+              ))}
+            </ComboboxList>
+            <p>
+              <a href="/new">Add a record</a>
+            </p>
+          </ComboboxPopover>
+        )}
+      </Combobox>
+    </div>
+  );
+}
+
+Example.story = { name };
+export const Comp = Example;
+export default { title: "Combobox" };
diff --git a/packages/combobox/src/index.tsx b/packages/combobox/src/index.tsx
@@ -70,10 +70,11 @@ const CLEAR = "CLEAR";
 // User is typing
 const CHANGE = "CHANGE";
 
-// Initial input value change handler for syncing user state with state machine
-// Prevents initial change from sending the user to the NAVIGATING state
+// Any input change that is not triggered by an actual onChange event.
+// For example an initial value or a controlled value that was changed.
+// Prevents sending the user to the NAVIGATING state
 // https://github.com/reach/reach-ui/issues/464
-const INITIAL_CHANGE = "INITIAL_CHANGE";
+const SIMULATED_CHANGE = "SIMULATED_CHANGE";
 
 // User is navigating w/ the keyboard
 const NAVIGATE = "NAVIGATE";
@@ -107,7 +108,7 @@ const stateChart: StateChart = {
         [BLUR]: IDLE,
         [CLEAR]: IDLE,
         [CHANGE]: SUGGESTING,
-        [INITIAL_CHANGE]: IDLE,
+        [SIMULATED_CHANGE]: IDLE,
         [FOCUS]: SUGGESTING,
         [NAVIGATE]: NAVIGATING,
         [OPEN_WITH_BUTTON]: SUGGESTING,
@@ -160,7 +161,7 @@ const reducer: Reducer = (data: StateData, event: MachineEvent) => {
   let nextState = { ...data, lastEventType: event.type };
   switch (event.type) {
     case CHANGE:
-    case INITIAL_CHANGE:
+    case SIMULATED_CHANGE:
       return {
         ...nextState,
         navigationValue: null,
@@ -428,11 +429,8 @@ export const ComboboxInput = React.forwardRef(function ComboboxInput(
   forwardedRef
 ) {
   // https://github.com/reach/reach-ui/issues/464
-  let { current: initialControlledValue } = React.useRef(controlledValue);
-  let controlledValueChangedRef = React.useRef(false);
-  useUpdateEffect(() => {
-    controlledValueChangedRef.current = true;
-  }, [controlledValue]);
+  // https://github.com/reach/reach-ui/issues/755
+  let inputValueChangedRef = React.useRef(false);
 
   let {
     data: { navigationValue, value, lastEventType },
@@ -471,16 +469,13 @@ export const ComboboxInput = React.forwardRef(function ComboboxInput(
     (value: ComboboxValue) => {
       if (value.trim() === "") {
         transition(CLEAR);
-      } else if (
-        value === initialControlledValue &&
-        !controlledValueChangedRef.current
-      ) {
-        transition(INITIAL_CHANGE, { value });
+      } else if (!inputValueChangedRef.current) {
+        transition(SIMULATED_CHANGE, { value });
       } else {
         transition(CHANGE, { value });
       }
     },
-    [initialControlledValue, transition]
+    [transition]
   );
 
   React.useEffect(() => {
@@ -495,13 +490,17 @@ export const ComboboxInput = React.forwardRef(function ComboboxInput(
     ) {
       handleValueChange(controlledValue!);
     }
+    // After we handled the changed value, we need to make sure the next
+    // controlled change won't trigger a CHANGE event. (instead of a SIMULATED_CHANGE)
+    inputValueChangedRef.current = false;
   }, [controlledValue, handleValueChange, isControlled, value]);
 
   // [*]... and when controlled, we don't trigger handleValueChange as the
   // user types, instead the developer controls it with the normal input
   // onChange prop
   function handleChange(event: React.ChangeEvent<HTMLInputElement>) {
     let { value } = event.target;
+    inputValueChangedRef.current = true;
     if (!isControlled) {
       handleValueChange(value);
     }
@@ -1331,7 +1330,7 @@ type State = "IDLE" | "SUGGESTING" | "NAVIGATING" | "INTERACTING";
 type MachineEventType =
   | "CLEAR"
   | "CHANGE"
-  | "INITIAL_CHANGE"
+  | "SIMULATED_CHANGE"
   | "NAVIGATE"
   | "SELECT_WITH_KEYBOARD"
   | "SELECT_WITH_CLICK"
@@ -1363,7 +1362,7 @@ interface StateData {
 type MachineEvent =
   | { type: "BLUR" }
   | { type: "CHANGE"; value: ComboboxValue }
-  | { type: "INITIAL_CHANGE"; value: ComboboxValue }
+  | { type: "SIMULATED_CHANGE"; value: ComboboxValue }
   | { type: "CLEAR" }
   | { type: "CLOSE_WITH_BUTTON" }
   | { type: "ESCAPE" }