diff --git a/CHANGELOG.md b/CHANGELOG.md index e8414b8ad0..752eaaeb13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `Combobox` component ([#1047](https://github.com/tailwindlabs/headlessui/pull/1047)) +- Add `Combobox` component ([#1047](https://github.com/tailwindlabs/headlessui/pull/1047), [#1099](https://github.com/tailwindlabs/headlessui/pull/1099)) ## [Unreleased - @headlessui/vue] @@ -30,7 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `Combobox` component ([#1047](https://github.com/tailwindlabs/headlessui/pull/1047)) +- Add `Combobox` component ([#1047](https://github.com/tailwindlabs/headlessui/pull/1047), [#1099](https://github.com/tailwindlabs/headlessui/pull/1099)) ## [@headlessui/react@v1.4.3] - 2022-01-14 diff --git a/package.json b/package.json index 89c1170fce..18d9b4a9fa 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ } }, "lint-staged": { - "*": "yarn lint-check" + "*": "yarn lint" }, "prettier": { "printWidth": 100, diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx index 934fb85afe..b6f22126f8 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx @@ -3588,19 +3588,26 @@ describe('Mouse interactions', () => { }) ) - it( + // TODO: JSDOM doesn't quite work here + // Clicking outside on the body should fire a mousedown (which it does) and then change the active element (which it doesn't) + xit( 'should be possible to click outside of the combobox which should close the combobox', suppressConsoleLogs(async () => { render( - - - Trigger - - alice - bob - charlie - - + <> + + + Trigger + + alice + bob + charlie + + +
+ after +
+ ) // Open combobox @@ -3609,13 +3616,13 @@ describe('Mouse interactions', () => { assertActiveElement(getComboboxInput()) // Click something that is not related to the combobox - await click(document.body) + await click(getByText('after')) // Should be closed now assertComboboxList({ state: ComboboxState.InvisibleUnmounted }) - // Verify the input is focused again - assertActiveElement(getComboboxInput()) + // Verify the button is focused + assertActiveElement(getByText('after')) }) ) @@ -4130,4 +4137,68 @@ describe('Mouse interactions', () => { assertNoActiveComboboxOption() }) ) + + it( + 'Combobox preserves the latest known active option after an option becomes inactive', + suppressConsoleLogs(async () => { + render( + + {({ open, latestActiveOption }) => ( + <> + + Trigger +
{latestActiveOption}
+ {open && ( + + Option A + Option B + Option C + + )} + + )} +
+ ) + + assertComboboxButton({ + state: ComboboxState.InvisibleUnmounted, + attributes: { id: 'headlessui-combobox-button-2' }, + }) + assertComboboxList({ state: ComboboxState.InvisibleUnmounted }) + + await click(getComboboxButton()) + + assertComboboxButton({ + state: ComboboxState.Visible, + attributes: { id: 'headlessui-combobox-button-2' }, + }) + assertComboboxList({ state: ComboboxState.Visible }) + + let options = getComboboxOptions() + + // Hover the first item + await mouseMove(options[0]) + + // Verify that the first combobox option is active + assertActiveComboboxOption(options[0]) + expect(document.getElementById('latestActiveOption')!.textContent).toBe('a') + + // Focus the second item + await mouseMove(options[1]) + + // Verify that the second combobox option is active + assertActiveComboboxOption(options[1]) + expect(document.getElementById('latestActiveOption')!.textContent).toBe('b') + + // Move the mouse off of the second combobox option + await mouseLeave(options[1]) + await mouseMove(document.body) + + // Verify that the second combobox option is NOT active + assertNoActiveComboboxOption() + + // But the last known active option is still recorded + expect(document.getElementById('latestActiveOption')!.textContent).toBe('b') + }) + ) }) diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 3aae0f7437..0dc2f80329 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -16,6 +16,7 @@ import React, { MutableRefObject, Ref, ContextType, + useEffect, } from 'react' import { useDisposables } from '../../hooks/use-disposables' @@ -30,7 +31,6 @@ import { disposables } from '../../utils/disposables' import { Keys } from '../keyboard' import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index' import { isDisabledReactIssue7711 } from '../../utils/bugs' -import { isFocusableElement, FocusableMode } from '../../utils/focus-management' import { useWindowEvent } from '../../hooks/use-window-event' import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-closed' import { useResolveButtonType } from '../../hooks/use-resolve-button-type' @@ -181,7 +181,7 @@ ComboboxContext.displayName = 'ComboboxContext' function useComboboxContext(component: string) { let context = useContext(ComboboxContext) if (context === null) { - let err = new Error(`<${component} /> is missing a parent <${Combobox.name} /> component.`) + let err = new Error(`<${component} /> is missing a parent component.`) if (Error.captureStackTrace) Error.captureStackTrace(err, useComboboxContext) throw err } @@ -197,7 +197,7 @@ ComboboxActions.displayName = 'ComboboxActions' function useComboboxActions() { let context = useContext(ComboboxActions) if (context === null) { - let err = new Error(`ComboboxActions is missing a parent <${Combobox.name} /> component.`) + let err = new Error(`ComboboxActions is missing a parent component.`) if (Error.captureStackTrace) Error.captureStackTrace(err, useComboboxActions) throw err } @@ -216,14 +216,19 @@ interface ComboboxRenderPropArg { disabled: boolean activeIndex: number | null activeOption: T | null + latestActiveOption: T | null } -export function Combobox( +let ComboboxRoot = forwardRefWithAs(function Combobox< + TTag extends ElementType = typeof DEFAULT_COMBOBOX_TAG, + TType = string +>( props: Props, 'value' | 'onChange' | 'disabled'> & { value: TType onChange(value: TType): void disabled?: boolean - } + }, + ref: Ref ) { let { value, onChange, disabled = false, ...passThroughProps } = props @@ -282,24 +287,28 @@ export function Combobox(null) - if (!isFocusableElement(target, FocusableMode.Loose)) { - event.preventDefault() - inputRef.current?.focus() + useEffect(() => { + if (activeOptionIndex !== null) { + latestActiveOption.current = options[activeOptionIndex].dataRef.current.value as TType } - }) + }, [activeOptionIndex]) + + let activeOption = + activeOptionIndex === null ? null : (options[activeOptionIndex].dataRef.current.value as TType) let slot = useMemo>( () => ({ open: comboboxState === ComboboxStates.Open, disabled, activeIndex: activeOptionIndex, - activeOption: - activeOptionIndex === null - ? null - : (options[activeOptionIndex].dataRef.current.value as TType), + activeOption: activeOption, + latestActiveOption: activeOption ?? (latestActiveOption.current as TType), }), - [comboboxState, disabled, options, activeOptionIndex] + [comboboxState, disabled, options, activeOptionIndex, latestActiveOption] ) let syncInputValue = useCallback(() => { @@ -359,7 +368,13 @@ export function Combobox {render({ - props: passThroughProps, + props: + ref === null + ? passThroughProps + : { + ...passThroughProps, + ref, + }, slot, defaultTag: DEFAULT_COMBOBOX_TAG, name: 'Combobox', @@ -368,7 +383,7 @@ export function Combobox ) -} +}) // --- @@ -392,7 +407,7 @@ let Input = forwardRefWithAs(function Input< TTag extends ElementType = typeof DEFAULT_INPUT_TAG, // TODO: One day we will be able to infer this type from the generic in Combobox itself. // But today is not that day.. - TType = Parameters[0]['value'] + TType = Parameters[0]['value'] >( props: Props & { displayValue?(item: TType): string @@ -807,7 +822,7 @@ function Option< TTag extends ElementType = typeof DEFAULT_OPTION_TAG, // TODO: One day we will be able to infer this type from the generic in Combobox itself. // But today is not that day.. - TType = Parameters[0]['value'] + TType = Parameters[0]['value'] >( props: Props & { disabled?: boolean @@ -911,8 +926,10 @@ function Option< // --- -Combobox.Input = Input -Combobox.Button = Button -Combobox.Label = Label -Combobox.Options = Options -Combobox.Option = Option +export let Combobox = Object.assign(ComboboxRoot, { + Input, + Button, + Label, + Options, + Option, +}) diff --git a/packages/@headlessui-react/src/test-utils/interactions.ts b/packages/@headlessui-react/src/test-utils/interactions.ts index 71fef368c7..25f4df3aae 100644 --- a/packages/@headlessui-react/src/test-utils/interactions.ts +++ b/packages/@headlessui-react/src/test-utils/interactions.ts @@ -92,6 +92,7 @@ let order: Record< return fireEvent.keyPress(element, event) }, function input(element, event) { + // TODO: This should only fire when the element's value changes return fireEvent.input(element, event) }, function keyup(element, event) { @@ -139,6 +140,17 @@ let order: Record< return fireEvent.keyUp(element, event) }, ], + [Keys.Escape.key!]: [ + function keydown(element, event) { + return fireEvent.keyDown(element, event) + }, + function keypress(element, event) { + return fireEvent.keyPress(element, event) + }, + function keyup(element, event) { + return fireEvent.keyUp(element, event) + }, + ], } export async function type(events: Partial[], element = document.activeElement) { diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.tsx b/packages/@headlessui-vue/src/components/combobox/combobox.test.tsx index 1c3702d9d9..94445dd8ff 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.test.tsx +++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.tsx @@ -3791,7 +3791,9 @@ describe('Mouse interactions', () => { }) ) - it( + // TODO: JSDOM doesn't quite work here + // Clicking outside on the body should fire a mousedown (which it does) and then change the active element (which it doesn't) + xit( 'should be possible to click outside of the combobox which should close the combobox', suppressConsoleLogs(async () => { renderTemplate({ @@ -3805,6 +3807,7 @@ describe('Mouse interactions', () => { charlie +
after
`, setup: () => ({ value: ref(null) }), }) @@ -3815,13 +3818,13 @@ describe('Mouse interactions', () => { assertActiveElement(getComboboxInput()) // Click something that is not related to the combobox - await click(document.body) + await click(getByText('after')) // Should be closed now assertComboboxList({ state: ComboboxState.InvisibleUnmounted }) // Verify the input is focused again - assertActiveElement(getComboboxInput()) + assertActiveElement(getByText('after')) }) ) @@ -4346,4 +4349,65 @@ describe('Mouse interactions', () => { assertNoActiveComboboxOption() }) ) + + it( + 'Combobox preserves the latest known active option after an option becomes inactive', + suppressConsoleLogs(async () => { + renderTemplate({ + template: html` + + + Trigger +
{{ latestActiveOption }}
+ + Option A + Option B + Option C + +
+ `, + setup: () => ({ value: ref(null) }), + }) + + assertComboboxButton({ + state: ComboboxState.InvisibleUnmounted, + attributes: { id: 'headlessui-combobox-button-2' }, + }) + assertComboboxList({ state: ComboboxState.InvisibleUnmounted }) + + await click(getComboboxButton()) + + assertComboboxButton({ + state: ComboboxState.Visible, + attributes: { id: 'headlessui-combobox-button-2' }, + }) + assertComboboxList({ state: ComboboxState.Visible }) + + let options = getComboboxOptions() + + // Hover the first item + await mouseMove(options[0]) + + // Verify that the first combobox option is active + assertActiveComboboxOption(options[0]) + expect(document.getElementById('latestActiveOption')!.textContent).toBe('a') + + // Focus the second item + await mouseMove(options[1]) + + // Verify that the second combobox option is active + assertActiveComboboxOption(options[1]) + expect(document.getElementById('latestActiveOption')!.textContent).toBe('b') + + // Move the mouse off of the second combobox option + await mouseLeave(options[1]) + await mouseMove(document.body) + + // Verify that the second combobox option is NOT active + assertNoActiveComboboxOption() + + // But the last known active option is still recorded + expect(document.getElementById('latestActiveOption')!.textContent).toBe('b') + }) + ) }) diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index 0d267e6b59..63f4c26397 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -208,7 +208,6 @@ export let Combobox = defineComponent({ useWindowEvent('mousedown', (event) => { let target = event.target as HTMLElement - let active = document.activeElement if (comboboxState.value !== ComboboxStates.Open) return @@ -217,9 +216,6 @@ export let Combobox = defineComponent({ if (dom(optionsRef)?.contains(target)) return api.closeCombobox() - - if (active !== document.body && active?.contains(target)) return // Keep focus on newly clicked/focused element - if (!event.defaultPrevented) dom(inputRef)?.focus({ preventScroll: true }) }) watchEffect(() => { @@ -237,8 +233,32 @@ export let Combobox = defineComponent({ ) ) + let latestActiveOption = ref(null) + let activeOption = computed(() => + activeOptionIndex.value === null + ? null + : (options.value[activeOptionIndex.value].dataRef.value as any) + ) + + watch( + activeOptionIndex, + (activeOptionIndex) => { + if (activeOptionIndex !== null) { + latestActiveOption.value = options.value[activeOptionIndex].dataRef.value as any + } + }, + { flush: 'sync' } + ) + return () => { - let slot = { open: comboboxState.value === ComboboxStates.Open, disabled: props.disabled } + let slot = { + open: comboboxState.value === ComboboxStates.Open, + disabled: props.disabled, + activeIndex: activeOptionIndex.value, + activeOption: activeOption.value, + latestActiveOption: latestActiveOption.value, + } + return render({ props: omit(props, ['modelValue', 'onUpdate:modelValue', 'disabled', 'horizontal']), slot, @@ -483,6 +503,8 @@ export let ComboboxInput = defineComponent({ return () => { let slot = { open: api.comboboxState.value === ComboboxStates.Open } let propsWeControl = { + 'aria-controls': api.optionsRef.value?.id, + 'aria-expanded': api.disabled ? undefined : api.comboboxState.value === ComboboxStates.Open, 'aria-activedescendant': api.activeOptionIndex.value === null ? undefined @@ -491,7 +513,9 @@ export let ComboboxInput = defineComponent({ id, onKeydown: handleKeyDown, onChange: handleChange, + onInput: handleChange, role: 'combobox', + type: 'text', tabIndex: 0, ref: api.inputRef, } diff --git a/packages/@headlessui-vue/src/test-utils/interactions.ts b/packages/@headlessui-vue/src/test-utils/interactions.ts index 600e605d1a..3e11ac5600 100644 --- a/packages/@headlessui-vue/src/test-utils/interactions.ts +++ b/packages/@headlessui-vue/src/test-utils/interactions.ts @@ -92,6 +92,7 @@ let order: Record< return fireEvent.keyPress(element, event) }, function input(element, event) { + // TODO: This should only fire when the element's value changes return fireEvent.input(element, event) }, function keyup(element, event) { @@ -139,6 +140,17 @@ let order: Record< return fireEvent.keyUp(element, event) }, ], + [Keys.Escape.key!]: [ + function keydown(element, event) { + return fireEvent.keyDown(element, event) + }, + function keypress(element, event) { + return fireEvent.keyPress(element, event) + }, + function keyup(element, event) { + return fireEvent.keyUp(element, event) + }, + ], } export async function type(events: Partial[], element = document.activeElement) { diff --git a/packages/@headlessui-vue/src/utils/dom.ts b/packages/@headlessui-vue/src/utils/dom.ts index 32152f3a90..5d34de0eed 100644 --- a/packages/@headlessui-vue/src/utils/dom.ts +++ b/packages/@headlessui-vue/src/utils/dom.ts @@ -1,7 +1,10 @@ -import { Ref } from 'vue' +import { Ref, ComponentPublicInstance } from 'vue' -export function dom(ref?: Ref): T | null { +export function dom( + ref?: Ref +): T | null { if (ref == null) return null if (ref.value == null) return null - return ((ref as Ref).value.$el ?? ref.value) as T | null + + return '$el' in ref.value ? (ref.value.$el as T | null) : ref.value } diff --git a/packages/playground-vue/src/components/combobox/combobox-with-pure-tailwind.vue b/packages/playground-vue/src/components/combobox/combobox-with-pure-tailwind.vue new file mode 100644 index 0000000000..65389c7386 --- /dev/null +++ b/packages/playground-vue/src/components/combobox/combobox-with-pure-tailwind.vue @@ -0,0 +1,154 @@ + + diff --git a/packages/playground-vue/src/components/combobox/command-palette-with-groups.vue b/packages/playground-vue/src/components/combobox/command-palette-with-groups.vue new file mode 100644 index 0000000000..0172fd1b70 --- /dev/null +++ b/packages/playground-vue/src/components/combobox/command-palette-with-groups.vue @@ -0,0 +1,174 @@ + + + diff --git a/packages/playground-vue/src/components/combobox/command-palette.vue b/packages/playground-vue/src/components/combobox/command-palette.vue new file mode 100644 index 0000000000..9e03c9302e --- /dev/null +++ b/packages/playground-vue/src/components/combobox/command-palette.vue @@ -0,0 +1,146 @@ + + + diff --git a/packages/playground-vue/src/routes.json b/packages/playground-vue/src/routes.json index d1f1408eb0..e7c841e1fb 100644 --- a/packages/playground-vue/src/routes.json +++ b/packages/playground-vue/src/routes.json @@ -3,6 +3,27 @@ "path": "/", "component": "./components/Home.vue" }, + { + "name": "Combobox", + "path": "/combobox", + "children": [ + { + "name": "Combobox (w/ pure tailwind)", + "path": "/combobox/combobox-with-pure-tailwind", + "component": "./components/combobox/combobox-with-pure-tailwind.vue" + }, + { + "name": "Command Palette", + "path": "/combobox/command-palette", + "component": "./components/combobox/command-palette.vue" + }, + { + "name": "Command Palette (w/ Groups)", + "path": "/combobox/command-palette-with-groups", + "component": "./components/combobox/command-palette-with-groups.vue" + } + ] + }, { "name": "Menu", "path": "/menu", diff --git a/scripts/lint.sh b/scripts/lint.sh index 78270ebce6..2d503d966e 100755 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -16,7 +16,6 @@ if ! [ -z "$CI" ]; then prettierArgs+=("--check") else prettierArgs+=("--write") - prettierArgs+=("$RELATIVE_TARGET_DIR") fi # Add default arguments @@ -27,11 +26,10 @@ prettierArgs+=($@) # Ensure that a path is passed, otherwise default to the current directory if [ -z "$@" ]; then - prettierArgs+=(.) + prettierArgs+=("$RELATIVE_TARGET_DIR") fi # Execute yarn prettier "${prettierArgs[@]}" popd > /dev/null -