huanld/headlessui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ensure correct order when conditionally rendering Menu.Item, Listbox.Option and RadioGroup.Option (#1045)

Browse Source 
* ensure correct order in `Menu.Item`

* Update Vue version of menu component ordering issue

* ensure correct order of `Listbox.Option`s

* add test to verify that RadioGroup.Option order is correct

* ensure correct order of `ListboxOption`s

* cleanup

* add test to verify that `RadioGroupOption` order is correct

* update changelog

* use similar a,z signature compared to other places

Co-authored-by: Jordan Pittman <jordan@cryptica.me>
This commit is contained in:
Robin Malfait
2022-01-18 16:05:01 +01:00
committed by GitHub
parent 1affad1271
commit a37197694f
13 changed files with 349 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+6 -2
View File
@@ -7,11 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased - @headlessui/react]
- Nothing yet!
### Fixed
- Ensure correct order when conditionally rendering `Menu.Item`, `Listbox.Option` and `RadioGroup.Option` ([#1045](https://github.com/tailwindlabs/headlessui/pull/1045))
## [Unreleased - @headlessui/vue]
- Nothing yet!
### Fixed
- Ensure correct order when conditionally rendering `MenuItem`, `ListboxOption` and `RadioGroupOption` ([#1045](https://github.com/tailwindlabs/headlessui/pull/1045))
## [@headlessui/react@v1.4.3] - 2022-01-14
packages/@headlessui-react/src/components/listbox/listbox.test.tsx
+43
View File
@@ -494,6 +494,49 @@ describe('Rendering', () => {
})
)
})
it('should guarantee the order of DOM nodes when performing actions', async () => {
function Example({ hide = false }) {
return (
<>
<Listbox value={undefined} onChange={console.log}>
<Listbox.Button>Trigger</Listbox.Button>
<Listbox.Options>
<Listbox.Option value="a">Option 1</Listbox.Option>
{!hide && <Listbox.Option value="b">Option 2</Listbox.Option>}
<Listbox.Option value="c">Option 3</Listbox.Option>
</Listbox.Options>
</Listbox>
</>
)
}
let { rerender } = render(<Example />)
// Open the Listbox
await click(getByText('Trigger'))
rerender(<Example hide={true} />) // Remove Listbox.Option 2
rerender(<Example hide={false} />) // Re-add Listbox.Option 2
assertListbox({ state: ListboxState.Visible })
let options = getListboxOptions()
// Focus the first item
await press(Keys.ArrowDown)
// Verify that the first menu item is active
assertActiveListboxOption(options[0])
await press(Keys.ArrowDown)
// Verify that the second menu item is active
assertActiveListboxOption(options[1])
await press(Keys.ArrowDown)
// Verify that the third menu item is active
assertActiveListboxOption(options[2])
})
})
describe('Rendering composition', () => {
packages/@headlessui-react/src/components/listbox/listbox.tsx
+14 -4
View File
@@ -146,10 +146,20 @@ let reducers: {
if (state.searchQuery === '') return state
return { ...state, searchQuery: '' }
},
[ActionTypes.RegisterOption]: (state, action) => ({
...state,
options: [...state.options, { id: action.id, dataRef: action.dataRef }],
}),
[ActionTypes.RegisterOption]: (state, action) => {
let orderMap = Array.from(
state.optionsRef.current?.querySelectorAll('[id^="headlessui-listbox-option-"]')!
).reduce(
(lookup, element, index) => Object.assign(lookup, { [element.id]: index }),
{}
) as Record<string, number>
let options = [...state.options, { id: action.id, dataRef: action.dataRef }].sort(
(a, z) => orderMap[a.id] - orderMap[z.id]
)
return { ...state, options }
},
[ActionTypes.UnregisterOption]: (state, action) => {
let nextOptions = state.options.slice()
let currentActiveOption =
packages/@headlessui-react/src/components/menu/menu.test.tsx
+43
View File
@@ -350,6 +350,49 @@ describe('Rendering', () => {
})
)
})
it('should guarantee the order of DOM nodes when performing actions', async () => {
function Example({ hide = false }) {
return (
<>
<Menu>
<Menu.Button>Trigger</Menu.Button>
<Menu.Items>
<Menu.Item as="button">Item 1</Menu.Item>
{!hide && <Menu.Item as="button">Item 2</Menu.Item>}
<Menu.Item as="button">Item 3</Menu.Item>
</Menu.Items>
</Menu>
</>
)
}
let { rerender } = render(<Example />)
// Open the Menu
await click(getByText('Trigger'))
rerender(<Example hide={true} />) // Remove Menu.Item 2
rerender(<Example hide={false} />) // Re-add Menu.Item 2
assertMenu({ state: MenuState.Visible })
let items = getMenuItems()
// Focus the first item
await press(Keys.ArrowDown)
// Verify that the first menu item is active
assertMenuLinkedWithMenuItem(items[0])
await press(Keys.ArrowDown)
// Verify that the second menu item is active
assertMenuLinkedWithMenuItem(items[1])
await press(Keys.ArrowDown)
// Verify that the third menu item is active
assertMenuLinkedWithMenuItem(items[2])
})
})
describe('Rendering composition', () => {
packages/@headlessui-react/src/components/menu/menu.tsx
+14 -4
View File
@@ -112,10 +112,20 @@ let reducers: {
if (state.searchQuery === '') return state
return { ...state, searchQuery: '' }
},
[ActionTypes.RegisterItem]: (state, action) => ({
...state,
items: [...state.items, { id: action.id, dataRef: action.dataRef }],
}),
[ActionTypes.RegisterItem]: (state, action) => {
let orderMap = Array.from(
state.itemsRef.current?.querySelectorAll('[id^="headlessui-menu-item-"]')!
).reduce(
(lookup, element, index) => Object.assign(lookup, { [element.id]: index }),
{}
) as Record<string, number>
let items = [...state.items, { id: action.id, dataRef: action.dataRef }].sort(
(a, z) => orderMap[a.id] - orderMap[z.id]
)
return { ...state, items }
},
[ActionTypes.UnregisterItem]: (state, action) => {
let nextItems = state.items.slice()
let currentActiveItem = state.activeItemIndex !== null ? nextItems[state.activeItemIndex] : null
packages/@headlessui-react/src/components/radio-group/radio-group.test.tsx
+31
View File
@@ -268,6 +268,37 @@ describe('Rendering', () => {
// Make sure that the onChange handler got called
expect(changeFn).toHaveBeenCalledTimes(1)
})
it('should guarantee the order of DOM nodes when performing actions', async () => {
function Example({ hide = false }) {
return (
<RadioGroup value={undefined} onChange={() => {}}>
<RadioGroup.Option value="a">Option 1</RadioGroup.Option>
{!hide && <RadioGroup.Option value="b">Option 2</RadioGroup.Option>}
<RadioGroup.Option value="c">Option 3</RadioGroup.Option>
</RadioGroup>
)
}
let { rerender } = render(<Example />)
// Focus the RadioGroup
await press(Keys.Tab)
rerender(<Example hide={true} />) // Remove RadioGroup.Option 2
rerender(<Example hide={false} />) // Re-add RadioGroup.Option 2
// Verify that the first radio group option is active
assertActiveElement(getByText('Option 1'))
await press(Keys.ArrowDown)
// Verify that the second radio group option is active
assertActiveElement(getByText('Option 2'))
await press(Keys.ArrowDown)
// Verify that the third radio group option is active
assertActiveElement(getByText('Option 3'))
})
})
describe('Keyboard interactions', () => {
packages/@headlessui-react/src/utils/focus-management.ts
+2 -2
View File
@@ -104,8 +104,8 @@ export function focusElement(element: HTMLElement | null) {
export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus) {
let elements = Array.isArray(container)
? container.slice().sort((a, b) => {
let position = a.compareDocumentPosition(b)
? container.slice().sort((a, z) => {
let position = a.compareDocumentPosition(z)
if (position & Node.DOCUMENT_POSITION_FOLLOWING) return -1
if (position & Node.DOCUMENT_POSITION_PRECEDING) return 1
packages/@headlessui-vue/src/components/listbox/listbox.test.tsx
+66 -1
View File
@@ -1,4 +1,4 @@
import { defineComponent, nextTick, ref, watch, h } from 'vue'
import { defineComponent, nextTick, ref, watch, h, reactive } from 'vue'
import { render } from '../../test-utils/vue-testing-library'
import { Listbox, ListboxLabel, ListboxButton, ListboxOptions, ListboxOption } from './listbox'
import { suppressConsoleLogs } from '../../test-utils/suppress-console-logs'
@@ -21,6 +21,7 @@ import {
getListboxOptions,
getListboxLabel,
ListboxState,
getByText,
} from '../../test-utils/accessibility-assertions'
import {
click,
@@ -46,6 +47,16 @@ beforeAll(() => {
afterAll(() => jest.restoreAllMocks())
function nextFrame() {
return new Promise(resolve => {
requestAnimationFrame(() => {
requestAnimationFrame(() => {
resolve()
})
})
})
}
function renderTemplate(input: string | Partial<Parameters<typeof defineComponent>[0]>) {
let defaultComponents = { Listbox, ListboxLabel, ListboxButton, ListboxOptions, ListboxOption }
@@ -571,6 +582,60 @@ describe('Rendering', () => {
})
)
})
it('should guarantee the order of DOM nodes when performing actions', async () => {
let props = reactive({ hide: false })
renderTemplate({
template: html`
<Listbox v-model="value">
<ListboxButton>Trigger</ListboxButton>
<ListboxOptions>
<ListboxOption value="a">Option 1</ListboxOption>
<ListboxOption v-if="!hide" value="b">Option 2</ListboxOption>
<ListboxOption value="c">Option 3</ListboxOption>
</ListboxOptions>
</Listbox>
`,
setup() {
return {
value: ref(null),
get hide() {
return props.hide
},
}
},
})
// Open the Listbox
await click(getByText('Trigger'))
props.hide = true
await nextFrame()
props.hide = false
await nextFrame()
assertListbox({ state: ListboxState.Visible })
let options = getListboxOptions()
// Focus the first option
await press(Keys.ArrowDown)
// Verify that the first listbox option is active
assertActiveListboxOption(options[0])
await press(Keys.ArrowDown)
// Verify that the second listbox option is active
assertActiveListboxOption(options[1])
await press(Keys.ArrowDown)
// Verify that the third listbox option is active
assertActiveListboxOption(options[2])
})
})
describe('Rendering composition', () => {
packages/@headlessui-vue/src/components/listbox/listbox.ts
+10 -1
View File
@@ -161,8 +161,17 @@ export let Listbox = defineComponent({
searchQuery.value = ''
},
registerOption(id: string, dataRef: ListboxOptionDataRef) {
let orderMap = Array.from(
optionsRef.value?.querySelectorAll('[id^="headlessui-listbox-option-"]') ?? []
).reduce(
(lookup, element, index) => Object.assign(lookup, { [element.id]: index }),
{}
) as Record<string, number>
// @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.push({ id, dataRef })
options.value = [...options.value, { id, dataRef }].sort(
(a, z) => orderMap[a.id] - orderMap[z.id]
)
},
unregisterOption(id: string) {
let nextOptions = options.value.slice()
packages/@headlessui-vue/src/components/menu/menu.test.tsx
+55 -1
View File
@@ -1,4 +1,4 @@
import { defineComponent, h, nextTick, ref, watch } from 'vue'
import { defineComponent, h, nextTick, reactive, ref, watch } from 'vue'
import { render } from '../../test-utils/vue-testing-library'
import { Menu, MenuButton, MenuItems, MenuItem } from './menu'
import { TransitionChild } from '../transitions/transition'
@@ -12,6 +12,7 @@ import {
assertMenuLinkedWithMenuItem,
assertActiveElement,
assertNoActiveMenuItem,
getByText,
getMenuButton,
getMenu,
getMenuItems,
@@ -719,6 +720,59 @@ describe('Rendering', () => {
await click(getMenuButton())
})
})
it('should guarantee the order of DOM nodes when performing actions', async () => {
let props = reactive({ hide: false })
renderTemplate({
template: jsx`
<Menu>
<MenuButton>Trigger</MenuButton>
<MenuItems>
<MenuItem as="button">Item 1</MenuItem>
<MenuItem v-if="!hide" as="button">Item 2</MenuItem>
<MenuItem as="button">Item 3</MenuItem>
</MenuItems>
</Menu>
`,
setup() {
return {
get hide() {
return props.hide
},
}
},
})
// Open the Menu
await click(getByText('Trigger'))
props.hide = true
await nextFrame()
props.hide = false
await nextFrame()
assertMenu({ state: MenuState.Visible })
let items = getMenuItems()
// Focus the first item
await press(Keys.ArrowDown)
// Verify that the first menu item is active
assertMenuLinkedWithMenuItem(items[0])
await press(Keys.ArrowDown)
// Verify that the second menu item is active
assertMenuLinkedWithMenuItem(items[1])
await press(Keys.ArrowDown)
// Verify that the third menu item is active
assertMenuLinkedWithMenuItem(items[2])
})
})
describe('Rendering composition', () => {
packages/@headlessui-vue/src/components/menu/menu.ts
+10 -1
View File
@@ -120,8 +120,17 @@ export let Menu = defineComponent({
searchQuery.value = ''
},
registerItem(id: string, dataRef: MenuItemDataRef) {
let orderMap = Array.from(
itemsRef.value?.querySelectorAll('[id^="headlessui-menu-item-"]') ?? []
).reduce(
(lookup, element, index) => Object.assign(lookup, { [element.id]: index }),
{}
) as Record<string, number>
// @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.push({ id, dataRef })
items.value = [...items.value, { id, dataRef }].sort(
(a, z) => orderMap[a.id] - orderMap[z.id]
)
},
unregisterItem(id: string) {
let nextItems = items.value.slice()
packages/@headlessui-vue/src/components/radio-group/radio-group.test.ts
+53 -1
View File
@@ -1,4 +1,4 @@
import { defineComponent, nextTick, ref, watch } from 'vue'
import { defineComponent, nextTick, ref, watch, reactive } from 'vue'
import { render } from '../../test-utils/vue-testing-library'
import { RadioGroup, RadioGroupOption, RadioGroupLabel, RadioGroupDescription } from './radio-group'
@@ -24,6 +24,16 @@ beforeAll(() => {
afterAll(() => jest.restoreAllMocks())
function nextFrame() {
return new Promise(resolve => {
requestAnimationFrame(() => {
requestAnimationFrame(() => {
resolve()
})
})
})
}
function renderTemplate(input: string | Partial<Parameters<typeof defineComponent>[0]>) {
let defaultComponents = { RadioGroup, RadioGroupOption, RadioGroupLabel, RadioGroupDescription }
@@ -454,6 +464,48 @@ describe('Rendering', () => {
// Make sure that the onChange handler got called
expect(changeFn).toHaveBeenCalledTimes(1)
})
it('should guarantee the order of DOM nodes when performing actions', async () => {
let props = reactive({ hide: false })
renderTemplate({
template: html`
<RadioGroup v-model="value">
<RadioGroupOption value="a">Option 1</RadioGroupOption>
<RadioGroupOption v-if="!hide" value="b">Option 2</RadioGroupOption>
<RadioGroupOption value="c">Option 3</RadioGroupOption>
</RadioGroup>
`,
setup() {
return {
value: ref('a'),
get hide() {
return props.hide
},
}
},
})
// Focus the RadioGroup
await press(Keys.Tab)
props.hide = true
await nextFrame()
props.hide = false
await nextFrame()
// Verify that the first radio group option is active
assertActiveElement(getByText('Option 1'))
await press(Keys.ArrowDown)
// Verify that the second radio group option is active
assertActiveElement(getByText('Option 2'))
await press(Keys.ArrowDown)
// Verify that the third radio group option is active
assertActiveElement(getByText('Option 3'))
})
})
describe('Keyboard interactions', () => {
packages/@headlessui-vue/src/utils/focus-management.ts
+2 -2
View File
@@ -97,8 +97,8 @@ export function focusElement(element: HTMLElement | null) {
export function focusIn(container: HTMLElement | HTMLElement[], focus: Focus) {
let elements = Array.isArray(container)
? container.slice().sort((a, b) => {
let position = a.compareDocumentPosition(b)
? container.slice().sort((a, z) => {
let position = a.compareDocumentPosition(z)
if (position & Node.DOCUMENT_POSITION_FOLLOWING) return -1
if (position & Node.DOCUMENT_POSITION_PRECEDING) return 1