Skip to content

Commit

Permalink
Fix Combobox with autoSelect + autoComplete="both" props (ariak…
Browse files Browse the repository at this point in the history
  • Loading branch information
diegohaz authored and natanelia committed May 30, 2022
1 parent 7e5823a commit eb5d5dd
Show file tree
Hide file tree
Showing 11 changed files with 295 additions and 17 deletions.
5 changes: 5 additions & 0 deletions .changeset/hot-plums-tie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ariakit-utils": minor
---

Add `usePreviousValue` function to `ariakit-utils/hooks`. ([#1219](https://github.com/ariakit/ariakit/pull/1219))
5 changes: 5 additions & 0 deletions .changeset/polite-points-act.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ariakit": patch
---

Fix `Combobox` with `autoSelect` and `autoComplete="both"` props setting an incorrect value when there are no matches. ([#1219](https://github.com/ariakit/ariakit/pull/1219))
11 changes: 11 additions & 0 deletions packages/ariakit-utils/src/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ export function useLiveRef<T>(value: T) {
return ref;
}

/**
* Keeps the reference of the previous value to be used in the render phase.
*/
export function usePreviousValue<T>(value: T) {
const [previousValue, setPreviousValue] = useState(value);
if (value !== previousValue) {
setPreviousValue(value);
}
return previousValue;
}

/**
* Creates a memoized callback function that is constantly updated with the
* incoming callback.
Expand Down
73 changes: 73 additions & 0 deletions packages/ariakit/src/combobox/__examples__/combobox-group/food.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
export default [
{ name: "Apple", type: "Fruits" },
{ name: "Avocado", type: "Fruits" },
{ name: "Banana", type: "Fruits" },
{ name: "Cherry", type: "Fruits" },
{ name: "Coconut", type: "Fruits" },
{ name: "Grape", type: "Fruits" },
{ name: "Kiwi", type: "Fruits" },
{ name: "Lemon", type: "Fruits" },
{ name: "Lime", type: "Fruits" },
{ name: "Mango", type: "Fruits" },
{ name: "Orange", type: "Fruits" },
{ name: "Papaya", type: "Fruits" },
{ name: "Pear", type: "Fruits" },
{ name: "Pineapple", type: "Fruits" },
{ name: "Pomegranate", type: "Fruits" },
{ name: "Strawberry", type: "Fruits" },
{ name: "Watermelon", type: "Fruits" },

{ name: "Bacon", type: "Meat" },
{ name: "Beef", type: "Meat" },
{ name: "Chicken", type: "Meat" },
{ name: "Fish", type: "Meat" },
{ name: "Ham", type: "Meat" },
{ name: "Lamb", type: "Meat" },
{ name: "Pork", type: "Meat" },
{ name: "Sausage", type: "Meat" },
{ name: "Turkey", type: "Meat" },

{ name: "Bread", type: "Bakery" },
{ name: "Cake", type: "Bakery" },
{ name: "Cookie", type: "Bakery" },
{ name: "Croissant", type: "Bakery" },
{ name: "Donut", type: "Bakery" },
{ name: "Eclair", type: "Bakery" },
{ name: "Fondant", type: "Bakery" },
{ name: "Gelato", type: "Bakery" },
{ name: "Muffin", type: "Bakery" },
{ name: "Pie", type: "Bakery" },
{ name: "Pudding", type: "Bakery" },
{ name: "Scone", type: "Bakery" },
{ name: "Souffle", type: "Bakery" },
{ name: "Tart", type: "Bakery" },
{ name: "Waffle", type: "Bakery" },

{ name: "Butter", type: "Dairy" },
{ name: "Cheese", type: "Dairy" },
{ name: "Cream", type: "Dairy" },
{ name: "Egg", type: "Dairy" },
{ name: "Milk", type: "Dairy" },
{ name: "Sour cream", type: "Dairy" },
{ name: "Yogurt", type: "Dairy" },

{ name: "Beer", type: "Beverages" },
{ name: "Brandy", type: "Beverages" },
{ name: "Cider", type: "Beverages" },
{ name: "Coffee", type: "Beverages" },
{ name: "Cognac", type: "Beverages" },
{ name: "Gin", type: "Beverages" },
{ name: "Juice", type: "Beverages" },
{ name: "Liqueur", type: "Beverages" },
{ name: "Milkshake", type: "Beverages" },
{ name: "Rum", type: "Beverages" },
{ name: "Sake", type: "Beverages" },
{ name: "Sherry", type: "Beverages" },
{ name: "Soda", type: "Beverages" },
{ name: "Tea", type: "Beverages" },
{ name: "Tequila", type: "Beverages" },
{ name: "Vodka", type: "Beverages" },
{ name: "Water", type: "Beverages" },
{ name: "Whiskey", type: "Beverages" },
{ name: "Wine", type: "Beverages" },
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { Fragment, useMemo } from "react";
import {
Combobox,
ComboboxGroup,
ComboboxGroupLabel,
ComboboxItem,
ComboboxPopover,
ComboboxSeparator,
useComboboxState,
} from "ariakit/combobox";
import groupBy from "lodash/groupBy";
import food from "./food";
import "./style.css";

const list = food.map((item) => item.name);

export default function Example() {
const combobox = useComboboxState({ gutter: 8, sameWidth: true, list });

// Transform combobox.matches into groups of objects.
const matches = useMemo(() => {
const items = combobox.matches
.map((value) => food.find((item) => item.name === value)!)
.filter(Boolean);
return Object.entries(groupBy(items, "type"));
}, [combobox.matches]);

return (
<div>
<label className="label">
Your favorite food
<Combobox
state={combobox}
placeholder="e.g., Apple"
className="combobox"
autoComplete="both"
autoSelect
/>
</label>
<ComboboxPopover state={combobox} className="popover">
{!!matches.length ? (
matches.map(([type, items], index, array) => (
<Fragment key={type}>
<ComboboxGroup className="group">
<ComboboxGroupLabel className="group-label">
{type}
</ComboboxGroupLabel>
{items.map((item) => (
<ComboboxItem
focusOnHover
key={item.name}
value={item.name}
className="combobox-item"
/>
))}
</ComboboxGroup>
{index < array.length - 1 && (
<ComboboxSeparator className="separator" />
)}
</Fragment>
))
) : (
<div className="no-results">No results found</div>
)}
</ComboboxPopover>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
@import url("../combobox/style.css");

.group-label {
@apply
text-sm
p-2
font-medium
opacity-60
cursor-default;
}

.separator {
border-width: 1px 0 0;
border-color: currentcolor;
@apply my-2 h-0 opacity-25;
}

.no-results {
@apply gap-2 p-2;
}
80 changes: 80 additions & 0 deletions packages/ariakit/src/combobox/__examples__/combobox-group/test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import {
click,
getByRole,
hover,
press,
render,
type,
} from "ariakit-test-utils";
import { axe } from "jest-axe";
import Example from ".";

const getCombobox = () => getByRole("combobox");
const getOption = (name: string) => getByRole("option", { name });

function getSelectionValue(element: Element) {
const input = element as HTMLInputElement;
const { selectionStart, selectionEnd } = input;
const selectionValue = input.value.slice(selectionStart!, selectionEnd!);
return selectionValue;
}

test("a11y", async () => {
const { container } = render(<Example />);
expect(await axe(container)).toHaveNoViolations();
});

test("auto select with inline autocomplete on typing", async () => {
render(<Example />);
await press.Tab();
await type("a");
expect(getCombobox()).toHaveValue("apple");
expect(getSelectionValue(getCombobox())).toBe("pple");
await press.ArrowDown();
expect(getCombobox()).toHaveValue("Avocado");
expect(getSelectionValue(getCombobox())).toBe("");
await press.ArrowUp();
expect(getCombobox()).toHaveValue("apple");
expect(getSelectionValue(getCombobox())).toBe("pple");
await type("e");
expect(getCombobox()).toHaveValue("ae");
expect(getSelectionValue(getCombobox())).toBe("");
await type("\b\bv");
expect(getCombobox()).toHaveValue("vodka");
expect(getSelectionValue(getCombobox())).toBe("odka");
});

test("auto select with inline autocomplete on arrow down", async () => {
render(<Example />);
await press.Tab();
await press.ArrowDown();
await press.ArrowDown();
expect(getCombobox()).toHaveValue("Apple");
expect(getSelectionValue(getCombobox())).toBe("Apple");
await press.ArrowDown();
expect(getCombobox()).toHaveValue("Avocado");
expect(getSelectionValue(getCombobox())).toBe("");
await press.ArrowUp();
expect(getCombobox()).toHaveValue("Apple");
});

test("blur input after autocomplete", async () => {
render(<Example />);
await press.Tab();
await type("a");
expect(getCombobox()).toHaveValue("apple");
await press.ArrowDown();
expect(getCombobox()).toHaveValue("Avocado");
await click(document.body);
await click(document.body);
expect(getCombobox()).toHaveValue("Avocado");
});

test("autocomplete on focus on hover", async () => {
render(<Example />);
await click(getCombobox());
await type("g");
expect(getCombobox()).toHaveValue("grape");
await hover(getOption("Gelato"));
expect(getCombobox()).toHaveValue("g");
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export default function Example() {
<ComboboxItem key={value} value={value} className="combobox-item" />
))
) : (
<div>No results found</div>
<div className="no-results">No results found</div>
)}
</ComboboxPopover>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
@import url("../combobox/style.css");

.no-results {
@apply gap-2 p-2;
}
28 changes: 13 additions & 15 deletions packages/ariakit/src/combobox/combobox-state.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { useEffect, useMemo, useState } from "react";
import { useMemo, useState } from "react";
import {
useControlledState,
useDeferredValue,
useLiveRef,
usePreviousValue,
useUpdateLayoutEffect,
} from "ariakit-utils/hooks";
import { normalizeString } from "ariakit-utils/misc";
Expand Down Expand Up @@ -98,24 +98,22 @@ export function useComboboxState({
includesBaseElement,
});
const popover = usePopoverState({ ...props, placement });
const [activeValue, setActiveValue] = useState<string | undefined>();
const compositeRef = useLiveRef(composite);
const prevActiveId = usePreviousValue(composite.activeId);
const prevMoves = usePreviousValue(composite.moves);
const [moved, setMoved] = useState(false);

// Always reset the active value when the active item changes.
useEffect(() => {
setActiveValue(undefined);
}, [composite.activeId]);
if (prevActiveId !== composite.activeId) {
setMoved(prevMoves !== composite.moves);
}

// Update the active value when the active item changes by moving (which
// usually happens when using the keyboard).
useEffect(() => {
const { items, activeId } = compositeRef.current;
if (!activeId) return;
const nextActiveValue = items.find(
(item) => item.id === activeId && item.value
const activeValue = useMemo(() => {
if (!moved) return undefined;
return composite.items.find(
(item) => item.id === composite.activeId && item.value
)?.value;
setActiveValue(nextActiveValue);
}, [composite.moves]);
}, [moved, composite.items, composite.activeId]);

const deferredValue = useDeferredValue(value);

Expand Down
16 changes: 15 additions & 1 deletion packages/ariakit/src/combobox/combobox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,21 @@ import {
createHook,
} from "ariakit-utils/system";
import { As, BooleanOrCallback, Props } from "ariakit-utils/types";
import { unstable_batchedUpdates } from "react-dom";
import { CompositeOptions, useComposite } from "../composite/composite";
import {
PopoverAnchorOptions,
usePopoverAnchor,
} from "../popover/popover-anchor";
import { ComboboxState } from "./combobox-state";

function batchUpdates(fn: () => void) {
if (unstable_batchedUpdates) {
return unstable_batchedUpdates(fn);
}
return fn();
}

function isFirstItemAutoSelected(
items: ComboboxState["items"],
activeValue: ComboboxState["activeValue"],
Expand Down Expand Up @@ -164,7 +172,13 @@ export const useCombobox = createHook<ComboboxOptions>(
// we want to automatically focus on the first suggestion. This effect
// will run both when value changes and when items change so we can also
// catch async items. We need to defer the focus to avoid scroll jumps.
const timeout = setTimeout(() => state.move(state.first()), 16);
const timeout = setTimeout(() => {
// Since we're updating the state inside the timeout, we need to
// manually batch the updates so they happen at the same time.
// Otherwise, state.moves and state.activeId may be out of sync. This
// only applies to React 17 and below.
batchUpdates(() => state.move(state.first()));
}, 16);
return () => clearTimeout(timeout);
}, [
valueUpdated,
Expand Down

0 comments on commit eb5d5dd

Please sign in to comment.