From 5f7e792e28d4544adc53b8602b3736ca579ff0cf Mon Sep 17 00:00:00 2001 From: Nicholas Boll Date: Fri, 11 Oct 2024 22:44:49 -0600 Subject: [PATCH 1/6] refactor(select): Fix all syncing issues with the Select component --- .../collection/lib/useSelectionListModel.tsx | 6 + modules/react/combobox/index.ts | 21 ++- modules/react/combobox/lib/hooks/index.ts | 9 - .../combobox/lib/hooks/useComboboxInput.ts | 4 +- .../lib/hooks/useComboboxInputConstrained.ts | 157 ++++++++++++++++++ .../lib/hooks/useComboboxKeyboardTypeAhead.ts | 28 ++-- .../combobox/lib/hooks/useComboboxModel.tsx | 10 +- modules/react/select/lib/Select.tsx | 34 +--- .../react/select/lib/hooks/useSelectInput.ts | 121 ++------------ modules/react/select/stories/Select.mdx | 11 ++ .../react/select/stories/Select.stories.ts | 4 + .../select/stories/examples/Controlled.tsx | 68 ++++++++ 12 files changed, 292 insertions(+), 181 deletions(-) delete mode 100644 modules/react/combobox/lib/hooks/index.ts create mode 100644 modules/react/combobox/lib/hooks/useComboboxInputConstrained.ts create mode 100644 modules/react/select/stories/examples/Controlled.tsx diff --git a/modules/react/collection/lib/useSelectionListModel.tsx b/modules/react/collection/lib/useSelectionListModel.tsx index 0d8976a57f..b03f5bcec4 100644 --- a/modules/react/collection/lib/useSelectionListModel.tsx +++ b/modules/react/collection/lib/useSelectionListModel.tsx @@ -102,6 +102,12 @@ export const useSelectionListModel = createModelHook({ unselectAll() { setSelectedIds([]); }, + /** + * Should be used with care and can be used to keep a model in sync with external controlled + * inputs. + */ setSelectedIds(ids: 'all' | string[]) { + setSelectedIds(ids); + }, }; return {...cursor, state, events, selection}; diff --git a/modules/react/combobox/index.ts b/modules/react/combobox/index.ts index f42dfdcab1..ee6de31dfa 100644 --- a/modules/react/combobox/index.ts +++ b/modules/react/combobox/index.ts @@ -2,14 +2,13 @@ export {Combobox} from './lib/Combobox'; export {useComboboxCard} from './lib/ComboboxCard'; export {useComboboxMenuItem} from './lib/ComboboxMenuItem'; export {useComboboxMenuList} from './lib/ComboboxMenuList'; -export { - useComboboxLoader, - useComboboxModel, - useComboboxInputOpenWithArrowKeys, - useComboboxKeyboardTypeAhead, - useSetPopupWidth, - useComboboxMoveCursorToSelected, - useComboboxInput, - useComboboxListKeyboardHandler, - useComboboxResetCursorToSelected, -} from './lib/hooks'; +export * from './lib/hooks/useComboboxInputOpenWithArrowKeys'; +export * from './lib/hooks/useComboboxLoader'; +export * from './lib/hooks/useComboboxModel'; +export * from './lib/hooks/useComboboxMoveCursorToSelected'; +export * from './lib/hooks/useSetPopupWidth'; +export * from './lib/hooks/useComboboxKeyboardTypeAhead'; +export * from './lib/hooks/useComboboxListKeyboardHandler'; +export * from './lib/hooks/useComboboxInput'; +export * from './lib/hooks/useComboboxInputConstrained'; +export * from './lib/hooks/useComboboxResetCursorToSelected'; diff --git a/modules/react/combobox/lib/hooks/index.ts b/modules/react/combobox/lib/hooks/index.ts deleted file mode 100644 index e19a0d6a95..0000000000 --- a/modules/react/combobox/lib/hooks/index.ts +++ /dev/null @@ -1,9 +0,0 @@ -export * from './useComboboxInputOpenWithArrowKeys'; -export * from './useComboboxLoader'; -export * from './useComboboxModel'; -export * from './useComboboxMoveCursorToSelected'; -export * from './useSetPopupWidth'; -export * from './useComboboxKeyboardTypeAhead'; -export * from './useComboboxListKeyboardHandler'; -export * from './useComboboxInput'; -export * from './useComboboxResetCursorToSelected'; diff --git a/modules/react/combobox/lib/hooks/useComboboxInput.ts b/modules/react/combobox/lib/hooks/useComboboxInput.ts index 1f168c1aee..3028408604 100644 --- a/modules/react/combobox/lib/hooks/useComboboxInput.ts +++ b/modules/react/combobox/lib/hooks/useComboboxInput.ts @@ -70,14 +70,14 @@ export const useComboboxInput = composeHooks( }, value: model.state.value, role: 'combobox', - 'aria-haspopup': 'true' as React.AriaAttributes['aria-haspopup'], + 'aria-haspopup': 'listbox', 'aria-expanded': model.state.visibility === 'visible', 'aria-autocomplete': 'list', 'aria-controls': `${model.state.id}-list`, 'aria-activedescendant': model.state.visibility === 'hidden' ? null : undefined, // Removes activedescendant on menu close id: model.state.id, ref: elementRef, - } as const; + }; }), useSetPopupWidth, useComboboxInputOpenWithArrowKeys, diff --git a/modules/react/combobox/lib/hooks/useComboboxInputConstrained.ts b/modules/react/combobox/lib/hooks/useComboboxInputConstrained.ts new file mode 100644 index 0000000000..793af274c0 --- /dev/null +++ b/modules/react/combobox/lib/hooks/useComboboxInputConstrained.ts @@ -0,0 +1,157 @@ +import React from 'react'; +import { + createElemPropsHook, + useLocalRef, + dispatchInputEvent, +} from '@workday/canvas-kit-react/common'; +import {useComboboxModel} from './useComboboxModel'; + +function onlyDefined(input: T | undefined): input is T { + return !!input; +} + +/** + * A constrained combobox input can only offer values that are part of the provided list of `items`. + * The default is an unconstrained. A constrained input should have both a form input that is hidden + * from the user as well as a user input that will be visible to the user. This hook is in charge of + * keeping the inputs and the model in sync with each other and working with a browser's + * autocomplete, form libraries and the model. + */ +export const useComboboxInputConstrained = createElemPropsHook(useComboboxModel)( + ( + model, + ref, + { + disabled, + value: reactValue, + onChange, + name, + }: Pick< + React.InputHTMLAttributes, + 'disabled' | 'value' | 'onChange' | 'name' + > = {} + ) => { + // The user element is what the user sees + const {elementRef: userElementRef, localRef: userLocalRef} = useLocalRef( + model.state.targetRef as React.Ref + ); + + // The form element is what is seen in `FormData` during for submission to the server + const {elementRef: formElementRef, localRef: formLocalRef} = useLocalRef( + ref as React.Ref + ); + + // Create React refs so we can get the current value inside an Effect without using those values + // as part of the dependency array. + const modelNavigationRef = React.useRef(model.navigation); + modelNavigationRef.current = model.navigation; + const modelStateRef = React.useRef(model.state); + modelStateRef.current = model.state; + + // Watch the `value` prop passed from React props and update the model accordingly + React.useLayoutEffect(() => { + if (formLocalRef.current && typeof reactValue === 'string') { + // const value = formLocalRef.current.value; + if (reactValue !== formLocalRef.current.value) { + model.events.setSelectedIds(reactValue ? reactValue.split(', ') : []); + } + } + }, [reactValue, formLocalRef, model.events]); + + // useImperativeHandle allows us to modify the `ref` before it is sent to the application, + // but after it is defined. We can add value watches, and redirect methods here. + React.useImperativeHandle( + formElementRef, + () => { + if (formLocalRef.current) { + // Hook into the DOM `value` property of the form input element and update the model + // accordingly + Object.defineProperty(formLocalRef.current, 'value', { + get() { + const value = Object.getOwnPropertyDescriptor( + Object.getPrototypeOf(formLocalRef.current), + 'value' + )?.get?.call(formLocalRef.current); + return value; + }, + set(value: string) { + if ( + formLocalRef.current && + value !== + (modelStateRef.current.selectedIds === 'all' + ? [] + : modelStateRef.current.selectedIds + ).join(', ') + ) { + model.events.setSelectedIds(value ? value.split(', ') : []); + } + }, + }); + + // forward calls to `.focus()` and `.blur()` to the user input + formLocalRef.current.focus = (options?: FocusOptions) => { + userLocalRef.current!.focus(options); + }; + formLocalRef.current.blur = () => { + userLocalRef.current!.blur(); + }; + } + return formLocalRef.current!; + }, + [formLocalRef, userLocalRef, model.events] + ); + + // sync model selection state with inputs + React.useLayoutEffect(() => { + if (userLocalRef.current) { + const userValue = + model.state.items.length === 0 + ? '' + : (model.state.selectedIds === 'all' + ? [] + : model.state.selectedIds + .map(id => + modelNavigationRef.current.getItem(id, {state: modelStateRef.current}) + ) + .filter(onlyDefined) + .map(item => item.textValue) + ).join(', '); + + if (userValue !== userLocalRef.current.value) { + dispatchInputEvent(userLocalRef.current, userValue); + } + } + + if (formLocalRef.current) { + const formValue = (model.state.selectedIds === 'all' ? [] : model.state.selectedIds).join( + ', ' + ); + + if (formValue !== formLocalRef.current.value) { + dispatchInputEvent(formLocalRef.current, formValue); + } + } + }, [model.state.selectedIds, model.state.items, formLocalRef, userLocalRef]); + + // The props here will go to the user input. + return { + ref: userElementRef, + form: '', // We don't want the user input to be part of the form [elements](https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/elements) + value: null, + onChange: null, + name: null, + disabled, + /** + * These props should be spread onto the form input. + */ + formInputProps: { + disabled, + tabIndex: -1, + 'aria-hidden': true, + ref: formElementRef, + onChange, + name, + }, + }; + } +); diff --git a/modules/react/combobox/lib/hooks/useComboboxKeyboardTypeAhead.ts b/modules/react/combobox/lib/hooks/useComboboxKeyboardTypeAhead.ts index 1029f303b1..2c69d4dd09 100644 --- a/modules/react/combobox/lib/hooks/useComboboxKeyboardTypeAhead.ts +++ b/modules/react/combobox/lib/hooks/useComboboxKeyboardTypeAhead.ts @@ -9,7 +9,7 @@ import {useComboboxModel} from './useComboboxModel'; * ```tsx * // Example for a `Select` input * export const useSelectInput = composeHooks( - * createElemPropsHook(useComboboxModel)((model, ref, elemProps: {keySofar?: string} = {}) => { + * createElemPropsHook(useComboboxModel)((model, ref, elemProps: {keySoFar?: string} = {}) => { * return { * onKeyDown(event: React.KeyboardEvent) { * // onKeyDown logic ... @@ -22,17 +22,17 @@ import {useComboboxModel} from './useComboboxModel'; * ``` */ export const useComboboxKeyboardTypeAhead = createElemPropsHook(useComboboxModel)(model => { - const keySofar = React.useRef(''); + const keySoFar = React.useRef(''); const timer = React.useRef>(); - const [keyTypedSofar, setKeyTypedSofar] = React.useState(keySofar.current); + const [keyTypedSofar, setKeyTypedSofar] = React.useState(keySoFar.current); const startClearKeysSoFarTimer = () => { if (timer.current) { clearTimeout(timer.current); } timer.current = setTimeout(() => { - keySofar.current = ''; - setKeyTypedSofar(keySofar.current); + keySoFar.current = ''; + setKeyTypedSofar(keySoFar.current); }, 500); }; @@ -65,24 +65,24 @@ export const useComboboxKeyboardTypeAhead = createElemPropsHook(useComboboxModel const handleKeyboardTypeAhead = (key: string, numOptions: number) => { // If the starting point is beyond the list of options, reset it // to the beginning of the list - const startNumber = keySofar.current.length === 0 ? currentItemIndex + 1 : currentItemIndex; + const startNumber = keySoFar.current.length === 0 ? currentItemIndex + 1 : currentItemIndex; const start = startNumber === numOptions ? 0 : startNumber; // Keeps track of the current key types and adds to it // if you type `de` vs `d` for denver - keySofar.current += key; - setKeyTypedSofar(keySofar.current); + keySoFar.current += key; + setKeyTypedSofar(keySoFar.current); startClearKeysSoFarTimer(); // First, look for a match from start to end - let matchIndex = getIndexByStartString(start, keySofar.current); - getIndexByStartString(start, keySofar.current); + let matchIndex = getIndexByStartString(start, keySoFar.current); + getIndexByStartString(start, keySoFar.current); // If a match isn't found between start and end, wrap the search // around and search again from the beginning (0) to start if (matchIndex === -1) { - matchIndex = getIndexByStartString(0, keySofar.current, start); + matchIndex = getIndexByStartString(0, keySoFar.current, start); } // A match was found... @@ -107,7 +107,7 @@ export const useComboboxKeyboardTypeAhead = createElemPropsHook(useComboboxModel if ( (event.key === 'Spacebar' || event.key === ' ') && model.state.visibility === 'visible' && - keySofar.current !== '' + keySoFar.current !== '' ) { // If the user is in the middle of typing a string, treat // space key as type-ahead rather than option selection @@ -117,13 +117,13 @@ export const useComboboxKeyboardTypeAhead = createElemPropsHook(useComboboxModel if ( (event.key === 'Spacebar' || event.key === ' ') && model.state.visibility === 'hidden' && - keySofar.current !== '' + keySoFar.current !== '' ) { // If the user is in the middle of typing a string, treat // space key as type-ahead rather than option selection handleKeyboardTypeAhead(' ', model.state.items.length); } }, - keySofar: keyTypedSofar, + keySoFar: keyTypedSofar, }; }); diff --git a/modules/react/combobox/lib/hooks/useComboboxModel.tsx b/modules/react/combobox/lib/hooks/useComboboxModel.tsx index dbd4af90f4..e590298fe5 100644 --- a/modules/react/combobox/lib/hooks/useComboboxModel.tsx +++ b/modules/react/combobox/lib/hooks/useComboboxModel.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import {createModelHook, dispatchInputEvent} from '@workday/canvas-kit-react/common'; +import {createModelHook} from '@workday/canvas-kit-react/common'; import {useMenuModel} from '@workday/canvas-kit-react/menu'; /** @@ -53,13 +53,7 @@ export const useComboboxModel = createModelHook({ })(config => { const input = useInputModel(config); - const menu = useMenuModel( - useMenuModel.mergeConfig(config, { - onSelect({id}) { - dispatchInputEvent(menu.state.targetRef.current, id); - }, - }) - ); + const menu = useMenuModel(config); const [width, setWidth] = React.useState(0); diff --git a/modules/react/select/lib/Select.tsx b/modules/react/select/lib/Select.tsx index 183e53ffd7..a6e521007b 100644 --- a/modules/react/select/lib/Select.tsx +++ b/modules/react/select/lib/Select.tsx @@ -19,11 +19,6 @@ export interface SelectInputProps extends ExtractProps, CSProp * ** Note:An option must be selected in order to render and icon.** */ inputStartIcon?: CanvasSystemIcon; - readonly textInputProps?: { - ref: React.Ref; - onChange: () => {}; - value: string; - }; } const selectInputStencil = createStencil({ @@ -61,20 +56,7 @@ export const SelectInput = createSubcomponent(TextInput)({ elemPropsHook: useSelectInput, })( ( - { - placeholder = 'Choose an option', - inputStartIcon, - error, - textInputProps, - disabled, - width, - ref, - onChange, - onInput, - value, - name, - ...elemProps - }, + {placeholder = 'Choose an option', inputStartIcon, formInputProps, width, ...elemProps}, Element, model ) => { @@ -87,26 +69,16 @@ export const SelectInput = createSubcomponent(TextInput)({ )} {/* Hidden input to handle ids */} {/* Visual input */} diff --git a/modules/react/select/lib/hooks/useSelectInput.ts b/modules/react/select/lib/hooks/useSelectInput.ts index 3d17c855d3..41d7f1aa97 100644 --- a/modules/react/select/lib/hooks/useSelectInput.ts +++ b/modules/react/select/lib/hooks/useSelectInput.ts @@ -4,75 +4,29 @@ import { createElemPropsHook, useLocalRef, useResizeObserver, - dispatchInputEvent, } from '@workday/canvas-kit-react/common'; import { useComboboxInput, + useComboboxInputConstrained, useComboboxKeyboardTypeAhead, useComboboxMoveCursorToSelected, useComboboxResetCursorToSelected, } from '@workday/canvas-kit-react/combobox'; import {useSelectModel} from './useSelectModel'; -function noop() { - // Do nothing -} - /** * `useSelectInput` extends {@link useComboboxInput useComboboxInput} and {@link useComboboxKeyboardTypeAhead useComboboxKeyboardTypeAhead} and adds type ahead functionality and Select-specific [keyboard support](https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only/). */ export const useSelectInput = composeHooks( - useComboboxInput, - useComboboxKeyboardTypeAhead, - useComboboxResetCursorToSelected, - useComboboxMoveCursorToSelected, createElemPropsHook(useSelectModel)( - (model, ref, elemProps: {keySofar?: string; placeholder?: string; value?: string} = {}) => { - const {elementRef: textInputElementRef, localRef: textInputRef} = useLocalRef( + (model, _ref, elemProps: {keySoFar?: string; placeholder?: string; value?: string} = {}) => { + const {elementRef, localRef} = useLocalRef( // PopupModel says the targetRef is a `HTMLButtonElement`, but it is a `HTMLInputElement` model.state.targetRef as any as React.Ref ); - const {localRef: hiddenLocalRef, elementRef: hiddenElementRef} = useLocalRef( - ref as React.Ref - ); - - // We need to create a proxy between the multiple inputs. We need to redirect a few methods to - // the visible input - React.useImperativeHandle( - hiddenElementRef, - () => { - if (hiddenLocalRef.current) { - hiddenLocalRef.current.focus = (options?: FocusOptions) => { - textInputRef.current!.focus(options); - }; - hiddenLocalRef.current.blur = () => { - textInputRef.current!.blur(); - }; - } - - return hiddenLocalRef.current!; - }, - [textInputRef, hiddenLocalRef] - ); - - // Remap the Popup model's targetRef to be the visible ref. `ref` and `model.state.targetRef` are already linked. We have to override that. - - // Update the text value of the input - const handleOnChange = (event: React.ChangeEvent) => { - const value = model.navigation.getItem(event.target.value, model)?.textValue; - const nativeInputValue = Object.getOwnPropertyDescriptor( - Object.getPrototypeOf(textInputRef.current), - 'value' - ); - - if (nativeInputValue && nativeInputValue.set) { - nativeInputValue.set.call(textInputRef.current, value); - } - }; - useResizeObserver({ - ref: textInputRef, + ref: localRef, onResize: data => { if (model.state.visibility === 'visible') { // Width of the Input + 2px border + 8px padding @@ -81,42 +35,6 @@ export const useSelectInput = composeHooks( } }, }); - // The intent is if items are loaded after the component is rendered, it will update the input with the value. - // **Note: We might need to watch for other things or how often we should do this** - React.useEffect(() => { - if ( - model.state.inputRef.current && - model.state.items.length > 0 && - model.state.selectedIds[0] - ) { - const value = model.state.selectedIds[0]; - const oldValue = model.state.inputRef.current.value; - - // force the hidden input to have the correct value - if (model.state.inputRef.current.value !== value) { - const nativeInputValue = Object.getOwnPropertyDescriptor( - Object.getPrototypeOf(model.state.inputRef.current), - 'value' - ); - - if (nativeInputValue && nativeInputValue.set) { - nativeInputValue.set.call(model.state.inputRef.current, value); - } - } - - if ( - model.state.selectedIds[0] !== oldValue && - model.state.inputRef.current.value !== oldValue - ) { - // Programmatically dispatch an onChange once items are loaded. This account for when a consumer wants an initial selected item and they're loading them from a server. - dispatchInputEvent(model.state.inputRef.current, value); - } - } - if (!model.state.selectedIds[0] && textInputRef.current?.value) { - dispatchInputEvent(textInputRef.current, ''); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [model.state.inputRef, model.state.items.length]); // This effect is a copy of what is in useComboboxInput. In this case, we need access to `textInputRef` instead of `model.state.inputRef` // since it points to the visual input and not the hidden input. This allows scroll to index to work @@ -126,7 +44,7 @@ export const useSelectInput = composeHooks( if (model.state.isVirtualized && item) { model.state.UNSTABLE_virtual.scrollToIndex(item.index); } else { - const listboxId = textInputRef.current?.getAttribute('aria-controls'); + const listboxId = localRef.current?.getAttribute('aria-controls'); if (listboxId) { const menuItem = document.querySelector( `[id="${listboxId}"] [data-id="${model.state.cursorId}"]` @@ -154,7 +72,7 @@ export const useSelectInput = composeHooks( // Select should open if Spacebar is typed and nothing has been typed AND the menu is hidden if ( event.key === 'Spacebar' || - (event.key === ' ' && model.state.visibility === 'hidden' && elemProps?.keySofar === '') + (event.key === ' ' && model.state.visibility === 'hidden' && elemProps?.keySoFar === '') ) { model.events.show(); } @@ -164,7 +82,7 @@ export const useSelectInput = composeHooks( model.state.visibility === 'visible' ) { // If key so far is empty, they're done typing, select the item where the cursor is located and hide the menu - if (elemProps?.keySofar === '') { + if (elemProps?.keySoFar === '') { model.events.select({ id: model.state.cursorId, }); @@ -172,24 +90,15 @@ export const useSelectInput = composeHooks( } } }, - onChange: handleOnChange, autoComplete: 'off', - ref: hiddenElementRef, - // When the hidden input is focused, we want to show the focus/hover states of the input that sits below it. - onFocus() { - textInputRef.current?.focus(); - }, - textInputProps: { - ref: textInputElementRef, - onChange: noop, - value: - model.state.selectedIds.length > 0 && model.state.items.length > 0 - ? model.navigation.getItem(model.state.selectedIds[0], model)?.textValue || '' - : '', - }, - 'aria-haspopup': 'menu', keySoFar: null, - } as const; + ref: elementRef, + }; } - ) + ), + useComboboxKeyboardTypeAhead, + useComboboxResetCursorToSelected, + useComboboxMoveCursorToSelected, + useComboboxInputConstrained, + useComboboxInput ); diff --git a/modules/react/select/stories/Select.mdx b/modules/react/select/stories/Select.mdx index 19de063ec8..74d03923e5 100644 --- a/modules/react/select/stories/Select.mdx +++ b/modules/react/select/stories/Select.mdx @@ -7,6 +7,7 @@ import { import {Alert} from './examples/Alert'; import {Basic} from './examples/Basic'; import {Complex} from './examples/Complex'; +import {Controlled} from './examples/Controlled'; import {Disabled} from './examples/Disabled'; import {DisabledOptions} from './examples/DisabledOption'; import {Error} from './examples/Error'; @@ -250,6 +251,16 @@ const items = [ ; ``` +### Controlled + +The Select can be a +[controlled input](https://react.dev/reference/react-dom/components/input#controlling-an-input-with-a-state-variable) +component by passing the `value` and `onChange` to either the ` item.serverId} getTextValue={item => item.label}> + + + + {item => {item.label}} + + + + + +

Id: {value}

+

Label: {label}

+ + { + setValue('fax'); + }} + > + Set to "Fax" + + { + setValue(''); + }} + > + Clear + + + + ); +}; From 16dc74ca224fb9dba89f9727501f5ede8b75820e Mon Sep 17 00:00:00 2001 From: Nicholas Boll Date: Sat, 12 Oct 2024 00:29:44 -0600 Subject: [PATCH 2/6] Updated upgrade guide --- modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx b/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx index be76be09cb..3b75813948 100644 --- a/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx +++ b/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx @@ -457,7 +457,8 @@ to set the appropiate aria attributes and `id` on the underlying input element. ### Select -**PR:** [#2865](https://github.com/Workday/canvas-kit/pull/2865) +**PRs:** [#2865](https://github.com/Workday/canvas-kit/pull/2865), +[#2983](https://github.com/Workday/canvas-kit/pull/2983) As we transition to use our CSS tokens to provide better theming capabilities and our new styling methods, we're moving away from components extending `Themeable`. `Themeable` has been removed from @@ -504,6 +505,9 @@ const theme: PartialEmotionCanvasTheme = { ; ``` +`Select` has been refactored to use `useComboboxInputConstrained` to sync the model and the `input` +element regardless of the source. This makes the `Select` be a controllable component. + ### Text Area **PR:** [#2825](https://github.com/Workday/canvas-kit/pull/2825) From b10addb76da69d8def8bf9727ab4638c94222e7c Mon Sep 17 00:00:00 2001 From: Nicholas Boll Date: Sat, 12 Oct 2024 22:16:22 -0600 Subject: [PATCH 3/6] Fix: Add arbitrary combobox input hook and fix Autocomplete --- cypress/component/Autocomplete.spec.tsx | 4 +- modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx | 31 ++++++++ modules/react/combobox/index.ts | 1 + .../lib/hooks/useComboboxInputArbitrary.ts | 32 ++++++++ modules/react/combobox/stories/Combobox.mdx | 37 +++++++++- .../stories/examples/Autocomplete.tsx | 74 ++++++++++--------- modules/react/common/lib/utils/components.ts | 7 ++ modules/react/common/spec/components.spec.tsx | 4 +- modules/styling/lib/cs.ts | 29 ++++---- 9 files changed, 167 insertions(+), 52 deletions(-) create mode 100644 modules/react/combobox/lib/hooks/useComboboxInputArbitrary.ts diff --git a/cypress/component/Autocomplete.spec.tsx b/cypress/component/Autocomplete.spec.tsx index ba912a5ddd..e4cd6dd228 100644 --- a/cypress/component/Autocomplete.spec.tsx +++ b/cypress/component/Autocomplete.spec.tsx @@ -19,8 +19,8 @@ describe('Autocomplete', () => { cy.mount(); }); - it('should have aria-haspopup set to true', () => { - cy.findByRole('combobox').should('have.attr', 'aria-haspopup', 'true'); + it('should have aria-haspopup set to listbox', () => { + cy.findByRole('combobox').should('have.attr', 'aria-haspopup', 'listbox'); }); it('should have aria-expanded set to false', () => { diff --git a/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx b/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx index 3b75813948..a1b1c1a361 100644 --- a/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx +++ b/modules/docs/mdx/12.0-UPGRADE-GUIDE.mdx @@ -45,6 +45,7 @@ A note to the reader: - [Component Updates](#component-updates) - [Styling API and CSS Tokens](#styling-api-and-css-tokens) - [Avatar](#avatar) + - [Combobox](#combmbox) - [Form Field](#form-field) - [Form Field Group](#form-field-group) - [Form Field Field](#form-field-field) @@ -252,6 +253,36 @@ The following changes have been made to the API: > union types. You will see a console warn message while in development if you're still using this. > Please update your types and usage before v13. +### Combobox + +**PR** [#2983](https://github.com/Workday/canvas-kit/pull/2983) + +The `useComboboxInput` hook, and the `Combobox.Input` component used to automatically update the +input when an option was selected. This functionality has been split between +`useComboboxInputArbitrary` and `useComboboxInputConstrained` depending on the combobox's value +type. The `useComboboxInput` had the arbitrary functionality built in. This has been separated out. +The `