Skip to content

Commit

Permalink
fix: popover-based focus behaviour (#2854)
Browse files Browse the repository at this point in the history
* fix(autocomplete): autocomplete focus behaviour

* feat(autocomplete): add test case for catching blur cases

* refactor(autocomplete): use isOpen instead

* feat(autocomplete): add "should focus when clicking autocomplete" test case

* feat(autocomplete): add should set the input after selection

* fix(autocomplete): remove shouldUseVirtualFocus

* fix(autocomplete): uncomment blur logic

* refactor(autocomplete): remove state as it is in getPopoverProps

* refactor(autocomplete): remove unnecessary blur

* refactor(select): remove unncessary props

* fix(popover): use domRef instead

* fix(popover): revise isNonModal and isDismissable

* fix(popover): use dialogRef back

* fix(popover): rollback

* fix(autocomplete): onFocus logic

* feat(popover): set disableFocusManagement to overlay

* feat(modal): set disableFocusManagement to overlay

* fix(autocomplete): set disableFocusManagement for autocomplete

* feat(popover): include disableFocusManagement prop

* refactor(autocomplete): revise type in selectorButton

* fix(autocomplete): revise focus logic

* feat(autocomplete): add internal focus state and add shouldCloseOnInteractOutside

* feat(autocomplete): handle selectedItem change

* feat(autocomplete): add clear button test

* feat(changeset): add changeset

* refactor(components): use the original order

* refactor(autocomplete): add more comments

* fix(autocomplete): revise focus behaviours

* refactor(autocomplete): rename to listbox

* chore(popover): remove disableFocusManagement from popover

* chore(autocomplete): remove disableFocusManagement from autocomplete

* chore(changeset): add issue number

* fix(popover): don't set default value to transformOrigin

* fix(autocomplete): revise shouldCloseOnInteractOutside logic

* feat(autocomplete): should close listbox by clicking another autocomplete

* fix(popover): add disableFocusManagement to overlay

* refactor(autocomplete): revise comments and refactor shouldCloseOnInteractOutside

* feat(changeset): add issue number

* fix(autocomplete): merge with selectorButtonProps.onClick

* refactor(autocomplete): remove extra line

* refactor(autocomplete): revise comment

* feat(select): add shouldCloseOnInteractOutside

* feat(dropdown): add shouldCloseOnInteractOutside

* feat(date-picker): add shouldCloseOnInteractOutside

* feat(changeset): add dropdown and date-picker

* fix(popover): revise shouldCloseOnInteractOutside

* feat(date-picker): integrate with ariaShouldCloseOnInteractOutside

* feat(select): integrate with ariaShouldCloseOnInteractOutside

* feat(dropdown): integrate with ariaShouldCloseOnInteractOutside

* feat(popover): integrate with ariaShouldCloseOnInteractOutside

* feat(aria-utils): ariaShouldCloseOnInteractOutside

* chore(deps): update pnpm-lock.yaml

* feat(autocomplete): integrate with ariaShouldCloseOnInteractOutside

* feat(aria-utils): handle setShouldFocus logic

* feat(changeset): add @nextui-org/aria-utils

* chore(autocomplete): put the test into correct group

* feat(select): should close listbox by clicking another select

* feat(dropdown): should close listbox by clicking another dropdown

* feat(popover): should close listbox by clicking another popover

* feat(date-picker): should close listbox by clicking another datepicker

* chore(changeset): add issue numbers and revise changeset message

* refactor(autocomplete): change to useRef instead

* refactor(autocomplete): change to useRef instead

* refactor(aria-utils): revise comments and format code

* chore(changeset): add issue number

* chore: take popoverProps.shouldCloseOnInteractOutside first

* refactor(autocomplete): remove unnecessary logic

* refactor(autocomplete): focus management logic
  • Loading branch information
wingkwong authored May 24, 2024
1 parent 540aa21 commit 3b14c21
Show file tree
Hide file tree
Showing 23 changed files with 616 additions and 52 deletions.
11 changes: 11 additions & 0 deletions .changeset/good-crabs-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@nextui-org/autocomplete": patch
"@nextui-org/modal": patch
"@nextui-org/popover": patch
"@nextui-org/dropdown": patch
"@nextui-org/select": patch
"@nextui-org/date-picker": patch
"@nextui-org/aria-utils": patch
---

Revise popover-based focus behaviours (#2849, #2834, #2779, #2962, #2872, #2974, #1920, #1287, #3060)
271 changes: 264 additions & 7 deletions packages/components/autocomplete/__tests__/autocomplete.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,136 @@ describe("Autocomplete", () => {
expect(() => wrapper.unmount()).not.toThrow();
});

it("should close dropdown when clicking outside autocomplete", async () => {
it("should focus when clicking autocomplete", async () => {
const wrapper = render(
<Autocomplete aria-label="Favorite Animal" data-testid="autocomplete" label="Favorite Animal">
<AutocompleteItem key="penguin" value="penguin">
Penguin
</AutocompleteItem>
<AutocompleteItem key="zebra" value="zebra">
Zebra
</AutocompleteItem>
<AutocompleteItem key="shark" value="shark">
Shark
</AutocompleteItem>
</Autocomplete>,
);

const autocomplete = wrapper.getByTestId("autocomplete");

// open the select listbox
await act(async () => {
await userEvent.click(autocomplete);
});

// assert that the autocomplete listbox is open
expect(autocomplete).toHaveAttribute("aria-expanded", "true");

// assert that input is focused
expect(autocomplete).toHaveFocus();
});

it("should clear value after clicking clear button", async () => {
const wrapper = render(
<Autocomplete aria-label="Favorite Animal" data-testid="autocomplete" label="Favorite Animal">
<AutocompleteItem key="penguin" value="penguin">
Penguin
</AutocompleteItem>
<AutocompleteItem key="zebra" value="zebra">
Zebra
</AutocompleteItem>
<AutocompleteItem key="shark" value="shark">
Shark
</AutocompleteItem>
</Autocomplete>,
);

const autocomplete = wrapper.getByTestId("autocomplete");

// open the select listbox
await act(async () => {
await userEvent.click(autocomplete);
});

// assert that the autocomplete listbox is open
expect(autocomplete).toHaveAttribute("aria-expanded", "true");

let options = wrapper.getAllByRole("option");

// select the target item
await act(async () => {
await userEvent.click(options[0]);
});

const {container} = wrapper;

const clearButton = container.querySelector(
"[data-slot='inner-wrapper'] button:nth-of-type(1)",
)!;

expect(clearButton).not.toBeNull();

// select the target item
await act(async () => {
await userEvent.click(clearButton);
});

// assert that the input has empty value
expect(autocomplete).toHaveValue("");

// assert that input is focused
expect(autocomplete).toHaveFocus();
});

it("should open and close listbox by clicking selector button", async () => {
const wrapper = render(
<Autocomplete aria-label="Favorite Animal" data-testid="autocomplete" label="Favorite Animal">
<AutocompleteItem key="penguin" value="penguin">
Penguin
</AutocompleteItem>
<AutocompleteItem key="zebra" value="zebra">
Zebra
</AutocompleteItem>
<AutocompleteItem key="shark" value="shark">
Shark
</AutocompleteItem>
</Autocomplete>,
);

const {container} = wrapper;

const selectorButton = container.querySelector(
"[data-slot='inner-wrapper'] button:nth-of-type(2)",
)!;

expect(selectorButton).not.toBeNull();

const autocomplete = wrapper.getByTestId("autocomplete");

// open the select listbox by clicking selector button
await act(async () => {
await userEvent.click(selectorButton);
});

// assert that the autocomplete listbox is open
expect(autocomplete).toHaveAttribute("aria-expanded", "true");

// assert that input is focused
expect(autocomplete).toHaveFocus();

// close the select listbox by clicking selector button again
await act(async () => {
await userEvent.click(selectorButton);
});

// assert that the autocomplete listbox is closed
expect(autocomplete).toHaveAttribute("aria-expanded", "false");

// assert that input is still focused
expect(autocomplete).toHaveFocus();
});

it("should close listbox when clicking outside autocomplete", async () => {
const wrapper = render(
<Autocomplete
aria-label="Favorite Animal"
Expand All @@ -161,12 +290,12 @@ describe("Autocomplete", () => {

const autocomplete = wrapper.getByTestId("close-when-clicking-outside-test");

// open the select dropdown
// open the select listbox
await act(async () => {
await userEvent.click(autocomplete);
});

// assert that the autocomplete dropdown is open
// assert that the autocomplete listbox is open
expect(autocomplete).toHaveAttribute("aria-expanded", "true");

// click outside the autocomplete component
Expand All @@ -176,9 +305,12 @@ describe("Autocomplete", () => {

// assert that the autocomplete is closed
expect(autocomplete).toHaveAttribute("aria-expanded", "false");

// assert that input is not focused
expect(autocomplete).not.toHaveFocus();
});

it("should close dropdown when clicking outside autocomplete with modal open", async () => {
it("should close listbox when clicking outside autocomplete with modal open", async () => {
const wrapper = render(
<Modal isOpen>
<ModalContent>
Expand Down Expand Up @@ -207,21 +339,146 @@ describe("Autocomplete", () => {

const autocomplete = wrapper.getByTestId("close-when-clicking-outside-test");

// open the autocomplete dropdown
// open the autocomplete listbox
await act(async () => {
await userEvent.click(autocomplete);
});

// assert that the autocomplete dropdown is open
// assert that the autocomplete listbox is open
expect(autocomplete).toHaveAttribute("aria-expanded", "true");

// click outside the autocomplete component
await act(async () => {
await userEvent.click(document.body);
});

// assert that the autocomplete dropdown is closed
// assert that the autocomplete listbox is closed
expect(autocomplete).toHaveAttribute("aria-expanded", "false");

// assert that input is not focused
expect(autocomplete).not.toHaveFocus();
});

it("should set the input after selection", async () => {
const wrapper = render(
<Autocomplete aria-label="Favorite Animal" data-testid="autocomplete" label="Favorite Animal">
<AutocompleteItem key="penguin" value="penguin">
Penguin
</AutocompleteItem>
<AutocompleteItem key="zebra" value="zebra">
Zebra
</AutocompleteItem>
<AutocompleteItem key="shark" value="shark">
Shark
</AutocompleteItem>
</Autocomplete>,
);

const autocomplete = wrapper.getByTestId("autocomplete");

// open the listbox
await act(async () => {
await userEvent.click(autocomplete);
});

// assert that the autocomplete listbox is open
expect(autocomplete).toHaveAttribute("aria-expanded", "true");

// assert that input is focused
expect(autocomplete).toHaveFocus();

let options = wrapper.getAllByRole("option");

expect(options.length).toBe(3);

// select the target item
await act(async () => {
await userEvent.click(options[0]);
});

// assert that the input has target selection
expect(autocomplete).toHaveValue("Penguin");
});

it("should close listbox by clicking another autocomplete", async () => {
const wrapper = render(
<>
<Autocomplete
aria-label="Favorite Animal"
data-testid="autocomplete"
label="Favorite Animal"
>
<AutocompleteItem key="penguin" value="penguin">
Penguin
</AutocompleteItem>
<AutocompleteItem key="zebra" value="zebra">
Zebra
</AutocompleteItem>
<AutocompleteItem key="shark" value="shark">
Shark
</AutocompleteItem>
</Autocomplete>
<Autocomplete
aria-label="Favorite Animal"
data-testid="autocomplete2"
label="Favorite Animal"
>
<AutocompleteItem key="penguin" value="penguin">
Penguin
</AutocompleteItem>
<AutocompleteItem key="zebra" value="zebra">
Zebra
</AutocompleteItem>
<AutocompleteItem key="shark" value="shark">
Shark
</AutocompleteItem>
</Autocomplete>
</>,
);

const {container} = wrapper;

const autocomplete = wrapper.getByTestId("autocomplete");

const autocomplete2 = wrapper.getByTestId("autocomplete2");

const innerWrappers = container.querySelectorAll("[data-slot='inner-wrapper']");

const selectorButton = innerWrappers[0].querySelector("button:nth-of-type(2)")!;

const selectorButton2 = innerWrappers[1].querySelector("button:nth-of-type(2)")!;

expect(selectorButton).not.toBeNull();

expect(selectorButton2).not.toBeNull();

// open the select listbox by clicking selector button in the first autocomplete
await act(async () => {
await userEvent.click(selectorButton);
});

// assert that the first autocomplete listbox is open
expect(autocomplete).toHaveAttribute("aria-expanded", "true");

// assert that input is focused
expect(autocomplete).toHaveFocus();

// close the select listbox by clicking the second autocomplete
await act(async () => {
await userEvent.click(selectorButton2);
});

// assert that the first autocomplete listbox is closed
expect(autocomplete).toHaveAttribute("aria-expanded", "false");

// assert that the second autocomplete listbox is open
expect(autocomplete2).toHaveAttribute("aria-expanded", "true");

// assert that the first autocomplete is not focused
expect(autocomplete).not.toHaveFocus();

// assert that the second autocomplete is focused
expect(autocomplete2).toHaveFocus();
});

describe("validation", () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/components/autocomplete/src/autocomplete.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ interface Props<T> extends UseAutocompleteProps<T> {}
function Autocomplete<T extends object>(props: Props<T>, ref: ForwardedRef<HTMLInputElement>) {
const {
Component,
state,
isOpen,
disableAnimation,
selectorIcon = <ChevronDownIcon />,
Expand All @@ -33,7 +32,7 @@ function Autocomplete<T extends object>(props: Props<T>, ref: ForwardedRef<HTMLI
} = useAutocomplete<T>({...props, ref});

const popoverContent = isOpen ? (
<FreeSoloPopover {...getPopoverProps()} state={state}>
<FreeSoloPopover {...getPopoverProps()}>
<ScrollShadow {...getListBoxWrapperProps()}>
<Listbox {...getListBoxProps()} />
</ScrollShadow>
Expand Down
Loading

0 comments on commit 3b14c21

Please sign in to comment.