Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[combobox] Fix binding value undesirely expands combobox #1

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions packages/combobox/examples/simulated-change.example.js
Original file line number Diff line number Diff line change
@@ -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" };
63 changes: 25 additions & 38 deletions packages/combobox/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,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";
Expand Down Expand Up @@ -106,7 +107,7 @@ const stateChart: StateChart = {
[BLUR]: IDLE,
[CLEAR]: IDLE,
[CHANGE]: SUGGESTING,
[INITIAL_CHANGE]: IDLE,
[SIMULATED_CHANGE]: IDLE,
[FOCUS]: SUGGESTING,
[NAVIGATE]: NAVIGATING,
[OPEN_WITH_BUTTON]: SUGGESTING,
Expand Down Expand Up @@ -159,7 +160,7 @@ const reducer: Reducer = (data: StateData, event: MachineEvent) => {
const nextState = { ...data, lastEventType: event.type };
switch (event.type) {
case CHANGE:
case INITIAL_CHANGE:
case SIMULATED_CHANGE:
return {
...nextState,
navigationValue: null,
Expand Down Expand Up @@ -427,11 +428,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 },
Expand Down Expand Up @@ -470,16 +468,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(() => {
Expand All @@ -494,13 +489,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>) {
const { value } = event.target;
inputValueChangedRef.current = true;
if (!isControlled) {
handleValueChange(value);
}
Expand Down Expand Up @@ -626,9 +625,8 @@ export const ComboboxPopover = React.forwardRef(function ComboboxPopover(
},
forwardedRef
) {
const { popoverRef, inputRef, isExpanded, state } = React.useContext(
ComboboxContext
);
const { popoverRef, inputRef, isExpanded, state } =
React.useContext(ComboboxContext);
const ref = useComposedRefs(popoverRef, forwardedRef);
const handleKeyDown = useKeyDown();
const handleBlur = useBlur();
Expand Down Expand Up @@ -912,13 +910,8 @@ export const ComboboxButton = React.forwardRef(function ComboboxButton(
{ as: Comp = "button", onClick, onKeyDown, ...props },
forwardedRef
) {
const {
transition,
state,
buttonRef,
listboxId,
isExpanded,
} = React.useContext(ComboboxContext);
const { transition, state, buttonRef, listboxId, isExpanded } =
React.useContext(ComboboxContext);
const ref = useComposedRefs(buttonRef, forwardedRef);

const handleKeyDown = useKeyDown();
Expand Down Expand Up @@ -1130,13 +1123,8 @@ function useKeyDown() {
}

function useBlur() {
const {
state,
transition,
popoverRef,
inputRef,
buttonRef,
} = React.useContext(ComboboxContext);
const { state, transition, popoverRef, inputRef, buttonRef } =
React.useContext(ComboboxContext);

return function handleBlur(event: React.FocusEvent) {
let popover = popoverRef.current;
Expand Down Expand Up @@ -1249,9 +1237,8 @@ export function escapeRegexp(str: string) {
* @see Docs https://reach.tech/combobox#usecomboboxcontext
*/
export function useComboboxContext(): ComboboxContextValue {
let { isExpanded, comboboxId, data, state } = React.useContext(
ComboboxContext
);
let { isExpanded, comboboxId, data, state } =
React.useContext(ComboboxContext);
let { navigationValue } = data;
return React.useMemo(
() => ({
Expand Down Expand Up @@ -1331,7 +1318,7 @@ type State = "IDLE" | "SUGGESTING" | "NAVIGATING" | "INTERACTING";
type MachineEventType =
| "CLEAR"
| "CHANGE"
| "INITIAL_CHANGE"
| "SIMULATED_CHANGE"
| "NAVIGATE"
| "SELECT_WITH_KEYBOARD"
| "SELECT_WITH_CLICK"
Expand Down Expand Up @@ -1363,7 +1350,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" }
Expand Down