From 5a43e197ade705a5b176e7b57157d0a6fec546e9 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Wed, 23 Aug 2023 13:23:22 +0200 Subject: [PATCH 1/9] AutoComplete should keep upper-/lowercase from the word being autocompleted --- .../filteredOptionsContext.tsx | 4 +- .../src/form/combobox/combobox.stories.tsx | 58 +++++++++++++++++++ yarn.lock | 36 ++++++------ 3 files changed, 77 insertions(+), 21 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index df0635af47f..fc8afb0a277 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -92,9 +92,7 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { filteredOptions.length > 0 && !isValueInList(searchTerm, filteredOptions) ) { - setValue( - `${searchTerm}${filteredOptions[0].substring(searchTerm.length)}` - ); + setValue(filteredOptions[0]); setSearchTerm(searchTerm); } }, [ diff --git a/@navikt/core/react/src/form/combobox/combobox.stories.tsx b/@navikt/core/react/src/form/combobox/combobox.stories.tsx index 5ff2a51f5f2..8395fd6d71e 100644 --- a/@navikt/core/react/src/form/combobox/combobox.stories.tsx +++ b/@navikt/core/react/src/form/combobox/combobox.stories.tsx @@ -509,3 +509,61 @@ export const TestThatCallbacksOnlyFireWhenExpected = { expect(args.onChange.mock.calls).toHaveLength(searchWord.length + 1); }, }; + +export const TestConsistentUpperCaseWhenAutoCompleting = { + args: { + onChange: jest.fn(), + onClear: jest.fn(), + onToggleSelected: jest.fn(), + }, + render: (props) => { + return ( + + + + ); + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const input = canvas.getByRole("combobox"); + + userEvent.click(input); + await userEvent.type(input, "upper", { delay: 250 }); + await sleep(250); + expect(input.value).toBe("UPPERCASE"); + }, +}; + +export const TestConsistentLowerCaseWhenAutoCompleting = { + args: { + onChange: jest.fn(), + onClear: jest.fn(), + onToggleSelected: jest.fn(), + }, + render: (props) => { + return ( + + + + ); + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const input = canvas.getByRole("combobox"); + + userEvent.click(input); + await userEvent.type(input, "LOWER", { delay: 250 }); + await sleep(250); + expect(input.value).toBe("lowercase"); + }, +}; diff --git a/yarn.lock b/yarn.lock index 1077c2cf324..ff494436c9b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3351,7 +3351,7 @@ __metadata: languageName: node linkType: hard -"@navikt/aksel-icons@^5.0.2, @navikt/aksel-icons@workspace:@navikt/aksel-icons": +"@navikt/aksel-icons@^5.0.3, @navikt/aksel-icons@workspace:@navikt/aksel-icons": version: 0.0.0-use.local resolution: "@navikt/aksel-icons@workspace:@navikt/aksel-icons" dependencies: @@ -3378,8 +3378,8 @@ __metadata: version: 0.0.0-use.local resolution: "@navikt/aksel-stylelint@workspace:@navikt/aksel-stylelint" dependencies: - "@navikt/ds-css": ^5.0.2 - "@navikt/ds-tokens": ^5.0.2 + "@navikt/ds-css": ^5.0.3 + "@navikt/ds-tokens": ^5.0.3 "@types/jest": ^29.0.0 concurrently: 7.2.1 copyfiles: 2.4.1 @@ -3396,7 +3396,7 @@ __metadata: version: 0.0.0-use.local resolution: "@navikt/aksel@workspace:@navikt/aksel" dependencies: - "@navikt/ds-css": 5.0.2 + "@navikt/ds-css": 5.0.3 "@types/inquirer": ^9.0.3 "@types/jest": ^29.0.0 axios: 1.3.6 @@ -3420,11 +3420,11 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-css@*, @navikt/ds-css@5.0.2, @navikt/ds-css@^5.0.2, @navikt/ds-css@workspace:@navikt/core/css": +"@navikt/ds-css@*, @navikt/ds-css@5.0.3, @navikt/ds-css@^5.0.3, @navikt/ds-css@workspace:@navikt/core/css": version: 0.0.0-use.local resolution: "@navikt/ds-css@workspace:@navikt/core/css" dependencies: - "@navikt/ds-tokens": ^5.0.2 + "@navikt/ds-tokens": ^5.0.3 cssnano: 6.0.0 fast-glob: 3.2.11 lodash: 4.17.21 @@ -3437,12 +3437,12 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-react@*, @navikt/ds-react@5.0.2, @navikt/ds-react@^5.0.2, @navikt/ds-react@workspace:@navikt/core/react": +"@navikt/ds-react@*, @navikt/ds-react@5.0.3, @navikt/ds-react@^5.0.3, @navikt/ds-react@workspace:@navikt/core/react": version: 0.0.0-use.local resolution: "@navikt/ds-react@workspace:@navikt/core/react" dependencies: "@floating-ui/react": 0.24.1 - "@navikt/aksel-icons": ^5.0.2 + "@navikt/aksel-icons": ^5.0.3 "@radix-ui/react-tabs": 1.0.0 "@radix-ui/react-toggle-group": 1.0.0 "@testing-library/dom": 8.13.0 @@ -3476,11 +3476,11 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-tailwind@^5.0.2, @navikt/ds-tailwind@workspace:@navikt/core/tailwind": +"@navikt/ds-tailwind@^5.0.3, @navikt/ds-tailwind@workspace:@navikt/core/tailwind": version: 0.0.0-use.local resolution: "@navikt/ds-tailwind@workspace:@navikt/core/tailwind" dependencies: - "@navikt/ds-tokens": ^5.0.2 + "@navikt/ds-tokens": ^5.0.3 "@types/jest": ^29.0.0 color: 4.2.3 jest: ^29.0.0 @@ -3491,7 +3491,7 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-tokens@^5.0.2, @navikt/ds-tokens@workspace:@navikt/core/tokens": +"@navikt/ds-tokens@^5.0.3, @navikt/ds-tokens@workspace:@navikt/core/tokens": version: 0.0.0-use.local resolution: "@navikt/ds-tokens@workspace:@navikt/core/tokens" dependencies: @@ -8033,11 +8033,11 @@ __metadata: version: 0.0.0-use.local resolution: "aksel.nav.no@workspace:aksel.nav.no" dependencies: - "@navikt/aksel-icons": ^5.0.2 - "@navikt/ds-css": ^5.0.2 - "@navikt/ds-react": ^5.0.2 - "@navikt/ds-tailwind": ^5.0.2 - "@navikt/ds-tokens": ^5.0.2 + "@navikt/aksel-icons": ^5.0.3 + "@navikt/ds-css": ^5.0.3 + "@navikt/ds-react": ^5.0.3 + "@navikt/ds-tailwind": ^5.0.3 + "@navikt/ds-tokens": ^5.0.3 prettier-plugin-tailwindcss: ^0.2.3 languageName: unknown linkType: soft @@ -22026,8 +22026,8 @@ __metadata: version: 0.0.0-use.local resolution: "shadow-dom@workspace:examples/shadow-dom" dependencies: - "@navikt/ds-css": 5.0.2 - "@navikt/ds-react": 5.0.2 + "@navikt/ds-css": 5.0.3 + "@navikt/ds-react": 5.0.3 "@types/react": ^18.0.0 "@types/react-dom": ^18.0.0 "@vitejs/plugin-react": ^3.1.0 From 52f7dcbea78ec009589f79b6c9df44ef3b6d8a28 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Wed, 23 Aug 2023 13:31:29 +0200 Subject: [PATCH 2/9] Fix issue where autocomplete doesn't format the casing of the last character in the word --- .../form/combobox/FilteredOptions/filteredOptionsContext.tsx | 3 +-- @navikt/core/react/src/form/combobox/combobox.stories.tsx | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index fc8afb0a277..6847b1f2d48 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -89,8 +89,7 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { shouldAutocomplete && normalizeText(searchTerm) !== "" && (previousSearchTerm?.length || 0) < searchTerm.length && - filteredOptions.length > 0 && - !isValueInList(searchTerm, filteredOptions) + filteredOptions.length > 0 ) { setValue(filteredOptions[0]); setSearchTerm(searchTerm); diff --git a/@navikt/core/react/src/form/combobox/combobox.stories.tsx b/@navikt/core/react/src/form/combobox/combobox.stories.tsx index 8395fd6d71e..91046e1792e 100644 --- a/@navikt/core/react/src/form/combobox/combobox.stories.tsx +++ b/@navikt/core/react/src/form/combobox/combobox.stories.tsx @@ -533,7 +533,7 @@ export const TestConsistentUpperCaseWhenAutoCompleting = { const input = canvas.getByRole("combobox"); userEvent.click(input); - await userEvent.type(input, "upper", { delay: 250 }); + await userEvent.type(input, "uppercase", { delay: 250 }); await sleep(250); expect(input.value).toBe("UPPERCASE"); }, @@ -562,7 +562,7 @@ export const TestConsistentLowerCaseWhenAutoCompleting = { const input = canvas.getByRole("combobox"); userEvent.click(input); - await userEvent.type(input, "LOWER", { delay: 250 }); + await userEvent.type(input, "LOWERCASE", { delay: 250 }); await sleep(250); expect(input.value).toBe("lowercase"); }, From 8ab8c7280f625badf33426be4f9f8d79e293745f Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Wed, 23 Aug 2023 13:35:53 +0200 Subject: [PATCH 3/9] Added changeset --- .changeset/early-colts-sneeze.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/early-colts-sneeze.md diff --git a/.changeset/early-colts-sneeze.md b/.changeset/early-colts-sneeze.md new file mode 100644 index 00000000000..2de68329554 --- /dev/null +++ b/.changeset/early-colts-sneeze.md @@ -0,0 +1,5 @@ +--- +"@navikt/ds-react": patch +--- + +Autocomplete in combobox will now change formatting of the typed word to match upper-/lowercase of word being autocompleted From 481d243ad7b43b354b997fc87aa6e69dc6209bea Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Fri, 6 Oct 2023 12:32:28 +0200 Subject: [PATCH 4/9] Append the autocompleted text in the same casing used in the list, but preserve casing for the typed characters --- .../filteredOptionsContext.tsx | 4 +++- .../react/src/form/combobox/Input/Input.tsx | 20 +++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index 7568cc2dbb9..60f314583ac 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -97,7 +97,9 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { (previousSearchTerm?.length || 0) < searchTerm.length && filteredOptions.length > 0 ) { - setValue(filteredOptions[0]); + setValue( + `${searchTerm}${filteredOptions[0].substring(searchTerm.length)}` + ); setSearchTerm(searchTerm); } }, [ diff --git a/@navikt/core/react/src/form/combobox/Input/Input.tsx b/@navikt/core/react/src/form/combobox/Input/Input.tsx index df14685e36b..b970b73b2d7 100644 --- a/@navikt/core/react/src/form/combobox/Input/Input.tsx +++ b/@navikt/core/react/src/form/combobox/Input/Input.tsx @@ -46,28 +46,40 @@ const Input = forwardRef( const onEnter = useCallback( (event: React.KeyboardEvent) => { + const isTextInSelectedOptions = (text: string) => { + return selectedOptions.find( + (item) => item.toLowerCase() === text.toLowerCase() + ); + }; + if (currentOption) { event.preventDefault(); // Selecting a value from the dropdown / FilteredOptions toggleOption(currentOption, event); - if (!isMultiSelect && !selectedOptions.includes(currentOption)) + if (!isMultiSelect && !isTextInSelectedOptions(currentOption)) toggleIsListOpen(false); - } else if (shouldAutocomplete && selectedOptions.includes(value)) { + } else if (shouldAutocomplete && isTextInSelectedOptions(value)) { event.preventDefault(); // Trying to set the same value that is already set, so just clearing the input clearInput(event); } else if ((allowNewValues || shouldAutocomplete) && value !== "") { event.preventDefault(); // Autocompleting or adding a new value - toggleOption(value, event); - if (!isMultiSelect && !selectedOptions.includes(value)) + const selectedValue = filteredOptions[0] || value; + toggleOption(selectedValue, event); + if ( + !isMultiSelect && + !isTextInSelectedOptions(filteredOptions[0] || selectedValue) + ) { toggleIsListOpen(false); + } } }, [ allowNewValues, clearInput, currentOption, + filteredOptions, isMultiSelect, selectedOptions, shouldAutocomplete, From da15b19af0553eb729f972c5f9088df7455369ed Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Tue, 10 Oct 2023 11:44:57 +0200 Subject: [PATCH 5/9] Preserve casing used while typing, then use the casing from the options when selecting a value --- .../src/form/combobox/combobox.stories.tsx | 40 +++++-------------- 1 file changed, 9 insertions(+), 31 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/combobox.stories.tsx b/@navikt/core/react/src/form/combobox/combobox.stories.tsx index 16dc773e329..242cddc7659 100644 --- a/@navikt/core/react/src/form/combobox/combobox.stories.tsx +++ b/@navikt/core/react/src/form/combobox/combobox.stories.tsx @@ -467,7 +467,7 @@ export const TestThatCallbacksOnlyFireWhenExpected: StoryObj<{ }, }; -export const TestConsistentUpperCaseWhenAutoCompleting = { +export const TestCasingWhenAutoCompleting = { args: { onChange: jest.fn(), onClear: jest.fn(), @@ -476,8 +476,8 @@ export const TestConsistentUpperCaseWhenAutoCompleting = { render: (props) => { return ( @@ -488,36 +488,14 @@ export const TestConsistentUpperCaseWhenAutoCompleting = { const input = canvas.getByRole("combobox"); userEvent.click(input); - await userEvent.type(input, "uppercase", { delay: 250 }); + await userEvent.type(input, "CaMeL cAsE", { delay: 250 }); await sleep(250); - expect(input.value).toBe("UPPERCASE"); - }, -}; - -export const TestConsistentLowerCaseWhenAutoCompleting = { - args: { - onChange: jest.fn(), - onClear: jest.fn(), - onToggleSelected: jest.fn(), - }, - render: (props) => { - return ( - - ); - }, - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - const input = canvas.getByRole("combobox"); - - userEvent.click(input); - await userEvent.type(input, "LOWERCASE", { delay: 250 }); + expect(input.value).toBe("CaMeL cAsE"); + await userEvent.type(input, "{Enter}"); await sleep(250); - expect(input.value).toBe("lowercase"); + const chips = canvas.getAllByRole("list")[0]; + const selectedChip = within(chips).getAllByRole("listitem")[0]; + expect(selectedChip).toHaveTextContent("Camel Case"); // A weird issue is preventing the accessible name from being used in the test, even if it works in VoiceOver }, }; From ac579eea6f57b86c630070875f7d8705af6f05fe Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Tue, 10 Oct 2023 11:58:24 +0200 Subject: [PATCH 6/9] Added custom options to test of casing when typing --- .../src/form/combobox/combobox.stories.tsx | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/combobox.stories.tsx b/@navikt/core/react/src/form/combobox/combobox.stories.tsx index 242cddc7659..eadc6e34b26 100644 --- a/@navikt/core/react/src/form/combobox/combobox.stories.tsx +++ b/@navikt/core/react/src/form/combobox/combobox.stories.tsx @@ -479,6 +479,7 @@ export const TestCasingWhenAutoCompleting = { options={["Camel Case", "lowercase", "UPPERCASE"]} label="Liker du best store eller små bokstaver?" shouldAutocomplete + allowNewValues {...props} /> ); @@ -487,15 +488,26 @@ export const TestCasingWhenAutoCompleting = { const canvas = within(canvasElement); const input = canvas.getByRole("combobox"); + // With exisiting option userEvent.click(input); - await userEvent.type(input, "CaMeL cAsE", { delay: 250 }); + await userEvent.type(input, "cAmEl CaSe", { delay: 250 }); await sleep(250); - expect(input.value).toBe("CaMeL cAsE"); + expect(input.value).toBe("cAmEl CaSe"); await userEvent.type(input, "{Enter}"); await sleep(250); const chips = canvas.getAllByRole("list")[0]; - const selectedChip = within(chips).getAllByRole("listitem")[0]; - expect(selectedChip).toHaveTextContent("Camel Case"); // A weird issue is preventing the accessible name from being used in the test, even if it works in VoiceOver + const selectedUpperCaseChip = within(chips).getAllByRole("listitem")[0]; + expect(selectedUpperCaseChip).toHaveTextContent("Camel Case"); // A weird issue is preventing the accessible name from being used in the test, even if it works in VoiceOver + + // With custom option + userEvent.click(input); + await userEvent.type(input, "PaScAl CaSe", { delay: 250 }); + await sleep(250); + expect(input.value).toBe("PaScAl CaSe"); + await userEvent.type(input, "{Enter}"); + await sleep(250); + const selectedSnakeCaseChip = within(chips).getAllByRole("listitem")[0]; + expect(selectedSnakeCaseChip).toHaveTextContent("PaScAl CaSe"); // A weird issue is preventing the accessible name from being used in the test, even if it works in VoiceOver }, }; From 8ac058e84a15621cdd0220c642ffc447dfdd868a Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Mon, 23 Oct 2023 15:59:22 +0200 Subject: [PATCH 7/9] Correct changeset --- .changeset/early-colts-sneeze.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/early-colts-sneeze.md b/.changeset/early-colts-sneeze.md index 2de68329554..6049aa3d082 100644 --- a/.changeset/early-colts-sneeze.md +++ b/.changeset/early-colts-sneeze.md @@ -2,4 +2,4 @@ "@navikt/ds-react": patch --- -Autocomplete in combobox will now change formatting of the typed word to match upper-/lowercase of word being autocompleted +Autocomplete in combobox will not change formatting of the letters while being typed, but will use the casing of the autocompleted word when selecting the option. From 2e89656fc29d4392d58a41dbec8f9be50125f9d4 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Mon, 23 Oct 2023 16:00:31 +0200 Subject: [PATCH 8/9] When allowNewValues is enabled, we should use the new value as is when pressing Enter, instead of using the first FilteredOption --- @navikt/core/react/src/form/combobox/Input/Input.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/Input/Input.tsx b/@navikt/core/react/src/form/combobox/Input/Input.tsx index fe5f8dbcd30..e56182daaba 100644 --- a/@navikt/core/react/src/form/combobox/Input/Input.tsx +++ b/@navikt/core/react/src/form/combobox/Input/Input.tsx @@ -31,6 +31,7 @@ const Input = forwardRef( allowNewValues, currentOption, filteredOptions, + isValueNew, toggleIsListOpen, isListOpen, filteredOptionsIndex, @@ -48,7 +49,7 @@ const Input = forwardRef( (event: React.KeyboardEvent) => { const isTextInSelectedOptions = (text: string) => { return selectedOptions.find( - (item) => item.toLowerCase() === text.toLowerCase() + (item) => item.toLocaleLowerCase() === text.toLocaleLowerCase() ); }; @@ -65,7 +66,8 @@ const Input = forwardRef( } else if ((allowNewValues || shouldAutocomplete) && value !== "") { event.preventDefault(); // Autocompleting or adding a new value - const selectedValue = filteredOptions[0] || value; + const selectedValue = + allowNewValues && isValueNew ? value : filteredOptions[0]; toggleOption(selectedValue, event); if ( !isMultiSelect && @@ -81,6 +83,7 @@ const Input = forwardRef( currentOption, filteredOptions, isMultiSelect, + isValueNew, selectedOptions, shouldAutocomplete, toggleIsListOpen, From 457f4b7fe7b2bd6241eee2af9bb0f5e5b49445cc Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Mon, 23 Oct 2023 16:01:36 +0200 Subject: [PATCH 9/9] Change test case to use a partial match for a word in options. This tests both that we will use "add new" option if available and that we preserve casing for new options --- @navikt/core/react/src/form/combobox/combobox.stories.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/combobox.stories.tsx b/@navikt/core/react/src/form/combobox/combobox.stories.tsx index eadc6e34b26..9bda18f805b 100644 --- a/@navikt/core/react/src/form/combobox/combobox.stories.tsx +++ b/@navikt/core/react/src/form/combobox/combobox.stories.tsx @@ -501,13 +501,13 @@ export const TestCasingWhenAutoCompleting = { // With custom option userEvent.click(input); - await userEvent.type(input, "PaScAl CaSe", { delay: 250 }); + await userEvent.type(input, "cAmEl{Backspace}", { delay: 250 }); await sleep(250); - expect(input.value).toBe("PaScAl CaSe"); + expect(input.value).toBe("cAmEl"); await userEvent.type(input, "{Enter}"); await sleep(250); - const selectedSnakeCaseChip = within(chips).getAllByRole("listitem")[0]; - expect(selectedSnakeCaseChip).toHaveTextContent("PaScAl CaSe"); // A weird issue is preventing the accessible name from being used in the test, even if it works in VoiceOver + const selectedNewValueChip = within(chips).getAllByRole("listitem")[0]; + expect(selectedNewValueChip).toHaveTextContent("cAmEl"); // A weird issue is preventing the accessible name from being used in the test, even if it works in VoiceOver }, };