From 795ed37efdb26dc674ef7efe36bd05800e979f4c Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Wed, 18 Oct 2023 15:15:25 +0200 Subject: [PATCH 01/12] Add new useVirtualFocus hook, for use in Combobox. Might be useful for other components later, but will scope it for Combobox at first. --- .../FilteredOptions/FilteredOptions.tsx | 25 ++-- .../filteredOptionsContext.tsx | 131 ++++-------------- .../FilteredOptions/useVirtualFocus.ts | 72 ++++++++++ .../react/src/form/combobox/Input/Input.tsx | 14 +- yarn.lock | 34 ++--- 5 files changed, 142 insertions(+), 134 deletions(-) create mode 100644 @navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx index 5678eaac329..4a312f23871 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx @@ -18,13 +18,13 @@ const FilteredOptions = () => { isLoading, isListOpen, filteredOptions, - filteredOptionsIndex, filteredOptionsRef, isMouseLastUsedInputDevice, setIsMouseLastUsedInputDevice, isValueNew, - setFilteredOptionsIndex, + moveFocusToElement, toggleIsListOpen, + activeDecendantId, } = useFilteredOptionsContext(); const { isMultiSelect, selectedOptions, toggleOption } = useSelectedOptionsContext(); @@ -46,6 +46,7 @@ const FilteredOptions = () => { role="option" aria-selected={false} id={`${id}-is-loading`} + data-no-focus="true" > @@ -54,8 +55,8 @@ const FilteredOptions = () => {
  • { - if (filteredOptionsIndex !== -1) { - setFilteredOptionsIndex(-1); + if (activeDecendantId !== `${id}-combobox-new-option`) { + moveFocusToElement(`${id}-combobox-new-option`); setIsMouseLastUsedInputDevice(true); } }} @@ -67,10 +68,11 @@ const FilteredOptions = () => { id={`${id}-combobox-new-option`} className={cl("navds-combobox__list-item__new-option", { "navds-combobox__list-item__new-option--focus": - filteredOptionsIndex === -1, + activeDecendantId === `${id}-combobox-new-option`, })} role="option" aria-selected={false} + data-value={value} > @@ -87,14 +89,16 @@ const FilteredOptions = () => { role="option" aria-selected={false} id={`${id}-no-hits`} + data-no-focus="true" > Ingen søketreff
    • )} - {filteredOptions.map((option, index) => ( + {filteredOptions.map((option) => (
  • { key={option} tabIndex={-1} onMouseMove={() => { - if (filteredOptionsIndex !== index) { - setFilteredOptionsIndex(index); + if ( + activeDecendantId !== `${id}-option-${option.replace(" ", "-")}` + ) { + moveFocusToElement(`${id}-option-${option.replace(" ", "-")}`); setIsMouseLastUsedInputDevice(true); } }} @@ -114,6 +120,7 @@ const FilteredOptions = () => { }} role="option" aria-selected={selectedOptions.includes(option)} + data-value={option} > {option} {selectedOptions.includes(option) && } diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index 397c4d310f5..d3dd528259d 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -1,6 +1,5 @@ import React, { useState, - useEffect, useMemo, createContext, useContext, @@ -13,6 +12,7 @@ import { useCustomOptionsContext } from "../customOptionsContext"; import { useInputContext } from "../Input/inputContext"; import usePrevious from "../../../util/usePrevious"; import { useClientLayoutEffect } from "../../../util"; +import useVirtualFocus from "./useVirtualFocus"; const normalizeText = (text: string): string => typeof text === "string" ? `${text}`.toLowerCase().trim() : ""; @@ -31,8 +31,6 @@ type FilteredOptionsContextType = { allowNewValues?: boolean; ariaDescribedBy?: string; filteredOptionsRef: React.RefObject; - filteredOptionsIndex: number | null; - setFilteredOptionsIndex: (index: number) => void; isListOpen: boolean; isLoading?: boolean; filteredOptions: string[]; @@ -41,9 +39,9 @@ type FilteredOptionsContextType = { isValueNew: boolean; toggleIsListOpen: (newState?: boolean) => void; currentOption: string | null; - resetFilteredOptionsIndex: () => void; moveFocusUp: () => void; moveFocusDown: () => void; + moveFocusToElement: (id: string) => void; moveFocusToInput: () => void; moveFocusToEnd: () => void; shouldAutocomplete?: boolean; @@ -61,6 +59,7 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { options, } = props; const filteredOptionsRef = useRef(null); + const virtualFocus = useVirtualFocus(filteredOptionsRef.current); const { inputProps: { "aria-describedby": partialAriaDescribedBy, id }, value, @@ -70,9 +69,6 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { shouldAutocomplete, } = useInputContext(); - const [filteredOptionsIndex, setFilteredOptionsIndex] = useState< - number | null - >(null); const [isInternalListOpen, setInternalListOpen] = useState(false); const { customOptions } = useCustomOptionsContext(); @@ -81,7 +77,6 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { return externalFilteredOptions; } const opts = [...customOptions, ...options]; - setFilteredOptionsIndex(null); return getMatchingValuesFromList(searchTerm, opts); }, [customOptions, externalFilteredOptions, options, searchTerm]); @@ -116,20 +111,19 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { return isExternalListOpen ?? isInternalListOpen; }, [isExternalListOpen, isInternalListOpen]); - const toggleIsListOpen = useCallback((newState?: boolean) => { - setFilteredOptionsIndex(null); - setInternalListOpen((oldState) => newState ?? !oldState); - }, []); + const toggleIsListOpen = useCallback( + (newState?: boolean) => { + virtualFocus.moveFocusToTop(); + setInternalListOpen((oldState) => newState ?? !oldState); + }, + [virtualFocus] + ); const isValueNew = useMemo( () => Boolean(value) && !isValueInList(value, filteredOptions), [value, filteredOptions] ); - const getMinimumIndex = useCallback(() => { - return isValueNew && allowNewValues ? -1 : 0; - }, [allowNewValues, isValueNew]); - const ariaDescribedBy = useMemo(() => { let activeOption; if (!isLoading && filteredOptions.length === 0) { @@ -152,102 +146,37 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { id, ]); - const currentOption = useMemo(() => { - if (filteredOptionsIndex == null) { - return null; - } - if (filteredOptionsIndex === -1) { - return value; - } - return filteredOptions[filteredOptionsIndex]; - }, [filteredOptionsIndex, filteredOptions, value]); - - const resetFilteredOptionsIndex = () => { - setFilteredOptionsIndex(getMinimumIndex()); - }; - - const scrollToOption = useCallback((newIndex: number) => { - if ( - filteredOptionsRef.current && - filteredOptionsRef.current.children[newIndex] - ) { - const child = filteredOptionsRef.current.children[newIndex]; - const { top, bottom } = child.getBoundingClientRect(); - const parentRect = filteredOptionsRef.current.getBoundingClientRect(); - if (top < parentRect.top || bottom > parentRect.bottom) { - child.scrollIntoView({ block: "nearest" }); - } - } - }, []); - - useEffect(() => { - if (filteredOptionsIndex !== null && isListOpen) { - scrollToOption(filteredOptionsIndex); - } - }, [filteredOptionsIndex, isListOpen, scrollToOption]); - - const moveFocusToInput = useCallback(() => { - setFilteredOptionsIndex(null); - toggleIsListOpen(false); - }, [toggleIsListOpen]); - - const moveFocusToEnd = useCallback(() => { - const lastIndex = filteredOptions.length - 1; - toggleIsListOpen(true); - setFilteredOptionsIndex(lastIndex); - }, [filteredOptions.length, toggleIsListOpen]); + // TODO: Re-write or remove after re-write? + const currentOption = useMemo( + () => virtualFocus.activeElement?.getAttribute("data-value"), + [virtualFocus] + ); + // TODO: Can be deleted if we move toggleIsListOpen(false) to the event handling in Input.tsx const moveFocusUp = useCallback(() => { - if (filteredOptionsIndex === null) { - return; - } - if (filteredOptionsIndex === getMinimumIndex()) { + if (virtualFocus.isFocusOnTheTop) { toggleIsListOpen(false); - setFilteredOptionsIndex(null); - } else { - const newIndex = Math.max(getMinimumIndex(), filteredOptionsIndex - 1); - setFilteredOptionsIndex(newIndex); } - }, [filteredOptionsIndex, getMinimumIndex, toggleIsListOpen]); + virtualFocus.moveFocusUp(); + }, [toggleIsListOpen, virtualFocus]); + // TODO: Can be deleted if we move toggleIsListOpen(true) to the event handling in Input.tsx const moveFocusDown = useCallback(() => { - if (filteredOptionsIndex === null || !isListOpen) { + if (virtualFocus.activeElement === null || !isListOpen) { toggleIsListOpen(true); - if (allowNewValues || filteredOptions.length >= 1) { - setFilteredOptionsIndex(getMinimumIndex()); - } - return; } - const newIndex = Math.min( - filteredOptionsIndex + 1, - Math.max(getMinimumIndex(), filteredOptions.length - 1) - ); - setFilteredOptionsIndex(newIndex); - }, [ - allowNewValues, - filteredOptions.length, - filteredOptionsIndex, - getMinimumIndex, - isListOpen, - toggleIsListOpen, - ]); + virtualFocus.moveFocusDown(); + }, [isListOpen, toggleIsListOpen, virtualFocus]); - const activeDecendantId = useMemo(() => { - if (filteredOptionsIndex === null) { - return undefined; - } else if (filteredOptionsIndex === -1) { - return `${id}-combobox-new-option`; - } else { - return `${id}-option-${currentOption?.replace(" ", "-")}`; - } - }, [filteredOptionsIndex, currentOption, id]); + const activeDecendantId = useMemo( + () => virtualFocus.activeElement?.getAttribute("id") || undefined, + [virtualFocus.activeElement] + ); const filteredOptionsState = { activeDecendantId, allowNewValues, filteredOptionsRef, - filteredOptionsIndex, - setFilteredOptionsIndex, shouldAutocomplete, isListOpen, isLoading, @@ -257,11 +186,11 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { isValueNew, toggleIsListOpen, currentOption, - resetFilteredOptionsIndex, moveFocusUp, moveFocusDown, - moveFocusToInput, - moveFocusToEnd, + moveFocusToElement: virtualFocus.moveFocusToElement, + moveFocusToInput: virtualFocus.moveFocusToTop, + moveFocusToEnd: virtualFocus.moveFocusToBottom, ariaDescribedBy, }; diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts b/@navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts new file mode 100644 index 00000000000..5d58c262f6a --- /dev/null +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts @@ -0,0 +1,72 @@ +import { useState } from "react"; + +const useVirtualFocus = (containerRef: HTMLElement | null) => { + const [index, setIndex] = useState(-1); + + const listOfAllChildren: Array = containerRef?.children + ? Array.prototype.slice.call(containerRef?.children) + : []; + const elementsAbleToReceiveFocus = listOfAllChildren.filter( + (child) => child.getAttribute("data-no-focus") !== "true" + ); + + const activeElement = elementsAbleToReceiveFocus[index]; + const getElementById = (id: string) => + listOfAllChildren.find((element) => element.id === id); + const isFocusOnTheTop = index === 0; + const isFocusOnTheBottom = index === elementsAbleToReceiveFocus.length - 1; + + const scrollToOption = (newIndex: number) => { + const indexOfElementToScrollTo = Math.min( + Math.max(newIndex, 0), + containerRef?.children.length || 0 + ); + if (containerRef?.children[indexOfElementToScrollTo]) { + const child = containerRef.children[indexOfElementToScrollTo]; + const { top, bottom } = child.getBoundingClientRect(); + const parentRect = containerRef.getBoundingClientRect(); + if (top < parentRect.top || bottom > parentRect.bottom) { + child.scrollIntoView({ block: "nearest" }); + } + } + }; + + const _moveFocusAndScrollTo = (_index: number) => { + setIndex(_index); + scrollToOption(_index); + }; + const moveFocusUp = () => _moveFocusAndScrollTo(Math.max(index - 1, -1)); + const moveFocusDown = () => + _moveFocusAndScrollTo( + Math.min(index + 1, elementsAbleToReceiveFocus.length - 1) + ); + const moveFocusToTop = () => _moveFocusAndScrollTo(-1); + const moveFocusToBottom = () => + _moveFocusAndScrollTo(elementsAbleToReceiveFocus.length - 1); + const moveFocusToElement = (id: string) => { + const thisElement = elementsAbleToReceiveFocus.find( + (_element) => _element.getAttribute("id") === id + ); + const indexOfElement = thisElement + ? elementsAbleToReceiveFocus.indexOf(thisElement) + : -1; + if (indexOfElement >= 0) { + setIndex(indexOfElement); + } + }; + + return { + activeElement, + getElementById, + isFocusOnTheTop, + isFocusOnTheBottom, + setIndex, + moveFocusUp, + moveFocusDown, + moveFocusToElement, + moveFocusToTop, + moveFocusToBottom, + }; +}; + +export default useVirtualFocus; diff --git a/@navikt/core/react/src/form/combobox/Input/Input.tsx b/@navikt/core/react/src/form/combobox/Input/Input.tsx index 4a8639567aa..b6be81cea8f 100644 --- a/@navikt/core/react/src/form/combobox/Input/Input.tsx +++ b/@navikt/core/react/src/form/combobox/Input/Input.tsx @@ -33,13 +33,11 @@ const Input = forwardRef( filteredOptions, toggleIsListOpen, isListOpen, - filteredOptionsIndex, moveFocusUp, moveFocusDown, ariaDescribedBy, moveFocusToInput, moveFocusToEnd, - setFilteredOptionsIndex, setIsMouseLastUsedInputDevice, shouldAutocomplete, } = useFilteredOptionsContext(); @@ -50,8 +48,9 @@ const Input = forwardRef( event.preventDefault(); // Selecting a value from the dropdown / FilteredOptions toggleOption(currentOption, event); - if (!isMultiSelect && !selectedOptions.includes(currentOption)) + if (!isMultiSelect && !selectedOptions.includes(currentOption)) { toggleIsListOpen(false); + } } else if (shouldAutocomplete && selectedOptions.includes(value)) { event.preventDefault(); // Trying to set the same value that is already set, so just clearing the input @@ -118,7 +117,7 @@ const Input = forwardRef( } else if (e.key === "ArrowUp") { // Check that the FilteredOptions list is open and has virtual focus. // Otherwise ignore keystrokes, so it doesn't interfere with text editing - if (isListOpen && filteredOptionsIndex !== null) { + if (isListOpen && activeDecendantId) { e.preventDefault(); moveFocusUp(); } @@ -130,7 +129,7 @@ const Input = forwardRef( removeSelectedOption, moveFocusDown, isListOpen, - filteredOptionsIndex, + activeDecendantId, moveFocusUp, setIsMouseLastUsedInputDevice, ] @@ -144,13 +143,14 @@ const Input = forwardRef( } else if (filteredOptions.length === 0) { toggleIsListOpen(false); } + moveFocusToInput(); onChange(event); }, - [filteredOptions.length, onChange, toggleIsListOpen] + [filteredOptions.length, moveFocusToInput, onChange, toggleIsListOpen] ); const onBlur = () => { - setFilteredOptionsIndex(-1); + moveFocusToInput(); }; return ( diff --git a/yarn.lock b/yarn.lock index b0d932faef5..62e7e20cd06 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3415,7 +3415,7 @@ __metadata: languageName: node linkType: hard -"@navikt/aksel-icons@^5.7.2, @navikt/aksel-icons@workspace:@navikt/aksel-icons": +"@navikt/aksel-icons@^5.7.3, @navikt/aksel-icons@workspace:@navikt/aksel-icons": version: 0.0.0-use.local resolution: "@navikt/aksel-icons@workspace:@navikt/aksel-icons" dependencies: @@ -3442,8 +3442,8 @@ __metadata: version: 0.0.0-use.local resolution: "@navikt/aksel-stylelint@workspace:@navikt/aksel-stylelint" dependencies: - "@navikt/ds-css": ^5.7.2 - "@navikt/ds-tokens": ^5.7.2 + "@navikt/ds-css": ^5.7.3 + "@navikt/ds-tokens": ^5.7.3 "@types/jest": ^29.0.0 concurrently: 7.2.1 copyfiles: 2.4.1 @@ -3461,7 +3461,7 @@ __metadata: version: 0.0.0-use.local resolution: "@navikt/aksel@workspace:@navikt/aksel" dependencies: - "@navikt/ds-css": 5.7.2 + "@navikt/ds-css": 5.7.3 "@types/inquirer": ^9.0.3 "@types/jest": ^29.0.0 axios: 1.3.6 @@ -3485,11 +3485,11 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-css@*, @navikt/ds-css@5.7.2, @navikt/ds-css@^5.7.2, @navikt/ds-css@workspace:@navikt/core/css": +"@navikt/ds-css@*, @navikt/ds-css@5.7.3, @navikt/ds-css@^5.7.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.7.2 + "@navikt/ds-tokens": ^5.7.3 cssnano: 6.0.0 fast-glob: 3.2.11 lodash: 4.17.21 @@ -3502,13 +3502,13 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-react@*, @navikt/ds-react@^5.7.2, @navikt/ds-react@workspace:@navikt/core/react": +"@navikt/ds-react@*, @navikt/ds-react@^5.7.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.25.4 - "@navikt/aksel-icons": ^5.7.2 - "@navikt/ds-tokens": ^5.7.2 + "@navikt/aksel-icons": ^5.7.3 + "@navikt/ds-tokens": ^5.7.3 "@radix-ui/react-tabs": 1.0.0 "@radix-ui/react-toggle-group": 1.0.0 "@testing-library/dom": 8.13.0 @@ -3542,11 +3542,11 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-tailwind@^5.7.2, @navikt/ds-tailwind@workspace:@navikt/core/tailwind": +"@navikt/ds-tailwind@^5.7.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.7.2 + "@navikt/ds-tokens": ^5.7.3 "@types/jest": ^29.0.0 color: 4.2.3 jest: ^29.0.0 @@ -3557,7 +3557,7 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-tokens@^5.7.2, @navikt/ds-tokens@workspace:@navikt/core/tokens": +"@navikt/ds-tokens@^5.7.3, @navikt/ds-tokens@workspace:@navikt/core/tokens": version: 0.0.0-use.local resolution: "@navikt/ds-tokens@workspace:@navikt/core/tokens" dependencies: @@ -8410,11 +8410,11 @@ __metadata: version: 0.0.0-use.local resolution: "aksel.nav.no@workspace:aksel.nav.no" dependencies: - "@navikt/aksel-icons": ^5.7.2 - "@navikt/ds-css": ^5.7.2 - "@navikt/ds-react": ^5.7.2 - "@navikt/ds-tailwind": ^5.7.2 - "@navikt/ds-tokens": ^5.7.2 + "@navikt/aksel-icons": ^5.7.3 + "@navikt/ds-css": ^5.7.3 + "@navikt/ds-react": ^5.7.3 + "@navikt/ds-tailwind": ^5.7.3 + "@navikt/ds-tokens": ^5.7.3 prettier-plugin-tailwindcss: ^0.2.3 languageName: unknown linkType: soft From 59ce7955da1f5f29a3153087cd4840d1d5f4a548 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Thu, 19 Oct 2023 10:20:49 +0200 Subject: [PATCH 02/12] Create filteredOptionsUtils, which has utiltity functions for creating IDs and for text lookups in lists. --- .../FilteredOptions/FilteredOptions.tsx | 25 +++++++------ .../FilteredOptions/filtered-options-util.ts | 36 +++++++++++++++++++ .../filteredOptionsContext.tsx | 28 ++++++--------- .../react/src/form/combobox/Input/Input.tsx | 3 +- 4 files changed, 62 insertions(+), 30 deletions(-) create mode 100644 @navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx index 4a312f23871..085371e7994 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx @@ -6,6 +6,7 @@ import { useSelectedOptionsContext } from "../SelectedOptions/selectedOptionsCon import { useInputContext } from "../Input/inputContext"; import { Loader } from "../../../loader"; import { BodyShort, Label } from "../../../typography"; +import filteredOptionsUtil from "./filtered-options-util"; const FilteredOptions = () => { const { @@ -36,7 +37,7 @@ const FilteredOptions = () => { "navds-combobox__list--closed": !isListOpen, "navds-combobox__list--with-hover": isMouseLastUsedInputDevice, })} - id={`${id}-filtered-options`} + id={filteredOptionsUtil.getFilteredOptionsId(id)} role="listbox" tabIndex={-1} > @@ -45,7 +46,7 @@ const FilteredOptions = () => { className="navds-combobox__list-item--loading" role="option" aria-selected={false} - id={`${id}-is-loading`} + id={filteredOptionsUtil.getIsLoadingId(id)} data-no-focus="true" > @@ -55,8 +56,10 @@ const FilteredOptions = () => {
  • { - if (activeDecendantId !== `${id}-combobox-new-option`) { - moveFocusToElement(`${id}-combobox-new-option`); + if ( + activeDecendantId !== filteredOptionsUtil.getAddNewOptionId(id) + ) { + moveFocusToElement(filteredOptionsUtil.getAddNewOptionId(id)); setIsMouseLastUsedInputDevice(true); } }} @@ -65,10 +68,10 @@ const FilteredOptions = () => { if (!isMultiSelect && !selectedOptions.includes(value)) toggleIsListOpen(false); }} - id={`${id}-combobox-new-option`} + id={filteredOptionsUtil.getAddNewOptionId(id)} className={cl("navds-combobox__list-item__new-option", { "navds-combobox__list-item__new-option--focus": - activeDecendantId === `${id}-combobox-new-option`, + activeDecendantId === filteredOptionsUtil.getAddNewOptionId(id), })} role="option" aria-selected={false} @@ -88,7 +91,7 @@ const FilteredOptions = () => { className="navds-combobox__list-item__no-options" role="option" aria-selected={false} - id={`${id}-no-hits`} + id={filteredOptionsUtil.getNoHitsId(id)} data-no-focus="true" > Ingen søketreff @@ -98,18 +101,18 @@ const FilteredOptions = () => {
  • { if ( - activeDecendantId !== `${id}-option-${option.replace(" ", "-")}` + activeDecendantId !== filteredOptionsUtil.getOptionId(id, option) ) { - moveFocusToElement(`${id}-option-${option.replace(" ", "-")}`); + moveFocusToElement(filteredOptionsUtil.getOptionId(id, option)); setIsMouseLastUsedInputDevice(true); } }} diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts b/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts new file mode 100644 index 00000000000..5d472bccf44 --- /dev/null +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts @@ -0,0 +1,36 @@ +const normalizeText = (text: string): string => + typeof text === "string" ? `${text}`.toLowerCase().trim() : ""; + +const isPartOfText = (value, text) => + normalizeText(text).startsWith(normalizeText(value ?? "")); + +const isValueInList = (value, list) => + list?.find((listItem) => normalizeText(value) === normalizeText(listItem)); + +const getMatchingValuesFromList = (value, list) => + list?.filter((listItem) => isPartOfText(value, listItem)); + +const getFilteredOptionsId = (comboboxId: string) => + `${comboboxId}-filtered-options`; + +const getOptionId = (comboboxId: string, option: string) => + `${comboboxId}-option-${option.replace(" ", "-")}`; + +const getAddNewOptionId = (comboboxId: string) => + `${comboboxId}-combobox-new-option`; + +const getIsLoadingId = (comboboxId: string) => `${comboboxId}-is-loading`; + +const getNoHitsId = (comboboxId: string) => `${comboboxId}-no-hits`; + +export default { + normalizeText, + isPartOfText, + isValueInList, + getMatchingValuesFromList, + getFilteredOptionsId, + getAddNewOptionId, + getOptionId, + getIsLoadingId, + getNoHitsId, +}; diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index d3dd528259d..6bd4061be0a 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -12,19 +12,9 @@ import { useCustomOptionsContext } from "../customOptionsContext"; import { useInputContext } from "../Input/inputContext"; import usePrevious from "../../../util/usePrevious"; import { useClientLayoutEffect } from "../../../util"; +import filteredOptionsUtils from "./filtered-options-util"; import useVirtualFocus from "./useVirtualFocus"; -const normalizeText = (text: string): string => - typeof text === "string" ? `${text}`.toLowerCase().trim() : ""; - -const isPartOfText = (value, text) => - normalizeText(text).startsWith(normalizeText(value ?? "")); - -const isValueInList = (value, list) => - list?.find((listItem) => normalizeText(value) === normalizeText(listItem)); - -const getMatchingValuesFromList = (value, list) => - list?.filter((listItem) => isPartOfText(value, listItem)); type FilteredOptionsContextType = { activeDecendantId?: string; @@ -77,7 +67,7 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { return externalFilteredOptions; } const opts = [...customOptions, ...options]; - return getMatchingValuesFromList(searchTerm, opts); + return filteredOptionsUtils.getMatchingValuesFromList(searchTerm, opts); }, [customOptions, externalFilteredOptions, options, searchTerm]); const previousSearchTerm = usePrevious(searchTerm); @@ -88,10 +78,10 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { useClientLayoutEffect(() => { if ( shouldAutocomplete && - normalizeText(searchTerm) !== "" && + filteredOptionsUtils.normalizeText(searchTerm) !== "" && (previousSearchTerm?.length || 0) < searchTerm.length && filteredOptions.length > 0 && - !isValueInList(searchTerm, filteredOptions) + !filteredOptionsUtils.isValueInList(searchTerm, filteredOptions) ) { setValue( `${searchTerm}${filteredOptions[0].substring(searchTerm.length)}` @@ -120,19 +110,21 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { ); const isValueNew = useMemo( - () => Boolean(value) && !isValueInList(value, filteredOptions), + () => + Boolean(value) && + !filteredOptionsUtils.isValueInList(value, filteredOptions), [value, filteredOptions] ); const ariaDescribedBy = useMemo(() => { let activeOption; if (!isLoading && filteredOptions.length === 0) { - activeOption = `${id}-no-hits`; + activeOption = filteredOptionsUtils.getNoHitsId(id); } else if ((value && value !== "") || isLoading) { if (shouldAutocomplete && filteredOptions[0]) { - activeOption = `${id}-option-${filteredOptions[0].replace(" ", "-")}`; + activeOption = filteredOptionsUtils.getOptionId(id, filteredOptions[0]); } else if (isListOpen && isLoading) { - activeOption = `${id}-is-loading`; + activeOption = filteredOptionsUtils.getIsLoadingId(id); } } return cl(activeOption, partialAriaDescribedBy) || undefined; diff --git a/@navikt/core/react/src/form/combobox/Input/Input.tsx b/@navikt/core/react/src/form/combobox/Input/Input.tsx index b6be81cea8f..8fa9d2125e4 100644 --- a/@navikt/core/react/src/form/combobox/Input/Input.tsx +++ b/@navikt/core/react/src/form/combobox/Input/Input.tsx @@ -9,6 +9,7 @@ import cl from "clsx"; import { useSelectedOptionsContext } from "../SelectedOptions/selectedOptionsContext"; import { useFilteredOptionsContext } from "../FilteredOptions/filteredOptionsContext"; import { useInputContext } from "./inputContext"; +import filteredOptionsUtil from "../FilteredOptions/filtered-options-util"; interface InputProps extends Omit, "value"> { @@ -165,7 +166,7 @@ const Input = forwardRef( onBlur={onBlur} onKeyUp={handleKeyUp} onKeyDown={handleKeyDown} - aria-controls={`${inputProps.id}-filtered-options`} + aria-controls={filteredOptionsUtil.getFilteredOptionsId(inputProps.id)} aria-expanded={!!isListOpen} autoComplete="off" aria-autocomplete={shouldAutocomplete ? "both" : "list"} From 0b080c8f5e84b1b26b42eb2fb182abb463b16e42 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Fri, 20 Oct 2023 09:51:10 +0200 Subject: [PATCH 03/12] Get rid of data-value, and use filteredOptionsMap to look up the value of the currently (virtually) focused element instead This allows use of e.g. numbers as options, which data-value didn't. --- .../core/react/src/form/combobox/Combobox.tsx | 4 ++-- .../FilteredOptions/FilteredOptions.tsx | 2 -- .../filteredOptionsContext.tsx | 23 +++++++++++++++---- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/Combobox.tsx b/@navikt/core/react/src/form/combobox/Combobox.tsx index 3026647f006..42d155375f2 100644 --- a/@navikt/core/react/src/form/combobox/Combobox.tsx +++ b/@navikt/core/react/src/form/combobox/Combobox.tsx @@ -33,7 +33,7 @@ export const Combobox = forwardRef< const toggleListButtonRef = useRef(null); - const { currentOption, toggleIsListOpen } = useFilteredOptionsContext(); + const { activeDecendantId, toggleIsListOpen } = useFilteredOptionsContext(); const { selectedOptions } = useSelectedOptionsContext(); const { @@ -92,7 +92,7 @@ export const Combobox = forwardRef< "navds-combobox__wrapper-inner navds-text-field__input", { "navds-combobox__wrapper-inner--virtually-unfocused": - currentOption !== null, + activeDecendantId !== null, } )} onClick={focusInput} diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx index 085371e7994..bb37e7dd9fe 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx @@ -75,7 +75,6 @@ const FilteredOptions = () => { })} role="option" aria-selected={false} - data-value={value} > @@ -123,7 +122,6 @@ const FilteredOptions = () => { }} role="option" aria-selected={selectedOptions.includes(option)} - data-value={option} > {option} {selectedOptions.includes(option) && } diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index 6bd4061be0a..cf6eb1b09a4 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -75,6 +75,20 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { const [isMouseLastUsedInputDevice, setIsMouseLastUsedInputDevice] = useState(false); + const filteredOptionsMap = useMemo( + () => + options.reduce( + (map: Map, _option: string) => ({ + ...map, + [filteredOptionsUtils.getOptionId(id, _option)]: _option, + }), + { + [filteredOptionsUtils.getAddNewOptionId(id)]: allowNewValues && value, + } + ), + [allowNewValues, id, options, value] + ); + useClientLayoutEffect(() => { if ( shouldAutocomplete && @@ -112,8 +126,8 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { const isValueNew = useMemo( () => Boolean(value) && - !filteredOptionsUtils.isValueInList(value, filteredOptions), - [value, filteredOptions] + !filteredOptionsMap[filteredOptionsUtils.getOptionId(id, value)], + [filteredOptionsMap, id, value] ); const ariaDescribedBy = useMemo(() => { @@ -140,8 +154,9 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { // TODO: Re-write or remove after re-write? const currentOption = useMemo( - () => virtualFocus.activeElement?.getAttribute("data-value"), - [virtualFocus] + () => + filteredOptionsMap[virtualFocus.activeElement?.getAttribute("id") || -1], + [filteredOptionsMap, virtualFocus] ); // TODO: Can be deleted if we move toggleIsListOpen(false) to the event handling in Input.tsx From 0e148027589f262e7a9de377d2f2ea2a6be2b890 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Fri, 20 Oct 2023 09:52:38 +0200 Subject: [PATCH 04/12] Using useState instead of useRef to handle filteredOptionsRef --- .../FilteredOptions/filteredOptionsContext.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index cf6eb1b09a4..ccf4ef027bf 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -4,7 +4,6 @@ import React, { createContext, useContext, useCallback, - useRef, SetStateAction, } from "react"; import cl from "clsx"; @@ -15,12 +14,13 @@ import { useClientLayoutEffect } from "../../../util"; import filteredOptionsUtils from "./filtered-options-util"; import useVirtualFocus from "./useVirtualFocus"; - type FilteredOptionsContextType = { activeDecendantId?: string; allowNewValues?: boolean; ariaDescribedBy?: string; - filteredOptionsRef: React.RefObject; + filteredOptionsRef: React.Dispatch< + React.SetStateAction + >; isListOpen: boolean; isLoading?: boolean; filteredOptions: string[]; @@ -48,8 +48,9 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { isLoading, options, } = props; - const filteredOptionsRef = useRef(null); - const virtualFocus = useVirtualFocus(filteredOptionsRef.current); + const [filteredOptionsRef, setFilteredOptionsRef] = + useState(null); + const virtualFocus = useVirtualFocus(filteredOptionsRef); const { inputProps: { "aria-describedby": partialAriaDescribedBy, id }, value, @@ -183,7 +184,7 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { const filteredOptionsState = { activeDecendantId, allowNewValues, - filteredOptionsRef, + filteredOptionsRef: setFilteredOptionsRef, shouldAutocomplete, isListOpen, isLoading, From d12190f96f0610935ca96b46101e3e29d1d43afd Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Fri, 20 Oct 2023 09:59:22 +0200 Subject: [PATCH 05/12] Added changeset --- .changeset/weak-dodos-attend.md | 5 +++++ yarn.lock | 34 ++++++++++++++++----------------- 2 files changed, 22 insertions(+), 17 deletions(-) create mode 100644 .changeset/weak-dodos-attend.md diff --git a/.changeset/weak-dodos-attend.md b/.changeset/weak-dodos-attend.md new file mode 100644 index 00000000000..a5c0e0afa24 --- /dev/null +++ b/.changeset/weak-dodos-attend.md @@ -0,0 +1,5 @@ +--- +"@navikt/ds-react": patch +--- + +Added useVirtualFocus hook - used in Combobox for now diff --git a/yarn.lock b/yarn.lock index 42996b2c2fb..dccc09f3ca9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3415,7 +3415,7 @@ __metadata: languageName: node linkType: hard -"@navikt/aksel-icons@^5.7.3, @navikt/aksel-icons@workspace:@navikt/aksel-icons": +"@navikt/aksel-icons@^5.7.4, @navikt/aksel-icons@workspace:@navikt/aksel-icons": version: 0.0.0-use.local resolution: "@navikt/aksel-icons@workspace:@navikt/aksel-icons" dependencies: @@ -3442,8 +3442,8 @@ __metadata: version: 0.0.0-use.local resolution: "@navikt/aksel-stylelint@workspace:@navikt/aksel-stylelint" dependencies: - "@navikt/ds-css": ^5.7.3 - "@navikt/ds-tokens": ^5.7.3 + "@navikt/ds-css": ^5.7.4 + "@navikt/ds-tokens": ^5.7.4 "@types/jest": ^29.0.0 concurrently: 7.2.1 copyfiles: 2.4.1 @@ -3461,7 +3461,7 @@ __metadata: version: 0.0.0-use.local resolution: "@navikt/aksel@workspace:@navikt/aksel" dependencies: - "@navikt/ds-css": 5.7.3 + "@navikt/ds-css": 5.7.4 "@types/inquirer": ^9.0.3 "@types/jest": ^29.0.0 axios: 1.3.6 @@ -3485,11 +3485,11 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-css@*, @navikt/ds-css@5.7.3, @navikt/ds-css@^5.7.3, @navikt/ds-css@workspace:@navikt/core/css": +"@navikt/ds-css@*, @navikt/ds-css@5.7.4, @navikt/ds-css@^5.7.4, @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.7.3 + "@navikt/ds-tokens": ^5.7.4 cssnano: 6.0.0 fast-glob: 3.2.11 lodash: 4.17.21 @@ -3502,13 +3502,13 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-react@*, @navikt/ds-react@^5.7.3, @navikt/ds-react@workspace:@navikt/core/react": +"@navikt/ds-react@*, @navikt/ds-react@^5.7.4, @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.25.4 - "@navikt/aksel-icons": ^5.7.3 - "@navikt/ds-tokens": ^5.7.3 + "@navikt/aksel-icons": ^5.7.4 + "@navikt/ds-tokens": ^5.7.4 "@radix-ui/react-tabs": 1.0.0 "@radix-ui/react-toggle-group": 1.0.0 "@testing-library/dom": 8.13.0 @@ -3542,11 +3542,11 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-tailwind@^5.7.3, @navikt/ds-tailwind@workspace:@navikt/core/tailwind": +"@navikt/ds-tailwind@^5.7.4, @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.7.3 + "@navikt/ds-tokens": ^5.7.4 "@types/jest": ^29.0.0 color: 4.2.3 jest: ^29.0.0 @@ -3557,7 +3557,7 @@ __metadata: languageName: unknown linkType: soft -"@navikt/ds-tokens@^5.7.3, @navikt/ds-tokens@workspace:@navikt/core/tokens": +"@navikt/ds-tokens@^5.7.4, @navikt/ds-tokens@workspace:@navikt/core/tokens": version: 0.0.0-use.local resolution: "@navikt/ds-tokens@workspace:@navikt/core/tokens" dependencies: @@ -8410,11 +8410,11 @@ __metadata: version: 0.0.0-use.local resolution: "aksel.nav.no@workspace:aksel.nav.no" dependencies: - "@navikt/aksel-icons": ^5.7.3 - "@navikt/ds-css": ^5.7.3 - "@navikt/ds-react": ^5.7.3 - "@navikt/ds-tailwind": ^5.7.3 - "@navikt/ds-tokens": ^5.7.3 + "@navikt/aksel-icons": ^5.7.4 + "@navikt/ds-css": ^5.7.4 + "@navikt/ds-react": ^5.7.4 + "@navikt/ds-tailwind": ^5.7.4 + "@navikt/ds-tokens": ^5.7.4 prettier-plugin-tailwindcss: ^0.2.3 languageName: unknown linkType: soft From fff5c515c949d0f4012356224588d809081b2ea5 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Tue, 24 Oct 2023 09:02:14 +0200 Subject: [PATCH 06/12] Force option id to be lower case, to avoid different casing from being interpreted as being different from the existing options --- .../form/combobox/FilteredOptions/filtered-options-util.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts b/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts index 5d472bccf44..32e9eb07267 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts @@ -14,7 +14,9 @@ const getFilteredOptionsId = (comboboxId: string) => `${comboboxId}-filtered-options`; const getOptionId = (comboboxId: string, option: string) => - `${comboboxId}-option-${option.replace(" ", "-")}`; + `${comboboxId.toLocaleLowerCase()}-option-${option + .replace(" ", "-") + .toLocaleLowerCase()}`; const getAddNewOptionId = (comboboxId: string) => `${comboboxId}-combobox-new-option`; From 7e52f2468f529d847f1f3df3d9b2872916fa4935 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Tue, 31 Oct 2023 09:57:34 +0100 Subject: [PATCH 07/12] Fix bug where it was not possible to select multiple custom (new) options in a multiselect selectedOptionsContext is not available inside customOptionsContext, so need to take isMultiSelect from props instead --- .../react/src/form/combobox/ComboboxProvider.tsx | 2 +- .../src/form/combobox/customOptionsContext.tsx | 15 ++++++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx b/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx index 87f59feb9f2..32e0bb68420 100644 --- a/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx +++ b/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx @@ -65,7 +65,7 @@ const ComboboxProvider = forwardRef( size, }} > - + ( {} as CustomOptionsContextType ); -export const CustomOptionsProvider = ({ children }) => { +export const CustomOptionsProvider = ({ + children, + value, +}: { + children: any; + value: { isMultiSelect: boolean }; +}) => { const [customOptions, setCustomOptions] = useState([]); const { focusInput } = useInputContext(); - const { isMultiSelect } = useSelectedOptionsContext(); + const { isMultiSelect } = value; const removeCustomOption = useCallback( - (option) => { + (option: string) => { setCustomOptions((prevCustomOptions) => prevCustomOptions.filter((o) => o !== option) ); @@ -29,7 +34,7 @@ export const CustomOptionsProvider = ({ children }) => { ); const addCustomOption = useCallback( - (option) => { + (option: string) => { if (isMultiSelect) { setCustomOptions((prevOptions) => [...prevOptions, option]); } else { From 148bdaeeb8d09c0d183fc307d485e92aecfc45d1 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Tue, 31 Oct 2023 10:43:17 +0100 Subject: [PATCH 08/12] Switch to use toLocalLowerCase and remove uneccessary use of string literal --- .../src/form/combobox/FilteredOptions/filtered-options-util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts b/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts index 32e9eb07267..f3a45e80021 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filtered-options-util.ts @@ -1,5 +1,5 @@ const normalizeText = (text: string): string => - typeof text === "string" ? `${text}`.toLowerCase().trim() : ""; + typeof text === "string" ? text.toLocaleLowerCase().trim() : ""; const isPartOfText = (value, text) => normalizeText(text).startsWith(normalizeText(value ?? "")); From 3bfb3bedcff3e71b183711bba2e75a2e0499e596 Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Tue, 31 Oct 2023 11:23:35 +0100 Subject: [PATCH 09/12] type mismatch --- @navikt/core/react/src/form/combobox/customOptionsContext.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/@navikt/core/react/src/form/combobox/customOptionsContext.tsx b/@navikt/core/react/src/form/combobox/customOptionsContext.tsx index d0f49ce1b4d..24c8fdc3e2e 100644 --- a/@navikt/core/react/src/form/combobox/customOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/customOptionsContext.tsx @@ -17,7 +17,7 @@ export const CustomOptionsProvider = ({ value, }: { children: any; - value: { isMultiSelect: boolean }; + value: { isMultiSelect?: boolean }; }) => { const [customOptions, setCustomOptions] = useState([]); const { focusInput } = useInputContext(); From c0d65ce7851469c1872cddf43e7ca0f76e69a68c Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Tue, 31 Oct 2023 11:49:56 +0100 Subject: [PATCH 10/12] Clean up types - reuse from ComboboxProps type definition, and fix resulting type mismatches --- .../src/form/combobox/ComboboxProvider.tsx | 1 - .../filteredOptionsContext.tsx | 26 ++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx b/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx index 32e0bb68420..12f5cef8b3e 100644 --- a/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx +++ b/@navikt/core/react/src/form/combobox/ComboboxProvider.tsx @@ -81,7 +81,6 @@ const ComboboxProvider = forwardRef( filteredOptions, isListOpen, isLoading, - isMultiSelect, options, }} > diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index a714d5eb694..183dbd103a8 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -13,6 +13,19 @@ import usePrevious from "../../../util/usePrevious"; import { useClientLayoutEffect } from "../../../util"; import filteredOptionsUtils from "./filtered-options-util"; import useVirtualFocus from "./useVirtualFocus"; +import { ComboboxProps } from "../types"; + +type FilteredOptionsProps = { + children: any; + value: Pick< + ComboboxProps, + | "allowNewValues" + | "filteredOptions" + | "isListOpen" + | "isLoading" + | "options" + >; +}; type FilteredOptionsContextType = { activeDecendantId?: string; @@ -28,7 +41,7 @@ type FilteredOptionsContextType = { setIsMouseLastUsedInputDevice: React.Dispatch>; isValueNew: boolean; toggleIsListOpen: (newState?: boolean) => void; - currentOption: string | null; + currentOption?: string; moveFocusUp: () => void; moveFocusDown: () => void; moveFocusToElement: (id: string) => void; @@ -40,7 +53,10 @@ const FilteredOptionsContext = createContext( {} as FilteredOptionsContextType ); -export const FilteredOptionsProvider = ({ children, value: props }) => { +export const FilteredOptionsProvider = ({ + children, + value: props, +}: FilteredOptionsProps) => { const { allowNewValues, filteredOptions: externalFilteredOptions, @@ -79,12 +95,14 @@ export const FilteredOptionsProvider = ({ children, value: props }) => { const filteredOptionsMap = useMemo( () => options.reduce( - (map: Map, _option: string) => ({ + (map, _option) => ({ ...map, [filteredOptionsUtils.getOptionId(id, _option)]: _option, }), { - [filteredOptionsUtils.getAddNewOptionId(id)]: allowNewValues && value, + [filteredOptionsUtils.getAddNewOptionId(id)]: allowNewValues + ? value + : undefined, } ), [allowNewValues, id, options, value] From 72edee9ecb138d0682fed99387f46a62bc97f16f Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Wed, 1 Nov 2023 10:49:28 +0100 Subject: [PATCH 11/12] Rename filteredOptionsRef variable returned from filteredOptionsContext to reflect that it is now actually a setState-function --- .../src/form/combobox/FilteredOptions/FilteredOptions.tsx | 4 ++-- .../form/combobox/FilteredOptions/filteredOptionsContext.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx index bb37e7dd9fe..fed35aeacb9 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx @@ -19,7 +19,7 @@ const FilteredOptions = () => { isLoading, isListOpen, filteredOptions, - filteredOptionsRef, + setFilteredOptionsRef, isMouseLastUsedInputDevice, setIsMouseLastUsedInputDevice, isValueNew, @@ -32,7 +32,7 @@ const FilteredOptions = () => { return (
      >; isListOpen: boolean; @@ -201,7 +201,7 @@ export const FilteredOptionsProvider = ({ const filteredOptionsState = { activeDecendantId, allowNewValues, - filteredOptionsRef: setFilteredOptionsRef, + setFilteredOptionsRef, shouldAutocomplete, isListOpen, isLoading, From 309669648d919e5121d9d5215ca9e711c07c31fe Mon Sep 17 00:00:00 2001 From: Vegard Haugstvedt Date: Wed, 1 Nov 2023 10:59:48 +0100 Subject: [PATCH 12/12] Expose virtualFocus as an object through filteredOptionsContext, instead of each separate method. Done to remove need to wrap moveFocusUp and moveFocusDown in order to toggle isListOpen --- .../FilteredOptions/FilteredOptions.tsx | 10 ++++-- .../filteredOptionsContext.tsx | 31 ++----------------- .../FilteredOptions/useVirtualFocus.ts | 19 ++++++++++-- .../react/src/form/combobox/Input/Input.tsx | 29 +++++++++-------- 4 files changed, 43 insertions(+), 46 deletions(-) diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx index fed35aeacb9..8c87e1a923d 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/FilteredOptions.tsx @@ -23,9 +23,9 @@ const FilteredOptions = () => { isMouseLastUsedInputDevice, setIsMouseLastUsedInputDevice, isValueNew, - moveFocusToElement, toggleIsListOpen, activeDecendantId, + virtualFocus, } = useFilteredOptionsContext(); const { isMultiSelect, selectedOptions, toggleOption } = useSelectedOptionsContext(); @@ -59,7 +59,9 @@ const FilteredOptions = () => { if ( activeDecendantId !== filteredOptionsUtil.getAddNewOptionId(id) ) { - moveFocusToElement(filteredOptionsUtil.getAddNewOptionId(id)); + virtualFocus.moveFocusToElement( + filteredOptionsUtil.getAddNewOptionId(id) + ); setIsMouseLastUsedInputDevice(true); } }} @@ -111,7 +113,9 @@ const FilteredOptions = () => { if ( activeDecendantId !== filteredOptionsUtil.getOptionId(id, option) ) { - moveFocusToElement(filteredOptionsUtil.getOptionId(id, option)); + virtualFocus.moveFocusToElement( + filteredOptionsUtil.getOptionId(id, option) + ); setIsMouseLastUsedInputDevice(true); } }} diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx index b33b732b169..d72961c04e0 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/filteredOptionsContext.tsx @@ -12,7 +12,7 @@ import { useInputContext } from "../Input/inputContext"; import usePrevious from "../../../util/usePrevious"; import { useClientLayoutEffect } from "../../../util"; import filteredOptionsUtils from "./filtered-options-util"; -import useVirtualFocus from "./useVirtualFocus"; +import useVirtualFocus, { VirtualFocusType } from "./useVirtualFocus"; import { ComboboxProps } from "../types"; type FilteredOptionsProps = { @@ -42,12 +42,8 @@ type FilteredOptionsContextType = { isValueNew: boolean; toggleIsListOpen: (newState?: boolean) => void; currentOption?: string; - moveFocusUp: () => void; - moveFocusDown: () => void; - moveFocusToElement: (id: string) => void; - moveFocusToInput: () => void; - moveFocusToEnd: () => void; shouldAutocomplete?: boolean; + virtualFocus: VirtualFocusType; }; const FilteredOptionsContext = createContext( {} as FilteredOptionsContextType @@ -170,29 +166,12 @@ export const FilteredOptionsProvider = ({ id, ]); - // TODO: Re-write or remove after re-write? const currentOption = useMemo( () => filteredOptionsMap[virtualFocus.activeElement?.getAttribute("id") || -1], [filteredOptionsMap, virtualFocus] ); - // TODO: Can be deleted if we move toggleIsListOpen(false) to the event handling in Input.tsx - const moveFocusUp = useCallback(() => { - if (virtualFocus.isFocusOnTheTop) { - toggleIsListOpen(false); - } - virtualFocus.moveFocusUp(); - }, [toggleIsListOpen, virtualFocus]); - - // TODO: Can be deleted if we move toggleIsListOpen(true) to the event handling in Input.tsx - const moveFocusDown = useCallback(() => { - if (virtualFocus.activeElement === null || !isListOpen) { - toggleIsListOpen(true); - } - virtualFocus.moveFocusDown(); - }, [isListOpen, toggleIsListOpen, virtualFocus]); - const activeDecendantId = useMemo( () => virtualFocus.activeElement?.getAttribute("id") || undefined, [virtualFocus.activeElement] @@ -211,11 +190,7 @@ export const FilteredOptionsProvider = ({ isValueNew, toggleIsListOpen, currentOption, - moveFocusUp, - moveFocusDown, - moveFocusToElement: virtualFocus.moveFocusToElement, - moveFocusToInput: virtualFocus.moveFocusToTop, - moveFocusToEnd: virtualFocus.moveFocusToBottom, + virtualFocus, ariaDescribedBy, }; diff --git a/@navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts b/@navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts index 5d58c262f6a..3df5b0f1b0a 100644 --- a/@navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts +++ b/@navikt/core/react/src/form/combobox/FilteredOptions/useVirtualFocus.ts @@ -1,6 +1,21 @@ -import { useState } from "react"; +import { Dispatch, SetStateAction, useState } from "react"; -const useVirtualFocus = (containerRef: HTMLElement | null) => { +export type VirtualFocusType = { + activeElement: HTMLElement | undefined; + getElementById: (id: string) => HTMLElement | undefined; + isFocusOnTheTop: boolean; + isFocusOnTheBottom: boolean; + setIndex: Dispatch>; + moveFocusUp: () => void; + moveFocusDown: () => void; + moveFocusToElement: (id: string) => void; + moveFocusToTop: () => void; + moveFocusToBottom: () => void; +}; + +const useVirtualFocus = ( + containerRef: HTMLElement | null +): VirtualFocusType => { const [index, setIndex] = useState(-1); const listOfAllChildren: Array = containerRef?.children diff --git a/@navikt/core/react/src/form/combobox/Input/Input.tsx b/@navikt/core/react/src/form/combobox/Input/Input.tsx index 16562a6cf9e..3b5f74cd379 100644 --- a/@navikt/core/react/src/form/combobox/Input/Input.tsx +++ b/@navikt/core/react/src/form/combobox/Input/Input.tsx @@ -35,13 +35,10 @@ const Input = forwardRef( isValueNew, toggleIsListOpen, isListOpen, - moveFocusUp, - moveFocusDown, ariaDescribedBy, - moveFocusToInput, - moveFocusToEnd, setIsMouseLastUsedInputDevice, shouldAutocomplete, + virtualFocus, } = useFilteredOptionsContext(); const onEnter = useCallback( @@ -104,10 +101,10 @@ const Input = forwardRef( onEnter(e); break; case "Home": - moveFocusToInput(); + virtualFocus.moveFocusToTop(); break; case "End": - moveFocusToEnd(); + virtualFocus.moveFocusToBottom(); break; default: break; @@ -128,14 +125,20 @@ const Input = forwardRef( // so we don't interfere with text editing if (e.target.selectionStart === value?.length) { e.preventDefault(); - moveFocusDown(); + if (virtualFocus.activeElement === null || !isListOpen) { + toggleIsListOpen(true); + } + virtualFocus.moveFocusDown(); } } else if (e.key === "ArrowUp") { // Check that the FilteredOptions list is open and has virtual focus. // Otherwise ignore keystrokes, so it doesn't interfere with text editing if (isListOpen && activeDecendantId) { e.preventDefault(); - moveFocusUp(); + if (virtualFocus.isFocusOnTheTop) { + toggleIsListOpen(false); + } + virtualFocus.moveFocusUp(); } } }, @@ -143,11 +146,11 @@ const Input = forwardRef( value, selectedOptions, removeSelectedOption, - moveFocusDown, isListOpen, activeDecendantId, - moveFocusUp, setIsMouseLastUsedInputDevice, + toggleIsListOpen, + virtualFocus, ] ); @@ -159,14 +162,14 @@ const Input = forwardRef( } else if (filteredOptions.length === 0) { toggleIsListOpen(false); } - moveFocusToInput(); + virtualFocus.moveFocusToTop(); onChange(event); }, - [filteredOptions.length, moveFocusToInput, onChange, toggleIsListOpen] + [filteredOptions.length, virtualFocus, onChange, toggleIsListOpen] ); const onBlur = () => { - moveFocusToInput(); + virtualFocus.moveFocusToTop(); }; return (