Skip to content

Commit

Permalink
Bugfix/prevent submit on enter in combobox (#2861)
Browse files Browse the repository at this point in the history
* Prevent "Enter" key in Combobox to...

* Added test for Enter not submitting form

* Added changeset

* Add test for un-selecting a selected option using "enter" on a Chip not submitting the form

* Do not prevent enter from submitting form if it should not perform another action (like selecting an option)

* Do not submit when the input field has a value

* Replace lots of sleep-calls with awaiting userEvents

* Create util function for looking up an option

* Fix bug where we couldn't un-select a custom option using "enter"

* Update test to account for the dropdown list closing when focus moves to the chips in the previous step

* Add possibility to slow down tests for debugging

* Fix issue with onFocus -> onToggleSelected(false), where clicking the open/close button didn't work

* Fix test

* Closing the FilteredOptionsList when focusing a Chip caused too many side effects, but we need to remove virtual focus at least. That way we won't double-click on enter.

* When no longer closing FilteredOptions on focusing a chip, we don't need to click the toggleIsOpen button to check that the option is un-selected

* Update .changeset/rotten-windows-cover.md

Co-authored-by: Ken <26967723+KenAJoh@users.noreply.github.com>

---------

Co-authored-by: Ken <26967723+KenAJoh@users.noreply.github.com>
  • Loading branch information
it-vegard and KenAJoh authored Apr 18, 2024
1 parent 2c14760 commit c2f3da8
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/rotten-windows-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@navikt/ds-react": patch
---

Combobox: Prevents "Enter" while Combobox is focused from submitting form.
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,17 @@ export const FilteredOptionsProvider = ({
[filteredOptionsUtils.getAddNewOptionId(id)]: allowNewValues
? toComboboxOption(value)
: undefined,
...customOptions.reduce(
(acc, customOption) => ({
...acc,
[filteredOptionsUtils.getOptionId(id, customOption.label)]:
customOption,
}),
{},
),
},
),
[allowNewValues, id, options, value],
[allowNewValues, customOptions, id, options, value],
);

useClientLayoutEffect(() => {
Expand Down
5 changes: 5 additions & 0 deletions @navikt/core/react/src/form/combobox/Input/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
removeSelectedOption(lastSelectedOption);
}
}
} else if (e.key === "Enter" || e.key === "Accept") {
if (activeDecendantId || value) {
e.preventDefault();
}
} else if (e.key === "ArrowDown") {
// Check that cursor position is at the end of the input field,
// so we don't interfere with text editing
Expand Down Expand Up @@ -182,6 +186,7 @@ const Input = forwardRef<HTMLInputElement, InputProps>(
{...omit(inputProps, ["aria-invalid"])}
ref={ref}
value={value}
onBlur={() => virtualFocus.moveFocusToTop()}
onChange={onChangeHandler}
type="text"
role="combobox"
Expand Down
68 changes: 68 additions & 0 deletions @navikt/core/react/src/form/combobox/combobox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -624,3 +624,71 @@ export const TestHoverAndFocusSwitching: StoryObject = {
);
},
};

export const TestEnterNotSubmittingForm: StoryObj<{
onSubmit: ReturnType<typeof fn>;
}> = {
args: {
onSubmit: fn(),
},
render: ({ onSubmit }) => {
return (
<form action="https://www.aksel.nav.no" method="get" onSubmit={onSubmit}>
<UNSAFE_Combobox
options={options}
label="Hva er dine favorittfrukter?"
isMultiSelect
allowNewValues
/>
</form>
);
},
play: async ({ canvasElement, args }) => {
args.onSubmit.mockClear();
const canvas = within(canvasElement);
const waitTime = 0; // Change for debugging

await sleep(waitTime);

const getInput = () =>
canvas.getByRole("combobox", {
name: "Hva er dine favorittfrukter?",
});

const getOption = (name: string, selected: boolean) =>
canvas.getByRole("option", { name, selected });
await userEvent.click(getInput(), { delay: waitTime });

await userEvent.keyboard("{ArrowDown}", { delay: waitTime });
expect(getInput().getAttribute("aria-activedescendant")).toBe(
getOption("banana", false).getAttribute("id"),
);

await userEvent.keyboard("{Enter}", { delay: waitTime });
expect(args.onSubmit).not.toHaveBeenCalled();
expect(getOption("banana", true)).toBeVisible();

await userEvent.keyboard("{Shift>}{Tab}", { delay: waitTime });

await userEvent.keyboard("{Enter}", { delay: waitTime });
expect(args.onSubmit).not.toHaveBeenCalled();

await userEvent.keyboard("test"); // Type option that does not exist
await userEvent.keyboard("{Enter}", { delay: waitTime });
expect(args.onSubmit).not.toHaveBeenCalled();

await userEvent.keyboard("{ArrowDown}", { delay: waitTime }); // Select "test" custom option
expect(
canvas.getByRole("option", { name: "test", selected: true }),
).toBeVisible();
await userEvent.keyboard("{Enter}", { delay: waitTime }); // De-select "test"
expect(
canvas.queryByRole("option", { name: "test", selected: false }),
).toBeNull();
expect(args.onSubmit).not.toHaveBeenCalled();

await userEvent.keyboard("{Escape}", { delay: waitTime }); // Clear input field
await userEvent.keyboard("{Enter}", { delay: waitTime }); // Enter on empty Input with closed list
expect(args.onSubmit).toHaveBeenCalledOnce();
},
};

0 comments on commit c2f3da8

Please sign in to comment.