Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve control over Menu and Listbox options while searching #2471

Merged
merged 9 commits into from
May 4, 2023
6 changes: 5 additions & 1 deletion jest/create-jest-config.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ module.exports = function createJestConfig(root, options) {
return Object.assign(
{
rootDir: root,
setupFilesAfterEnv: ['<rootDir>../../jest/custom-matchers.ts', ...setupFilesAfterEnv],
setupFilesAfterEnv: [
'<rootDir>../../jest/custom-matchers.ts',
'<rootDir>../../jest/polyfills.ts',
...setupFilesAfterEnv,
],
transform: {
'^.+\\.(t|j)sx?$': '@swc/jest',
...transform,
Expand Down
15 changes: 15 additions & 0 deletions jest/polyfills.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// JSDOM Doesn't implement innerText yet: https://github.com/jsdom/jsdom/issues/1245
// So this is a hacky way of implementing it using `textContent`.
// Real implementation doesn't use textContent because:
// > textContent gets the content of all elements, including <script> and <style> elements. In
// > contrast, innerText only shows "human-readable" elements.
// >
// > — https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent#differences_from_innertext
Object.defineProperty(HTMLElement.prototype, 'innerText', {
get() {
return this.textContent
},
set(value) {
this.textContent = value
},
})
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Ensure `FocusTrap` is only active when the given `enabled` value is `true` ([#2456](https://github.com/tailwindlabs/headlessui/pull/2456))
- Stop `<Transition appear>` from overwriting classes on re-render ([#2457](https://github.com/tailwindlabs/headlessui/pull/2457))
- Improve control over `Menu` and `Listbox` options while searching ([#2471](https://github.com/tailwindlabs/headlessui/pull/2471))

## [1.7.14] - 2023-04-12

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { useEvent } from '../../hooks/use-event'
import { useControllable } from '../../hooks/use-controllable'
import { useLatestValue } from '../../hooks/use-latest-value'
import { useTrackedPointer } from '../../hooks/use-tracked-pointer'
import { getTextValue } from '../../utils/get-text-value'

enum ListboxStates {
Open,
Expand Down Expand Up @@ -939,7 +940,11 @@ function OptionFn<
value,
domRef: internalOptionRef,
get textValue() {
return internalOptionRef.current?.textContent?.toLowerCase()
let element = internalOptionRef.current
if (element) {
return getTextValue(element).trim().toLowerCase()
}
return ''
},
})
let optionRef = useSyncRefs(ref, internalOptionRef)
Expand Down
16 changes: 12 additions & 4 deletions packages/@headlessui-react/src/components/menu/menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { useResolveButtonType } from '../../hooks/use-resolve-button-type'
import { useOwnerDocument } from '../../hooks/use-owner'
import { useEvent } from '../../hooks/use-event'
import { useTrackedPointer } from '../../hooks/use-tracked-pointer'
import { getTextValue } from '../../utils/get-text-value'

enum MenuStates {
Open,
Expand Down Expand Up @@ -636,14 +637,21 @@ function ItemFn<TTag extends ElementType = typeof DEFAULT_ITEM_TAG>(
/* We also want to trigger this when the position of the active item changes so that we can re-trigger the scrollIntoView */ state.activeItemIndex,
])

let bag = useRef<MenuItemDataRef['current']>({ disabled, domRef: internalItemRef })
let bag = useRef<MenuItemDataRef['current']>({
disabled,
domRef: internalItemRef,
get textValue() {
let element = internalItemRef.current
if (element) {
return getTextValue(element).trim().toLowerCase()
}
return ''
},
})

useIsoMorphicEffect(() => {
bag.current.disabled = disabled
}, [bag, disabled])
useIsoMorphicEffect(() => {
bag.current.textValue = internalItemRef.current?.textContent?.toLowerCase()
}, [bag, internalItemRef])

useIsoMorphicEffect(() => {
dispatch({ type: ActionTypes.RegisterItem, id, dataRef: bag })
Expand Down
95 changes: 95 additions & 0 deletions packages/@headlessui-react/src/utils/get-text-value.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { getTextValue } from './get-text-value'

let html = String.raw

it('should be possible to get the text value from an element', () => {
let element = document.createElement('div')
element.innerText = 'Hello World'
expect(getTextValue(element)).toEqual('Hello World')
})

it('should strip out emojis when receiving the text from the element', () => {
let element = document.createElement('div')
element.innerText = '🇨🇦 Canada'
expect(getTextValue(element)).toEqual('Canada')
})

it('should strip out hidden elements', () => {
let element = document.createElement('div')
element.innerHTML = html`<div><span hidden>Hello</span> world</div>`
expect(getTextValue(element)).toEqual('world')
})

it('should strip out aria-hidden elements', () => {
let element = document.createElement('div')
element.innerHTML = html`<div><span aria-hidden>Hello</span> world</div>`
expect(getTextValue(element)).toEqual('world')
})

it('should strip out role="img" elements', () => {
let element = document.createElement('div')
element.innerHTML = html`<div><span role="img">°</span> world</div>`
expect(getTextValue(element)).toEqual('world')
})

it('should be possible to get the text value from the aria-label', () => {
let element = document.createElement('div')
element.setAttribute('aria-label', 'Hello World')
expect(getTextValue(element)).toEqual('Hello World')
})

it('should be possible to get the text value from the aria-label (even if there is content)', () => {
let element = document.createElement('div')
element.setAttribute('aria-label', 'Hello World')
element.innerHTML = 'Hello Universe'
element.innerText = 'Hello Universe'
expect(getTextValue(element)).toEqual('Hello World')
})

it('should be possible to get the text value from the element referenced by aria-labelledby (using `aria-label`)', () => {
document.body.innerHTML = html`
<div>
<div id="foo" aria-labelledby="bar">Contents of foo</div>
<div id="bar" aria-label="Actual value of bar">Contents of bar</div>
</div>
`

expect(getTextValue(document.querySelector('#foo')!)).toEqual('Actual value of bar')
})

it('should be possible to get the text value from the element referenced by aria-labelledby (using its contents)', () => {
document.body.innerHTML = html`
<div>
<div id="foo" aria-labelledby="bar">Contents of foo</div>
<div id="bar">Contents of bar</div>
</div>
`

expect(getTextValue(document.querySelector('#foo')!)).toEqual('Contents of bar')
})

it('should be possible to get the text value from the element referenced by aria-labelledby (using `aria-label`, multiple)', () => {
document.body.innerHTML = html`
<div>
<div id="foo" aria-labelledby="bar baz">Contents of foo</div>
<div id="bar" aria-label="Actual value of bar">Contents of bar</div>
<div id="baz" aria-label="Actual value of baz">Contents of baz</div>
</div>
`

expect(getTextValue(document.querySelector('#foo')!)).toEqual(
'Actual value of bar, Actual value of baz'
)
})

it('should be possible to get the text value from the element referenced by aria-labelledby (using its contents, multiple)', () => {
document.body.innerHTML = html`
<div>
<div id="foo" aria-labelledby="bar baz">Contents of foo</div>
<div id="bar">Contents of bar</div>
<div id="baz">Contents of baz</div>
</div>
`

expect(getTextValue(document.querySelector('#foo')!)).toEqual('Contents of bar, Contents of baz')
})
79 changes: 79 additions & 0 deletions packages/@headlessui-react/src/utils/get-text-value.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
function getTextContents(element: HTMLElement): string {
// Using innerText instad of textContent because:
//
// > textContent gets the content of all elements, including <script> and <style> elements. In
// > contrast, innerText only shows "human-readable" elements.
// >
// > — https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent#differences_from_innertext
let value = element.innerText ?? ''

// Check if it contains some emojis or not, if so, we need to remove them
// because ideally we work with simple text values.

// Ideally we can use the much simpler RegEx: /\p{Extended_Pictographic}/u
// but we can't rely on this yet, so we use the more complex one.
if (
/([\u2700-\u27BF]|[\uE000-\uF8FF]|\uD83C[\uDC00-\uDFFF]|\uD83D[\uDC00-\uDFFF]|[\u2011-\u26FF]|\uD83E[\uDD10-\uDDFF])/.test(
value
)
) {
value = value.replace(
/([\u2700-\u27BF]|[\uE000-\uF8FF]|\uD83C[\uDC00-\uDFFF]|\uD83D[\uDC00-\uDFFF]|[\u2011-\u26FF]|\uD83E[\uDD10-\uDDFF])/g,
''
)
}
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved

// Remove all the elements that shouldn't be there.
//
// [hidden] — The user doesn't see it
// [aria-hidden] — The screen reader doesn't see it
// [role="img"] — Even if it is text, it is used as an image
//
// This is probably the slowest part, but if you want complete control over the text value,
// you better set an `aria-label` instead.
let children = element.querySelectorAll('[hidden],[aria-hidden],[role="img"]')
if (children.length > 0) {
for (let el of children) {
if (el instanceof HTMLElement) {
value = value.replace(el.innerText ?? '', '')
}
}
}
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved

return value
}

export function getTextValue(element: HTMLElement): string {
// Try to use the `aria-label` first
let label = element.getAttribute('aria-label')
if (typeof label === 'string') return label.trim()

// Try to use the `aria-labelledby` second
let labelledby = element.getAttribute('aria-labelledby')
if (labelledby) {
// aria-labelledby can be a space-separated list of IDs, so we need to split them up and
// combine them into a single string.
let labels = labelledby
.split(' ')
.map((labelledby) => {
let labelEl = document.getElementById(labelledby)
if (labelEl) {
let label = labelEl.getAttribute('aria-label')
// Try to use the `aria-label` first (of the referenced element)
if (typeof label === 'string') return label.trim()

// This time, the `aria-labelledby` isn't used anymore (in Safari), so we just have to
// look at the contents itself.
return getTextContents(labelEl).trim()
}

return null
})
.filter(Boolean)

if (labels.length > 0) return labels.join(', ')
}

// Try to use the text contents of the element itself
return getTextContents(element).trim()
}
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix memory leak in `Popover` component ([#2430](https://github.com/tailwindlabs/headlessui/pull/2430))
- Ensure `FocusTrap` is only active when the given `enabled` value is `true` ([#2456](https://github.com/tailwindlabs/headlessui/pull/2456))
- Ensure the exposed `activeIndex` is up to date for the `Combobox` component ([#2463](https://github.com/tailwindlabs/headlessui/pull/2463))
- Improve control over `Menu` and `Listbox` options while searching ([#2471](https://github.com/tailwindlabs/headlessui/pull/2471))

## [1.7.13] - 2023-04-12

Expand Down
13 changes: 8 additions & 5 deletions packages/@headlessui-vue/src/components/listbox/listbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { Hidden, Features as HiddenFeatures } from '../../internal/hidden'
import { objectToFormEntries } from '../../utils/form'
import { useControllable } from '../../hooks/use-controllable'
import { useTrackedPointer } from '../../hooks/use-tracked-pointer'
import { getTextValue } from '../../utils/get-text-value'

function defaultComparator<T>(a: T, z: T): boolean {
return a === z
Expand Down Expand Up @@ -734,13 +735,15 @@ export let ListboxOption = defineComponent({
let dataRef = computed<ListboxOptionData>(() => ({
disabled: props.disabled,
value: props.value,
textValue: '',
get textValue() {
let element = dom(internalOptionRef)
if (element) {
return getTextValue(element).trim().toLowerCase()
}
return ''
},
domRef: internalOptionRef,
}))
onMounted(() => {
let textValue = dom(internalOptionRef)?.textContent?.toLowerCase().trim()
if (textValue !== undefined) dataRef.value.textValue = textValue
})

onMounted(() => api.registerOption(props.id, dataRef))
onUnmounted(() => api.unregisterOption(props.id))
Expand Down
13 changes: 8 additions & 5 deletions packages/@headlessui-vue/src/components/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
} from '../../utils/focus-management'
import { useOutsideClick } from '../../hooks/use-outside-click'
import { useTrackedPointer } from '../../hooks/use-tracked-pointer'
import { getTextValue } from '../../utils/get-text-value'

enum MenuStates {
Open,
Expand Down Expand Up @@ -513,13 +514,15 @@ export let MenuItem = defineComponent({

let dataRef = computed<MenuItemData>(() => ({
disabled: props.disabled,
textValue: '',
get textValue() {
let element = dom(internalItemRef)
if (element) {
return getTextValue(element).trim().toLowerCase()
}
return ''
},
domRef: internalItemRef,
}))
onMounted(() => {
let textValue = dom(internalItemRef)?.textContent?.toLowerCase().trim()
if (textValue !== undefined) dataRef.value.textValue = textValue
})

onMounted(() => api.registerItem(props.id, dataRef))
onUnmounted(() => api.unregisterItem(props.id))
Expand Down