Skip to content

Commit

Permalink
Merge branch 'dlacaille-fix/binding-value-undesirely-expands-combobox…
Browse files Browse the repository at this point in the history 
…' into develop
  • Loading branch information
@chaance
chaance committed Oct 17, 2021
2 parents 63b4299 + d271a06 commit 04c2987
Show file tree
Hide file tree
Showing 6 changed files with 416 additions and 244 deletions.
10 changes: 5 additions & 5 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@
"@preconstruct/cli": "^2.1.0",
"@reach/router": "^1.3.4",
"@react-spring/web": "^9.2.3",
"@storybook/addon-actions": "^6.3.1",
"@storybook/addon-docs": "^6.3.1",
"@storybook/addon-links": "^6.3.1",
"@storybook/addon-actions": "^6.3.12",
"@storybook/addon-docs": "^6.3.12",
"@storybook/addon-links": "^6.3.12",
"@storybook/addon-postcss": "^2.0.0",
"@storybook/addons": "^6.3.1",
"@storybook/react": "^6.3.1",
"@storybook/addons": "^6.3.12",
"@storybook/react": "^6.3.12",
"@testing-library/dom": "^8.0.0",
"@testing-library/jest-dom": "^5.14.1",
"@testing-library/react": "^12.0.0",
Expand Down
108 changes: 92 additions & 16 deletions packages/combobox/__tests__/combobox.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import cities from "../examples/cities";

describe("<Combobox />", () => {
describe("rendering", () => {
it("renders as any HTML element", async () => {
it("renders as any HTML element", () => {
function MyCombobox() {
let [term, setTerm] = React.useState("");
let results = useCityMatch(term);
Expand Down Expand Up @@ -45,18 +45,18 @@ describe("<Combobox />", () => {
);
}

let { getByTestId, getAllByRole } = render(<MyCombobox />);
let { getByTestId, getByRole, getAllByRole } = render(<MyCombobox />);
expect(getByTestId("box").tagName).toBe("SPAN");
expect(getByTestId("input").tagName).toBe("TEXTAREA");
expect(getByRole("combobox").tagName).toBe("TEXTAREA");

// Type to show the list
await userEvent.type(getByTestId("input"), "e");
userEvent.type(getByRole("combobox"), "e");

expect(getByTestId("list").tagName).toBe("DIV");
expect(getByRole("listbox").tagName).toBe("DIV");
expect(getAllByRole("option")[0].tagName).toBe("DIV");
});

it("renders when using the useComboboxContext hook", async () => {
it("renders when using the useComboboxContext hook", () => {
function CustomComboboxInput(props: ComboboxInputProps) {
const { isExpanded } = useComboboxContext();
return (
Expand Down Expand Up @@ -85,14 +85,14 @@ describe("<Combobox />", () => {
);
}

let { getByTestId, getAllByRole } = render(<MyCombobox />);
let { getByRole, getAllByRole } = render(<MyCombobox />);

// Type to show the list

await userEvent.type(getByTestId("input"), "a");
userEvent.type(getByRole("combobox"), "a");
//jest.advanceTimersByTime(100);

expect(getByTestId("list")).toBeTruthy();
expect(getByRole("listbox")).toBeTruthy();
expect(getAllByRole("option")[0]).toBeTruthy();
});
});
Expand Down Expand Up @@ -195,17 +195,44 @@ describe("<Combobox />", () => {
});

describe("user events", () => {
it("should open a list on text entry", async () => {
it("should open a list on text entry", () => {
let optionToSelect = "Eagle Pass, Texas";
let { getByTestId, getByText } = render(<BasicCombobox />);
let { getByRole, getByText } = render(<BasicCombobox />);
let getByTextWithMarkup = withMarkup(getByText);
let input = getByTestId("input");
let input = getByRole("combobox");

await userEvent.type(input, "e");
userEvent.type(input, "e");

expect(getByTestId("list")).toBeInTheDocument();
expect(getByRole("listbox")).toBeInTheDocument();
expect(getByTextWithMarkup(optionToSelect)).toBeInTheDocument();
});

it("should *not* open a list when input value changes without text entry", () => {
let optionToSelect = "Eagle Pass, Texas";

function EaglePassSelector() {
let [term, setTerm] = React.useState("");
return (
<div>
<button
type="button"
onClick={() => {
setTerm(optionToSelect);
}}
>
Select Eagle Pass
</button>
<ControlledCombobox term={term} setTerm={setTerm} />
</div>
);
}

let { getByRole, queryByRole } = render(<EaglePassSelector />);

let button = getByRole("button");
userEvent.click(button);
expect(queryByRole("listbox")).toBeFalsy();
});
});
});

Expand All @@ -214,9 +241,9 @@ function BasicCombobox() {
let [term, setTerm] = React.useState("");
let results = useCityMatch(term);

const handleChange = (event: any) => {
function handleChange(event: any) {
setTerm(event.target.value);
};
}

return (
<div>
Expand Down Expand Up @@ -250,6 +277,55 @@ function BasicCombobox() {
);
}

function ControlledCombobox({
term,
setTerm,
}: {
term: string;
setTerm:
| ((term: string) => void)
| ((setter: (prevTerm: string) => string) => void);
}) {
let results = useCityMatch(term);

function handleChange(event: any) {
setTerm(event.target.value);
}

return (
<div>
<h2>Clientside Search</h2>
<Combobox id="holy-smokes">
<ComboboxInput
aria-label="cool search"
data-testid="input"
name="awesome"
onChange={handleChange}
value={term}
/>
{results ? (
<ComboboxPopover portal={false}>
{results.length === 0 ? (
<p>No results</p>
) : (
<ComboboxList data-testid="list">
{results.slice(0, 10).map((result, index) => (
<ComboboxOption
key={index}
value={`${result.city}, ${result.state}`}
/>
))}
</ComboboxList>
)}
</ComboboxPopover>
) : (
<span>No Results!</span>
)}
</Combobox>
</div>
);
}

function useCityMatch(term: string) {
return term.trim() === ""
? null
Expand Down
7 changes: 4 additions & 3 deletions packages/combobox/examples/index.story.js
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
export { Example as BasicTsTS } from "./basic-ts.example.tsx";
export { Example as BasicTs } from "./basic-ts.example.tsx";
export { Example as Basic } from "./basic.example.js";
export { Example as ControlledTsTS } from "./controlled-ts.example.tsx";
export { Example as ControlledTs } from "./controlled-ts.example.tsx";
export { Example as Controlled } from "./controlled.example.js";
export { Example as LotsOfElements } from "./lots-of-elements.example.js";
export { Example as NoPopover } from "./no-popover.example.js";
export { Example as OpenOnFocus } from "./open-on-focus.example.js";
export { Example as SimulatedChange } from "./simulated-change.example.js";
export { Example as TokenInput } from "./token-input.example.js";
export { Example as WithButton } from "./with-button.example.js";
export { Example as WithCustomSelectDataTS } from "./with-custom-select-data.example.tsx";
export { Example as WithCustomSelectDataTs } from "./with-custom-select-data.example.tsx";
export { Example as WithUsecomboboxcontextHookTS } from "./with-usecomboboxcontext-hook.example.tsx";

export default {
Expand Down
90 changes: 90 additions & 0 deletions packages/combobox/examples/simulated-change.example.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
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.storyName = name;
export { Example };
35 changes: 17 additions & 18 deletions packages/combobox/src/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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);
}
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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" }
Expand Down
Loading

0 comments on commit 04c2987

Please sign in to comment.