From 8208c07572cb39571df3532dc6e5fe9f95ccdf85 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 2 Mar 2022 22:01:14 +0100 Subject: [PATCH] Adjust active {item,option} index (#1184) * adjust active {item,option} index We had various ordering issues, and now we properly sort all the notes which is awesome. However, there is this case where we still use the `activeOptionIndex` / `activeItemIndex` from _before_ the sort happens. Now we will ensure that this is properly adjusted when performing the sort of the items. In addition, we will also properly adjust these values when `registering` and `unregistering` items, not only when performing actions. * update changelog --- CHANGELOG.md | 2 + .../src/components/combobox/combobox.tsx | 77 ++++++++------- .../src/components/listbox/listbox.tsx | 73 +++++++++------ .../src/components/menu/menu.tsx | 65 ++++++++----- .../src/components/combobox/combobox.ts | 93 +++++++++++-------- .../src/components/listbox/listbox.ts | 81 ++++++++++------ .../src/components/menu/menu.ts | 82 ++++++++++------ 7 files changed, 299 insertions(+), 174 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50af4627da..eb1a838802 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure that `appear` works regardless of multiple rerenders ([#1179](https://github.com/tailwindlabs/headlessui/pull/1179)) - Reset Combobox Input when the value gets reset ([#1181](https://github.com/tailwindlabs/headlessui/pull/1181)) - Fix double `beforeEnter` due to SSR ([#1183](https://github.com/tailwindlabs/headlessui/pull/1183)) +- Adjust active {item,option} index ([#1184](https://github.com/tailwindlabs/headlessui/pull/1184)) ## [Unreleased - @headlessui/vue] @@ -32,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Guarantee DOM sort order when performing actions ([#1168](https://github.com/tailwindlabs/headlessui/pull/1168)) - Improve outside click support ([#1175](https://github.com/tailwindlabs/headlessui/pull/1175)) - Reset Combobox Input when the value gets reset ([#1181](https://github.com/tailwindlabs/headlessui/pull/1181)) +- Adjust active {item,option} index ([#1184](https://github.com/tailwindlabs/headlessui/pull/1184)) ## [@headlessui/react@v1.5.0] - 2022-02-17 diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx index 04bc3c9cfd..e3dab395e2 100644 --- a/packages/@headlessui-react/src/components/combobox/combobox.tsx +++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx @@ -92,6 +92,35 @@ enum ActionTypes { UnregisterOption, } +function adjustOrderedState( + state: StateDefinition, + adjustment: (options: StateDefinition['options']) => StateDefinition['options'] = (i) => i +) { + let currentActiveOption = + state.activeOptionIndex !== null ? state.options[state.activeOptionIndex] : null + + let sortedOptions = sortByDomNode( + adjustment(state.options.slice()), + (option) => option.dataRef.current.domRef.current + ) + + // If we inserted an option before the current active option then the active option index + // would be wrong. To fix this, we will re-lookup the correct index. + let adjustedActiveOptionIndex = currentActiveOption + ? sortedOptions.indexOf(currentActiveOption) + : null + + // Reset to `null` in case the currentActiveOption was removed. + if (adjustedActiveOptionIndex === -1) { + adjustedActiveOptionIndex = null + } + + return { + options: sortedOptions, + activeOptionIndex: adjustedActiveOptionIndex, + } +} + type Actions = | { type: ActionTypes.CloseCombobox } | { type: ActionTypes.OpenCombobox } @@ -135,39 +164,29 @@ let reducers: { return state } - let options = sortByDomNode(state.options, (option) => option.dataRef.current.domRef.current) - + let adjustedState = adjustOrderedState(state) let activeOptionIndex = calculateActiveIndex(action, { - resolveItems: () => options, - resolveActiveIndex: () => state.activeOptionIndex, + resolveItems: () => adjustedState.options, + resolveActiveIndex: () => adjustedState.activeOptionIndex, resolveId: (item) => item.id, resolveDisabled: (item) => item.dataRef.current.disabled, }) - if (state.activeOptionIndex === activeOptionIndex) return state return { ...state, - options, // Sorted options + ...adjustedState, activeOptionIndex, activationTrigger: action.trigger ?? ActivationTrigger.Other, } }, [ActionTypes.RegisterOption]: (state, action) => { - let currentActiveOption = - state.activeOptionIndex !== null ? state.options[state.activeOptionIndex] : null + let adjustedState = adjustOrderedState(state, (options) => { + return [...options, { id: action.id, dataRef: action.dataRef }] + }) - let options = [...state.options, { id: action.id, dataRef: action.dataRef }] let nextState = { ...state, - options, - activeOptionIndex: (() => { - if (currentActiveOption === null) return null - - // If we inserted an option before the current active option then the - // active option index would be wrong. To fix this, we will re-lookup - // the correct index. - return options.indexOf(currentActiveOption) - })(), + ...adjustedState, activationTrigger: ActivationTrigger.Other, } @@ -181,25 +200,15 @@ let reducers: { return nextState }, [ActionTypes.UnregisterOption]: (state, action) => { - let nextOptions = state.options.slice() - let currentActiveOption = - state.activeOptionIndex !== null ? nextOptions[state.activeOptionIndex] : null - - let idx = nextOptions.findIndex((a) => a.id === action.id) - - if (idx !== -1) nextOptions.splice(idx, 1) + let adjustedState = adjustOrderedState(state, (options) => { + let idx = options.findIndex((a) => a.id === action.id) + if (idx !== -1) options.splice(idx, 1) + return options + }) return { ...state, - options: nextOptions, - activeOptionIndex: (() => { - if (idx === state.activeOptionIndex) return null - if (currentActiveOption === null) return null - - // If we removed the option before the actual active index, then it would be out of sync. To - // fix this, we will find the correct (new) index position. - return nextOptions.indexOf(currentActiveOption) - })(), + ...adjustedState, activationTrigger: ActivationTrigger.Other, } }, diff --git a/packages/@headlessui-react/src/components/listbox/listbox.tsx b/packages/@headlessui-react/src/components/listbox/listbox.tsx index b0e1ff44ce..eb973a6d8e 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.tsx @@ -83,6 +83,35 @@ enum ActionTypes { UnregisterOption, } +function adjustOrderedState( + state: StateDefinition, + adjustment: (options: StateDefinition['options']) => StateDefinition['options'] = (i) => i +) { + let currentActiveOption = + state.activeOptionIndex !== null ? state.options[state.activeOptionIndex] : null + + let sortedOptions = sortByDomNode( + adjustment(state.options.slice()), + (option) => option.dataRef.current.domRef.current + ) + + // If we inserted an option before the current active option then the active option index + // would be wrong. To fix this, we will re-lookup the correct index. + let adjustedActiveOptionIndex = currentActiveOption + ? sortedOptions.indexOf(currentActiveOption) + : null + + // Reset to `null` in case the currentActiveOption was removed. + if (adjustedActiveOptionIndex === -1) { + adjustedActiveOptionIndex = null + } + + return { + options: sortedOptions, + activeOptionIndex: adjustedActiveOptionIndex, + } +} + type Actions = | { type: ActionTypes.CloseListbox } | { type: ActionTypes.OpenListbox } @@ -127,19 +156,17 @@ let reducers: { if (state.disabled) return state if (state.listboxState === ListboxStates.Closed) return state - let options = sortByDomNode(state.options, (option) => option.dataRef.current.domRef.current) - + let adjustedState = adjustOrderedState(state) let activeOptionIndex = calculateActiveIndex(action, { - resolveItems: () => options, - resolveActiveIndex: () => state.activeOptionIndex, - resolveId: (item) => item.id, - resolveDisabled: (item) => item.dataRef.current.disabled, + resolveItems: () => adjustedState.options, + resolveActiveIndex: () => adjustedState.activeOptionIndex, + resolveId: (option) => option.id, + resolveDisabled: (option) => option.dataRef.current.disabled, }) - if (state.searchQuery === '' && state.activeOptionIndex === activeOptionIndex) return state return { ...state, - options, // Sorted options + ...adjustedState, searchQuery: '', activeOptionIndex, activationTrigger: action.trigger ?? ActivationTrigger.Other, @@ -184,29 +211,23 @@ let reducers: { return { ...state, searchQuery: '' } }, [ActionTypes.RegisterOption]: (state, action) => { - let options = [...state.options, { id: action.id, dataRef: action.dataRef }] - return { ...state, options } + let adjustedState = adjustOrderedState(state, (options) => [ + ...options, + { id: action.id, dataRef: action.dataRef }, + ]) + + return { ...state, ...adjustedState } }, [ActionTypes.UnregisterOption]: (state, action) => { - let nextOptions = state.options.slice() - let currentActiveOption = - state.activeOptionIndex !== null ? nextOptions[state.activeOptionIndex] : null - - let idx = nextOptions.findIndex((a) => a.id === action.id) - - if (idx !== -1) nextOptions.splice(idx, 1) + let adjustedState = adjustOrderedState(state, (options) => { + let idx = options.findIndex((a) => a.id === action.id) + if (idx !== -1) options.splice(idx, 1) + return options + }) return { ...state, - options: nextOptions, - activeOptionIndex: (() => { - if (idx === state.activeOptionIndex) return null - if (currentActiveOption === null) return null - - // If we removed the option before the actual active index, then it would be out of sync. To - // fix this, we will find the correct (new) index position. - return nextOptions.indexOf(currentActiveOption) - })(), + ...adjustedState, activationTrigger: ActivationTrigger.Other, } }, diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 4d24a7edc1..253bd9646e 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -73,6 +73,32 @@ enum ActionTypes { UnregisterItem, } +function adjustOrderedState( + state: StateDefinition, + adjustment: (items: StateDefinition['items']) => StateDefinition['items'] = (i) => i +) { + let currentActiveItem = state.activeItemIndex !== null ? state.items[state.activeItemIndex] : null + + let sortedItems = sortByDomNode( + adjustment(state.items.slice()), + (item) => item.dataRef.current.domRef.current + ) + + // If we inserted an item before the current active item then the active item index + // would be wrong. To fix this, we will re-lookup the correct index. + let adjustedActiveItemIndex = currentActiveItem ? sortedItems.indexOf(currentActiveItem) : null + + // Reset to `null` in case the currentActiveItem was removed. + if (adjustedActiveItemIndex === -1) { + adjustedActiveItemIndex = null + } + + return { + items: sortedItems, + activeItemIndex: adjustedActiveItemIndex, + } +} + type Actions = | { type: ActionTypes.CloseMenu } | { type: ActionTypes.OpenMenu } @@ -102,19 +128,17 @@ let reducers: { return { ...state, menuState: MenuStates.Open } }, [ActionTypes.GoToItem]: (state, action) => { - let items = sortByDomNode(state.items, (item) => item.dataRef.current.domRef.current) - + let adjustedState = adjustOrderedState(state) let activeItemIndex = calculateActiveIndex(action, { - resolveItems: () => items, - resolveActiveIndex: () => state.activeItemIndex, + resolveItems: () => adjustedState.items, + resolveActiveIndex: () => adjustedState.activeItemIndex, resolveId: (item) => item.id, resolveDisabled: (item) => item.dataRef.current.disabled, }) - if (state.searchQuery === '' && state.activeItemIndex === activeItemIndex) return state return { ...state, - items, // Sorted items + ...adjustedState, searchQuery: '', activeItemIndex, activationTrigger: action.trigger ?? ActivationTrigger.Other, @@ -151,28 +175,23 @@ let reducers: { return { ...state, searchQuery: '', searchActiveItemIndex: null } }, [ActionTypes.RegisterItem]: (state, action) => { - let items = [...state.items, { id: action.id, dataRef: action.dataRef }] - return { ...state, items } + let adjustedState = adjustOrderedState(state, (items) => [ + ...items, + { id: action.id, dataRef: action.dataRef }, + ]) + + return { ...state, ...adjustedState } }, [ActionTypes.UnregisterItem]: (state, action) => { - let nextItems = state.items.slice() - let currentActiveItem = state.activeItemIndex !== null ? nextItems[state.activeItemIndex] : null - - let idx = nextItems.findIndex((a) => a.id === action.id) - - if (idx !== -1) nextItems.splice(idx, 1) + let adjustedState = adjustOrderedState(state, (items) => { + let idx = items.findIndex((a) => a.id === action.id) + if (idx !== -1) items.splice(idx, 1) + return items + }) return { ...state, - items: nextItems, - activeItemIndex: (() => { - if (idx === state.activeItemIndex) return null - if (currentActiveItem === null) return null - - // If we removed the item before the actual active index, then it would be out of sync. To - // fix this, we will find the correct (new) index position. - return nextItems.indexOf(currentActiveItem) - })(), + ...adjustedState, activationTrigger: ActivationTrigger.Other, } }, diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts index a5d58726e9..ba9a22f7f6 100644 --- a/packages/@headlessui-vue/src/components/combobox/combobox.ts +++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts @@ -14,6 +14,7 @@ import { InjectionKey, PropType, Ref, + UnwrapNestedRefs, } from 'vue' import { Features, render, omit } from '../../utils/render' @@ -38,11 +39,11 @@ enum ActivationTrigger { Other, } -type ComboboxOptionDataRef = Ref<{ +type ComboboxOptionData = { disabled: boolean value: unknown domRef: Ref -}> +} type StateDefinition = { // State comboboxState: Ref @@ -57,7 +58,7 @@ type StateDefinition = { optionsRef: Ref disabled: Ref - options: Ref<{ id: string; dataRef: ComboboxOptionDataRef }[]> + options: Ref<{ id: string; dataRef: ComputedRef }[]> activeOptionIndex: Ref activationTrigger: Ref @@ -67,7 +68,7 @@ type StateDefinition = { goToOption(focus: Focus, id?: string, trigger?: ActivationTrigger): void selectOption(id: string): void selectActiveOption(): void - registerOption(id: string, dataRef: ComboboxOptionDataRef): void + registerOption(id: string, dataRef: ComputedRef): void unregisterOption(id: string): void select(value: unknown): void } @@ -113,6 +114,36 @@ export let Combobox = defineComponent({ let activationTrigger = ref( ActivationTrigger.Other ) + + function adjustOrderedState( + adjustment: ( + options: UnwrapNestedRefs + ) => UnwrapNestedRefs = (i) => i + ) { + let currentActiveOption = + activeOptionIndex.value !== null ? options.value[activeOptionIndex.value] : null + + let sortedOptions = sortByDomNode(adjustment(options.value.slice()), (option) => + dom(option.dataRef.domRef) + ) + + // If we inserted an option before the current active option then the active option index + // would be wrong. To fix this, we will re-lookup the correct index. + let adjustedActiveOptionIndex = currentActiveOption + ? sortedOptions.indexOf(currentActiveOption) + : null + + // Reset to `null` in case the currentActiveOption was removed. + if (adjustedActiveOptionIndex === -1) { + adjustedActiveOptionIndex = null + } + + return { + options: sortedOptions, + activeOptionIndex: adjustedActiveOptionIndex, + } + } + let value = computed(() => props.modelValue) let api = { @@ -149,15 +180,14 @@ export let Combobox = defineComponent({ return } - let orderedOptions = sortByDomNode(options.value, (option) => option.dataRef.domRef.value) - + let adjustedState = adjustOrderedState() let nextActiveOptionIndex = calculateActiveIndex( focus === Focus.Specific ? { focus: Focus.Specific, id: id! } : { focus: focus as Exclude }, { - resolveItems: () => orderedOptions, - resolveActiveIndex: () => activeOptionIndex.value, + resolveItems: () => adjustedState.options, + resolveActiveIndex: () => adjustedState.activeOptionIndex, resolveId: (option) => option.id, resolveDisabled: (option) => option.dataRef.disabled, } @@ -165,7 +195,7 @@ export let Combobox = defineComponent({ activeOptionIndex.value = nextActiveOptionIndex activationTrigger.value = trigger ?? ActivationTrigger.Other - options.value = orderedOptions + options.value = adjustedState.options }, syncInputValue() { let value = api.value.value @@ -196,37 +226,24 @@ export let Combobox = defineComponent({ emit('update:modelValue', dataRef.value) api.syncInputValue() }, - registerOption(id: string, dataRef: ComboboxOptionDataRef) { - let currentActiveOption = - activeOptionIndex.value !== null ? options.value[activeOptionIndex.value] : null - - // @ts-expect-error The expected type comes from property 'dataRef' which is declared here on type '{ id: string; dataRef: { textValue: string; disabled: boolean; }; }' - options.value = [...options.value, { id, dataRef }] - - // If we inserted an option before the current active option then the - // active option index would be wrong. To fix this, we will re-lookup - // the correct index. - activeOptionIndex.value = (() => { - if (currentActiveOption === null) return null - return options.value.indexOf(currentActiveOption) - })() + registerOption(id: string, dataRef: ComboboxOptionData) { + let adjustedState = adjustOrderedState((options) => { + return [...options, { id, dataRef }] + }) + + options.value = adjustedState.options + activeOptionIndex.value = adjustedState.activeOptionIndex activationTrigger.value = ActivationTrigger.Other }, unregisterOption(id: string) { - let nextOptions = options.value.slice() - let currentActiveOption = - activeOptionIndex.value !== null ? nextOptions[activeOptionIndex.value] : null - let idx = nextOptions.findIndex((a) => a.id === id) - if (idx !== -1) nextOptions.splice(idx, 1) - options.value = nextOptions - activeOptionIndex.value = (() => { - if (idx === activeOptionIndex.value) return null - if (currentActiveOption === null) return null - - // If we removed the option before the actual active index, then it would be out of sync. To - // fix this, we will find the correct (new) index position. - return nextOptions.indexOf(currentActiveOption) - })() + let adjustedState = adjustOrderedState((options) => { + let idx = options.findIndex((a) => a.id === id) + if (idx !== -1) options.splice(idx, 1) + return options + }) + + options.value = adjustedState.options + activeOptionIndex.value = adjustedState.activeOptionIndex activationTrigger.value = ActivationTrigger.Other }, } @@ -636,7 +653,7 @@ export let ComboboxOption = defineComponent({ let selected = computed(() => toRaw(api.value.value) === toRaw(props.value)) - let dataRef = computed(() => ({ + let dataRef = computed(() => ({ disabled: props.disabled, value: props.value, domRef: internalOptionRef, diff --git a/packages/@headlessui-vue/src/components/listbox/listbox.ts b/packages/@headlessui-vue/src/components/listbox/listbox.ts index 0c2f942efc..93be2b2c34 100644 --- a/packages/@headlessui-vue/src/components/listbox/listbox.ts +++ b/packages/@headlessui-vue/src/components/listbox/listbox.ts @@ -13,6 +13,7 @@ import { watchEffect, toRaw, watch, + UnwrapNestedRefs, } from 'vue' import { Features, render, omit } from '../../utils/render' @@ -40,12 +41,12 @@ function nextFrame(cb: () => void) { requestAnimationFrame(() => requestAnimationFrame(cb)) } -type ListboxOptionDataRef = Ref<{ +type ListboxOptionData = { textValue: string disabled: boolean value: unknown domRef: Ref -}> +} type StateDefinition = { // State listboxState: Ref @@ -57,7 +58,7 @@ type StateDefinition = { optionsRef: Ref disabled: Ref - options: Ref<{ id: string; dataRef: ListboxOptionDataRef }[]> + options: Ref<{ id: string; dataRef: ComputedRef }[]> searchQuery: Ref activeOptionIndex: Ref activationTrigger: Ref @@ -68,7 +69,7 @@ type StateDefinition = { goToOption(focus: Focus, id?: string, trigger?: ActivationTrigger): void search(value: string): void clearSearch(): void - registerOption(id: string, dataRef: ListboxOptionDataRef): void + registerOption(id: string, dataRef: ComputedRef): void unregisterOption(id: string): void select(value: unknown): void } @@ -110,6 +111,35 @@ export let Listbox = defineComponent({ ActivationTrigger.Other ) + function adjustOrderedState( + adjustment: ( + options: UnwrapNestedRefs + ) => UnwrapNestedRefs = (i) => i + ) { + let currentActiveOption = + activeOptionIndex.value !== null ? options.value[activeOptionIndex.value] : null + + let sortedOptions = sortByDomNode(adjustment(options.value.slice()), (option) => + dom(option.dataRef.domRef) + ) + + // If we inserted an option before the current active option then the active option index + // would be wrong. To fix this, we will re-lookup the correct index. + let adjustedActiveOptionIndex = currentActiveOption + ? sortedOptions.indexOf(currentActiveOption) + : null + + // Reset to `null` in case the currentActiveOption was removed. + if (adjustedActiveOptionIndex === -1) { + adjustedActiveOptionIndex = null + } + + return { + options: sortedOptions, + activeOptionIndex: adjustedActiveOptionIndex, + } + } + let value = computed(() => props.modelValue) let api = { @@ -139,15 +169,14 @@ export let Listbox = defineComponent({ if (props.disabled) return if (listboxState.value === ListboxStates.Closed) return - let orderedOptions = sortByDomNode(options.value, (option) => option.dataRef.domRef.value) - + let adjustedState = adjustOrderedState() let nextActiveOptionIndex = calculateActiveIndex( focus === Focus.Specific ? { focus: Focus.Specific, id: id! } : { focus: focus as Exclude }, { - resolveItems: () => orderedOptions, - resolveActiveIndex: () => activeOptionIndex.value, + resolveItems: () => adjustedState.options, + resolveActiveIndex: () => adjustedState.activeOptionIndex, resolveId: (option) => option.id, resolveDisabled: (option) => option.dataRef.disabled, } @@ -156,7 +185,7 @@ export let Listbox = defineComponent({ searchQuery.value = '' activeOptionIndex.value = nextActiveOptionIndex activationTrigger.value = trigger ?? ActivationTrigger.Other - options.value = orderedOptions + options.value = adjustedState.options }, search(value: string) { if (props.disabled) return @@ -192,25 +221,23 @@ export let Listbox = defineComponent({ searchQuery.value = '' }, - registerOption(id: string, dataRef: ListboxOptionDataRef) { - // @ts-expect-error The expected type comes from property 'dataRef' which is declared here on type '{ id: string; dataRef: { textValue: string; disabled: boolean; }; }' - options.value = [...options.value, { id, dataRef }] + registerOption(id: string, dataRef: ListboxOptionData) { + let adjustedState = adjustOrderedState((options) => { + return [...options, { id, dataRef }] + }) + + options.value = adjustedState.options + activeOptionIndex.value = adjustedState.activeOptionIndex }, unregisterOption(id: string) { - let nextOptions = options.value.slice() - let currentActiveOption = - activeOptionIndex.value !== null ? nextOptions[activeOptionIndex.value] : null - let idx = nextOptions.findIndex((a) => a.id === id) - if (idx !== -1) nextOptions.splice(idx, 1) - options.value = nextOptions - activeOptionIndex.value = (() => { - if (idx === activeOptionIndex.value) return null - if (currentActiveOption === null) return null - - // If we removed the option before the actual active index, then it would be out of sync. To - // fix this, we will find the correct (new) index position. - return nextOptions.indexOf(currentActiveOption) - })() + let adjustedState = adjustOrderedState((options) => { + let idx = options.findIndex((a) => a.id === id) + if (idx !== -1) options.splice(idx, 1) + return options + }) + + options.value = adjustedState.options + activeOptionIndex.value = adjustedState.activeOptionIndex activationTrigger.value = ActivationTrigger.Other }, select(value: unknown) { @@ -526,7 +553,7 @@ export let ListboxOption = defineComponent({ let selected = computed(() => toRaw(api.value.value) === toRaw(props.value)) - let dataRef = computed(() => ({ + let dataRef = computed(() => ({ disabled: props.disabled, value: props.value, textValue: '', diff --git a/packages/@headlessui-vue/src/components/menu/menu.ts b/packages/@headlessui-vue/src/components/menu/menu.ts index ff8e79b958..25ea49c746 100644 --- a/packages/@headlessui-vue/src/components/menu/menu.ts +++ b/packages/@headlessui-vue/src/components/menu/menu.ts @@ -10,6 +10,8 @@ import { InjectionKey, Ref, watchEffect, + ComputedRef, + UnwrapNestedRefs, } from 'vue' import { Features, render } from '../../utils/render' import { useId } from '../../hooks/use-id' @@ -37,17 +39,17 @@ function nextFrame(cb: () => void) { requestAnimationFrame(() => requestAnimationFrame(cb)) } -type MenuItemDataRef = Ref<{ +type MenuItemData = { textValue: string disabled: boolean domRef: Ref -}> +} type StateDefinition = { // State menuState: Ref buttonRef: Ref itemsRef: Ref - items: Ref<{ id: string; dataRef: MenuItemDataRef }[]> + items: Ref<{ id: string; dataRef: ComputedRef }[]> searchQuery: Ref activeItemIndex: Ref activationTrigger: Ref @@ -58,7 +60,7 @@ type StateDefinition = { goToItem(focus: Focus, id?: string, trigger?: ActivationTrigger): void search(value: string): void clearSearch(): void - registerItem(id: string, dataRef: MenuItemDataRef): void + registerItem(id: string, dataRef: ComputedRef): void unregisterItem(id: string): void } @@ -90,6 +92,35 @@ export let Menu = defineComponent({ ActivationTrigger.Other ) + function adjustOrderedState( + adjustment: ( + items: UnwrapNestedRefs + ) => UnwrapNestedRefs = (i) => i + ) { + let currentActiveItem = + activeItemIndex.value !== null ? items.value[activeItemIndex.value] : null + + let sortedItems = sortByDomNode(adjustment(items.value.slice()), (item) => + dom(item.dataRef.domRef) + ) + + // If we inserted an item before the current active item then the active item index + // would be wrong. To fix this, we will re-lookup the correct index. + let adjustedActiveItemIndex = currentActiveItem + ? sortedItems.indexOf(currentActiveItem) + : null + + // Reset to `null` in case the currentActiveItem was removed. + if (adjustedActiveItemIndex === -1) { + adjustedActiveItemIndex = null + } + + return { + items: sortedItems, + activeItemIndex: adjustedActiveItemIndex, + } + } + let api = { menuState, buttonRef, @@ -104,14 +135,14 @@ export let Menu = defineComponent({ }, openMenu: () => (menuState.value = MenuStates.Open), goToItem(focus: Focus, id?: string, trigger?: ActivationTrigger) { - let orderedItems = sortByDomNode(items.value, (item) => item.dataRef.domRef.value) + let adjustedState = adjustOrderedState() let nextActiveItemIndex = calculateActiveIndex( focus === Focus.Specific ? { focus: Focus.Specific, id: id! } : { focus: focus as Exclude }, { - resolveItems: () => orderedItems, - resolveActiveIndex: () => activeItemIndex.value, + resolveItems: () => adjustedState.items, + resolveActiveIndex: () => adjustedState.activeItemIndex, resolveId: (item) => item.id, resolveDisabled: (item) => item.dataRef.disabled, } @@ -120,7 +151,7 @@ export let Menu = defineComponent({ searchQuery.value = '' activeItemIndex.value = nextActiveItemIndex activationTrigger.value = trigger ?? ActivationTrigger.Other - items.value = orderedItems + items.value = adjustedState.items }, search(value: string) { let wasAlreadySearching = searchQuery.value !== '' @@ -147,25 +178,24 @@ export let Menu = defineComponent({ clearSearch() { searchQuery.value = '' }, - registerItem(id: string, dataRef: MenuItemDataRef) { - // @ts-expect-error The expected type comes from property 'dataRef' which is declared here on type '{ id: string; dataRef: { textValue: string; disabled: boolean; }; }' - items.value = [...items.value, { id, dataRef }] + registerItem(id: string, dataRef: MenuItemData) { + let adjustedState = adjustOrderedState((items) => { + return [...items, { id, dataRef }] + }) + + items.value = adjustedState.items + activeItemIndex.value = adjustedState.activeItemIndex + activationTrigger.value = ActivationTrigger.Other }, unregisterItem(id: string) { - let nextItems = items.value.slice() - let currentActiveItem = - activeItemIndex.value !== null ? nextItems[activeItemIndex.value] : null - let idx = nextItems.findIndex((a) => a.id === id) - if (idx !== -1) nextItems.splice(idx, 1) - items.value = nextItems - activeItemIndex.value = (() => { - if (idx === activeItemIndex.value) return null - if (currentActiveItem === null) return null - - // If we removed the item before the actual active index, then it would be out of sync. To - // fix this, we will find the correct (new) index position. - return nextItems.indexOf(currentActiveItem) - })() + let adjustedState = adjustOrderedState((items) => { + let idx = items.findIndex((a) => a.id === id) + if (idx !== -1) items.splice(idx, 1) + return items + }) + + items.value = adjustedState.items + activeItemIndex.value = adjustedState.activeItemIndex activationTrigger.value = ActivationTrigger.Other }, } @@ -453,7 +483,7 @@ export let MenuItem = defineComponent({ : false }) - let dataRef = computed(() => ({ + let dataRef = computed(() => ({ disabled: props.disabled, textValue: '', domRef: internalItemRef,