Skip to content

Commit

Permalink
Fix ref element (#249)
Browse files Browse the repository at this point in the history 
* add small dom utility to resolve the dom node from a ref

* use dom() to resolve underlying DOM node

There is probably a better way to do this, the idea is that we apply a
ref to the component. However by default for html components
`yourRef.value` will be the underlying DOM node. However if you pass the
ref to another component, the actual DOM node will be located at
`yourRef.value.$el`.

Fixes: #21

* update changelog
  • Loading branch information
@RobinMalfait
RobinMalfait authored Feb 19, 2021
1 parent c4549e4 commit 2505c5f
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 31 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased - Vue]

### Fixes

- Fix incorrect `DOM` node from ref ([#249](https://github.com/tailwindlabs/headlessui/pull/249))

### Added

- Add `SwitchDescription` component, which adds the `aria-describedby` to the actual Switch ([#220](https://github.com/tailwindlabs/headlessui/pull/220))
Expand Down
29 changes: 15 additions & 14 deletions packages/@headlessui-vue/src/components/listbox/listbox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { useId } from '../../hooks/use-id'
import { Keys } from '../../keyboard'
import { calculateActiveIndex, Focus } from '../../utils/calculate-active-index'
import { resolvePropValue } from '../../utils/resolve-prop-value'
import { dom } from '../../utils/dom'

enum ListboxStates {
Open,
Expand Down Expand Up @@ -184,11 +185,11 @@ export let Listbox = defineComponent({
let active = document.activeElement

if (listboxState.value !== ListboxStates.Open) return
if (buttonRef.value?.contains(target)) return
if (dom(buttonRef)?.contains(target)) return

if (!optionsRef.value?.contains(target)) api.closeListbox()
if (!dom(optionsRef)?.contains(target)) api.closeListbox()
if (active !== document.body && active?.contains(target)) return // Keep focus on newly clicked/focused element
if (!event.defaultPrevented) buttonRef.value?.focus({ preventScroll: true })
if (!event.defaultPrevented) dom(buttonRef)?.focus({ preventScroll: true })
}

window.addEventListener('mousedown', handler)
Expand Down Expand Up @@ -231,7 +232,7 @@ export let ListboxLabel = defineComponent({
id,
el: api.labelRef,
handleClick() {
api.buttonRef.value?.focus({ preventScroll: true })
dom(api.buttonRef)?.focus({ preventScroll: true })
},
}
},
Expand All @@ -253,10 +254,10 @@ export let ListboxButton = defineComponent({
id: this.id,
type: 'button',
'aria-haspopup': true,
'aria-controls': api.optionsRef.value?.id,
'aria-controls': dom(api.optionsRef)?.id,
'aria-expanded': api.listboxState.value === ListboxStates.Open ? true : undefined,
'aria-labelledby': api.labelRef.value
? [api.labelRef.value.id, this.id].join(' ')
? [dom(api.labelRef)?.id, this.id].join(' ')
: undefined,
disabled: api.disabled,
onKeyDown: this.handleKeyDown,
Expand Down Expand Up @@ -284,7 +285,7 @@ export let ListboxButton = defineComponent({
event.preventDefault()
api.openListbox()
nextTick(() => {
api.optionsRef.value?.focus({ preventScroll: true })
dom(api.optionsRef)?.focus({ preventScroll: true })
if (!api.value.value) api.goToOption(Focus.First)
})
break
Expand All @@ -293,7 +294,7 @@ export let ListboxButton = defineComponent({
event.preventDefault()
api.openListbox()
nextTick(() => {
api.optionsRef.value?.focus({ preventScroll: true })
dom(api.optionsRef)?.focus({ preventScroll: true })
if (!api.value.value) api.goToOption(Focus.Last)
})
break
Expand All @@ -304,11 +305,11 @@ export let ListboxButton = defineComponent({
if (api.disabled) return
if (api.listboxState.value === ListboxStates.Open) {
api.closeListbox()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
} else {
event.preventDefault()
api.openListbox()
nextFrame(() => api.optionsRef.value?.focus({ preventScroll: true }))
nextFrame(() => dom(api.optionsRef)?.focus({ preventScroll: true }))
}
}

Expand All @@ -334,7 +335,7 @@ export let ListboxOptions = defineComponent({
api.activeOptionIndex.value === null
? undefined
: api.options.value[api.activeOptionIndex.value]?.id,
'aria-labelledby': api.labelRef.value?.id ?? api.buttonRef.value?.id,
'aria-labelledby': dom(api.labelRef)?.id ?? dom(api.buttonRef)?.id,
id: this.id,
onKeyDown: this.handleKeyDown,
role: 'listbox',
Expand Down Expand Up @@ -377,7 +378,7 @@ export let ListboxOptions = defineComponent({
api.select(dataRef.value)
}
api.closeListbox()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
break

case Keys.ArrowDown:
Expand All @@ -401,7 +402,7 @@ export let ListboxOptions = defineComponent({
case Keys.Escape:
event.preventDefault()
api.closeListbox()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
break

case Keys.Tab:
Expand Down Expand Up @@ -477,7 +478,7 @@ export let ListboxOption = defineComponent({
if (disabled) return event.preventDefault()
api.select(value)
api.closeListbox()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
}

function handleFocus() {
Expand Down
40 changes: 40 additions & 0 deletions packages/@headlessui-vue/src/components/menu/menu.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,46 @@ describe('Rendering', () => {
assertMenu({ state: MenuState.Visible })
})

it('should be possible to render a MenuButton using a template `as` prop and a custom element', async () => {
renderTemplate({
template: `
<Menu>
<MenuButton as="template" v-slot="{ open }">
<MyCustomButton :data-open="open">Options</MyCustomButton>
</MenuButton>
<MenuItems>
<MenuItem>Item A</MenuItem>
<MenuItem>Item B</MenuItem>
<MenuItem>Item C</MenuItem>
</MenuItems>
</Menu>
`,
components: {
MyCustomButton: defineComponent({
setup(_, { slots }) {
return () => {
return h('button', slots.default?.())
}
},
}),
},
})

assertMenuButton({
state: MenuState.InvisibleUnmounted,
attributes: { id: 'headlessui-menu-button-1', 'data-open': 'false' },
})
assertMenu({ state: MenuState.InvisibleUnmounted })

await click(getMenuButton())

assertMenuButton({
state: MenuState.Visible,
attributes: { id: 'headlessui-menu-button-1', 'data-open': 'true' },
})
assertMenu({ state: MenuState.Visible })
})

it(
'should yell when we render a MenuButton using a template `as` prop that contains multiple children',
suppressConsoleLogs(() => {
Expand Down
27 changes: 14 additions & 13 deletions packages/@headlessui-vue/src/components/menu/menu.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { useId } from '../../hooks/use-id'
import { Keys } from '../../keyboard'
import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index'
import { resolvePropValue } from '../../utils/resolve-prop-value'
import { dom } from '../../utils/dom'

enum MenuStates {
Open,
Expand Down Expand Up @@ -141,11 +142,11 @@ export let Menu = defineComponent({
let active = document.activeElement

if (menuState.value !== MenuStates.Open) return
if (buttonRef.value?.contains(target)) return
if (dom(buttonRef)?.contains(target)) return

if (!itemsRef.value?.contains(target)) api.closeMenu()
if (!dom(itemsRef)?.contains(target)) api.closeMenu()
if (active !== document.body && active?.contains(target)) return // Keep focus on newly clicked/focused element
if (!event.defaultPrevented) buttonRef.value?.focus({ preventScroll: true })
if (!event.defaultPrevented) dom(buttonRef)?.focus({ preventScroll: true })
}

window.addEventListener('mousedown', handler)
Expand Down Expand Up @@ -176,7 +177,7 @@ export let MenuButton = defineComponent({
id: this.id,
type: 'button',
'aria-haspopup': true,
'aria-controls': api.itemsRef.value?.id,
'aria-controls': dom(api.itemsRef)?.id,
'aria-expanded': api.menuState.value === MenuStates.Open ? true : undefined,
onKeyDown: this.handleKeyDown,
onClick: this.handleClick,
Expand All @@ -203,7 +204,7 @@ export let MenuButton = defineComponent({
event.preventDefault()
api.openMenu()
nextTick(() => {
api.itemsRef.value?.focus({ preventScroll: true })
dom(api.itemsRef)?.focus({ preventScroll: true })
api.goToItem(Focus.First)
})
break
Expand All @@ -212,7 +213,7 @@ export let MenuButton = defineComponent({
event.preventDefault()
api.openMenu()
nextTick(() => {
api.itemsRef.value?.focus({ preventScroll: true })
dom(api.itemsRef)?.focus({ preventScroll: true })
api.goToItem(Focus.Last)
})
break
Expand All @@ -223,11 +224,11 @@ export let MenuButton = defineComponent({
if (props.disabled) return
if (api.menuState.value === MenuStates.Open) {
api.closeMenu()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
} else {
event.preventDefault()
api.openMenu()
nextFrame(() => api.itemsRef.value?.focus({ preventScroll: true }))
nextFrame(() => dom(api.itemsRef)?.focus({ preventScroll: true }))
}
}

Expand Down Expand Up @@ -255,7 +256,7 @@ export let MenuItems = defineComponent({
api.activeItemIndex.value === null
? undefined
: api.items.value[api.activeItemIndex.value]?.id,
'aria-labelledby': api.buttonRef.value?.id,
'aria-labelledby': dom(api.buttonRef)?.id,
id: this.id,
onKeyDown: this.handleKeyDown,
role: 'menu',
Expand All @@ -279,7 +280,7 @@ export let MenuItems = defineComponent({
let searchDebounce = ref<ReturnType<typeof setTimeout> | null>(null)

watchEffect(() => {
let container = api.itemsRef.value
let container = dom(api.itemsRef)
if (!container) return
if (api.menuState.value !== MenuStates.Open) return

Expand Down Expand Up @@ -316,7 +317,7 @@ export let MenuItems = defineComponent({
document.getElementById(id)?.click()
}
api.closeMenu()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
break

case Keys.ArrowDown:
Expand All @@ -340,7 +341,7 @@ export let MenuItems = defineComponent({
case Keys.Escape:
event.preventDefault()
api.closeMenu()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
break

case Keys.Tab:
Expand Down Expand Up @@ -398,7 +399,7 @@ export let MenuItem = defineComponent({
function handleClick(event: MouseEvent) {
if (disabled) return event.preventDefault()
api.closeMenu()
nextTick(() => api.buttonRef.value?.focus({ preventScroll: true }))
nextTick(() => dom(api.buttonRef)?.focus({ preventScroll: true }))
}

function handleFocus() {
Expand Down
11 changes: 7 additions & 4 deletions packages/@headlessui-vue/src/components/switch/switch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { render } from '../../utils/render'
import { useId } from '../../hooks/use-id'
import { Keys } from '../../keyboard'
import { resolvePropValue } from '../../utils/resolve-prop-value'
import { dom } from '../../utils/dom'

type StateDefinition = {
// State
Expand Down Expand Up @@ -61,8 +62,8 @@ export let Switch = defineComponent({
let api = inject(GroupContext, null)
let { class: defaultClass, className = defaultClass } = this.$props

let labelledby = computed(() => api?.labelRef.value?.id)
let describedby = computed(() => api?.descriptionRef.value?.id)
let labelledby = computed(() => dom(api?.labelRef)?.id)
let describedby = computed(() => dom(api?.descriptionRef)?.id)

let slot = { checked: this.$props.modelValue }
let propsWeControl = {
Expand Down Expand Up @@ -144,8 +145,10 @@ export let SwitchLabel = defineComponent({
id,
el: api.labelRef,
handleClick() {
api.switchRef.value?.click()
api.switchRef.value?.focus({ preventScroll: true })
let el = dom(api.switchRef)

el?.click()
el?.focus({ preventScroll: true })
},
}
},
Expand Down
7 changes: 7 additions & 0 deletions packages/@headlessui-vue/src/utils/dom.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { Ref } from 'vue'

export function dom<T extends HTMLElement>(ref?: Ref<T | null>): T | null {
if (ref == null) return null
if (ref.value == null) return null
return ((ref as Ref<T & { $el: unknown }>).value.$el ?? ref.value) as T | null
}

0 comments on commit 2505c5f

Please sign in to comment.