fix: simplify skintone listbox (#336)
This commit is contained in:
parent
a414e08e6c
commit
7d9096b06c
|
@ -31,8 +31,13 @@
|
|||
<label class="sr-only" for="search">{i18n.searchLabel}</label>
|
||||
<span id="search-description" class="sr-only">{i18n.searchDescription}</span>
|
||||
</div>
|
||||
<!-- For the pattern used for the skintone dropdown, see
|
||||
https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only/ -->
|
||||
<!-- For the pattern used for the skintone dropdown, see:
|
||||
https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only/
|
||||
The one case where we deviate from the example is that we move focus from the button to the
|
||||
listbox. (The example uses a combobox, so it's not exactly the same.) This was tested in NVDA and VoiceOver.
|
||||
Note that Svelte's a11y checker will warn if the listbox does not have a tabindex.
|
||||
https://github.com/sveltejs/svelte/blob/3bc791b/site/content/docs/06-accessibility-warnings.md#a11y-aria-activedescendant-has-tabindex
|
||||
-->
|
||||
<div class="skintone-button-wrapper {skinTonePickerExpandedAfterAnimation ? 'expanded' : ''}">
|
||||
<button id="skintone-button"
|
||||
class="emoji {skinTonePickerExpanded ? 'hide-focus' : ''}"
|
||||
|
@ -47,31 +52,25 @@
|
|||
</button>
|
||||
</div>
|
||||
<span id="skintone-description" class="sr-only">{i18n.skinToneDescription}</span>
|
||||
<!-- In this pattern, the listbox is not focusable, only the button is focusable.
|
||||
This was tested in NVDA and VoiceOver and worked in both. -->
|
||||
<!-- svelte-ignore a11y-aria-activedescendant-has-tabindex -->
|
||||
<div id="skintone-list"
|
||||
class="skintone-list {skinTonePickerExpanded ? '' : 'hidden no-animate'}"
|
||||
class="skintone-list hide-focus {skinTonePickerExpanded ? '' : 'hidden no-animate'}"
|
||||
style="transform:translateY({ skinTonePickerExpanded ? 0 : 'calc(-1 * var(--num-skintones) * var(--total-emoji-size))'})"
|
||||
role="listbox"
|
||||
aria-label={i18n.skinTonesLabel}
|
||||
aria-activedescendant="skintone-{activeSkinTone}"
|
||||
aria-hidden={!skinTonePickerExpanded}
|
||||
tabindex="-1"
|
||||
on:focusout={onSkinToneOptionsFocusOut}
|
||||
on:click={onSkinToneOptionsClick}
|
||||
on:keydown={onSkinToneOptionsKeydown}
|
||||
on:keyup={onSkinToneOptionsKeyup}
|
||||
bind:this={skinToneDropdown}>
|
||||
{#each skinTones as skinTone, i (skinTone)}
|
||||
<!-- would use a button here, but iOS Safari misreports relatedTarget in that case, see issue #14 -->
|
||||
<!-- see https://stackoverflow.com/a/42764495/680742 -->
|
||||
<!-- see also https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus -->
|
||||
<div id="skintone-{i}"
|
||||
class="emoji hide-focus {i === activeSkinTone ? 'active' : ''}"
|
||||
class="emoji {i === activeSkinTone ? 'active' : ''}"
|
||||
aria-selected={i === activeSkinTone}
|
||||
role="option"
|
||||
title={i18n.skinTones[i]}
|
||||
tabindex="-1"
|
||||
aria-label={i18n.skinTones[i]}>
|
||||
{skinTone}
|
||||
</div>
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
/* eslint-disable prefer-const,no-labels,no-inner-declarations */
|
||||
import { onMount, tick } from 'svelte'
|
||||
import { onMount } from 'svelte'
|
||||
import { groups as defaultGroups, customGroup } from '../../groups'
|
||||
import { MIN_SEARCH_TEXT_LENGTH, NUM_SKIN_TONES } from '../../../shared/constants'
|
||||
import { requestIdleCallback } from '../../utils/requestIdleCallback'
|
||||
|
@ -89,9 +89,6 @@ const labelWithSkin = (emoji, currentSkinTone) => (
|
|||
uniq([(emoji.name || unicodeWithSkin(emoji, currentSkinTone)), ...(emoji.shortcodes || [])]).join(', ')
|
||||
)
|
||||
|
||||
// Detect a skintone option button
|
||||
const isSkinToneOption = element => /^skintone-/.test(element.id)
|
||||
|
||||
//
|
||||
// Determine the emoji support level (in requestIdleCallback)
|
||||
//
|
||||
|
@ -503,13 +500,7 @@ async function onEmojiClick (event) {
|
|||
//
|
||||
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
async function onSkinToneOptionsClick (event) {
|
||||
const { target } = event
|
||||
if (!isSkinToneOption(target)) {
|
||||
return
|
||||
}
|
||||
halt(event)
|
||||
const skinTone = parseInt(target.id.slice(9), 10) // remove 'skintone-' prefix
|
||||
function changeSkinTone (skinTone) {
|
||||
currentSkinTone = skinTone
|
||||
skinTonePickerExpanded = false
|
||||
focus('skintone-button')
|
||||
|
@ -518,12 +509,24 @@ async function onSkinToneOptionsClick (event) {
|
|||
}
|
||||
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
async function onClickSkinToneButton (event) {
|
||||
function onSkinToneOptionsClick (event) {
|
||||
const { target: { id } } = event
|
||||
const match = id && id.match(/^skintone-(\d)/) // skintone option format
|
||||
if (!match) { // not a skintone option
|
||||
return
|
||||
}
|
||||
halt(event)
|
||||
const skinTone = parseInt(match[1], 10) // remove 'skintone-' prefix
|
||||
changeSkinTone(skinTone)
|
||||
}
|
||||
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
function onClickSkinToneButton (event) {
|
||||
skinTonePickerExpanded = !skinTonePickerExpanded
|
||||
activeSkinTone = currentSkinTone
|
||||
if (skinTonePickerExpanded) {
|
||||
halt(event)
|
||||
requestAnimationFrame(() => focus(`skintone-${activeSkinTone}`))
|
||||
requestAnimationFrame(() => focus('skintone-list'))
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -549,8 +552,6 @@ function onSkinToneOptionsKeydown (event) {
|
|||
const changeActiveSkinTone = async nextSkinTone => {
|
||||
halt(event)
|
||||
activeSkinTone = nextSkinTone
|
||||
await tick()
|
||||
focus(`skintone-${activeSkinTone}`)
|
||||
}
|
||||
|
||||
switch (event.key) {
|
||||
|
@ -565,7 +566,8 @@ function onSkinToneOptionsKeydown (event) {
|
|||
case 'Enter':
|
||||
// enter on keydown, space on keyup. this is just how browsers work for buttons
|
||||
// https://lists.w3.org/Archives/Public/w3c-wai-ig/2019JanMar/0086.html
|
||||
return onSkinToneOptionsClick(event)
|
||||
halt(event)
|
||||
return changeSkinTone(activeSkinTone)
|
||||
case 'Escape':
|
||||
halt(event)
|
||||
skinTonePickerExpanded = false
|
||||
|
@ -582,16 +584,16 @@ function onSkinToneOptionsKeyup (event) {
|
|||
case ' ':
|
||||
// enter on keydown, space on keyup. this is just how browsers work for buttons
|
||||
// https://lists.w3.org/Archives/Public/w3c-wai-ig/2019JanMar/0086.html
|
||||
return onSkinToneOptionsClick(event)
|
||||
halt(event)
|
||||
return changeSkinTone(activeSkinTone)
|
||||
}
|
||||
}
|
||||
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
async function onSkinToneOptionsFocusOut (event) {
|
||||
// On blur outside of the skintone options, collapse the skintone picker.
|
||||
// Except if focus is just moving to another skintone option, e.g. pressing up/down to change focus
|
||||
// On blur outside of the skintone listbox, collapse the skintone picker.
|
||||
const { relatedTarget } = event
|
||||
if (!relatedTarget || !isSkinToneOption(relatedTarget)) {
|
||||
if (!relatedTarget || relatedTarget.id !== 'skintone-list') {
|
||||
skinTonePickerExpanded = false
|
||||
}
|
||||
}
|
||||
|
|
|
@ -21,7 +21,7 @@ describe('Picker tests', () => {
|
|||
}
|
||||
}
|
||||
})
|
||||
const { getAllByRole, getByRole, queryAllByRole } = proxy
|
||||
const { getAllByRole, getByRole, queryAllByRole, queryByRole } = proxy
|
||||
|
||||
const activeElement = () => container.getRootNode().activeElement
|
||||
|
||||
|
@ -85,13 +85,23 @@ describe('Picker tests', () => {
|
|||
})
|
||||
|
||||
await openSkintoneListbox(container)
|
||||
await waitFor(() => expect(getByRole('option', { name: 'Default', selected: true }))
|
||||
.toBe(activeElement()))
|
||||
|
||||
await waitFor(() => (
|
||||
expect(
|
||||
getByRole('option', { name: 'Default', selected: true }).id)
|
||||
.toBe(queryByRole('listbox', { name: 'Skin tones' }).getAttribute('aria-activedescendant'))
|
||||
)
|
||||
)
|
||||
|
||||
const pressKeyAndExpectActiveOption = async (key, name) => {
|
||||
await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))) // delay
|
||||
await fireEvent.keyDown(activeElement(), { key, code: key })
|
||||
await waitFor(() => expect(getByRole('option', { name, selected: true })).toBe(activeElement()))
|
||||
await waitFor(() => {
|
||||
const selectedOption = getByRole('option', { name, selected: true })
|
||||
return expect(selectedOption.id).toBe(
|
||||
queryByRole('listbox', { name: 'Skin tones' }).getAttribute('aria-activedescendant')
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
await pressKeyAndExpectActiveOption('ArrowDown', 'Light')
|
||||
|
@ -106,7 +116,7 @@ describe('Picker tests', () => {
|
|||
await pressKeyAndExpectActiveOption('End', 'Dark')
|
||||
await pressKeyAndExpectActiveOption('End', 'Dark')
|
||||
|
||||
await fireEvent.click(activeElement(), { key: 'Enter', code: 'Enter' })
|
||||
await fireEvent.keyDown(activeElement(), { key: 'Enter', code: 'Enter' })
|
||||
|
||||
await waitFor(() => expect(event && event.detail).toStrictEqual({ skinTone: 5 }))
|
||||
await waitFor(() => expect(queryAllByRole('listbox', { name: 'Skin tones' })).toHaveLength(0))
|
||||
|
@ -152,8 +162,12 @@ describe('Picker tests', () => {
|
|||
|
||||
test('Escape key dismisses skintone listbox', async () => {
|
||||
await openSkintoneListbox(container)
|
||||
await waitFor(() => expect(getByRole('option', { name: 'Default', selected: true }))
|
||||
.toBe(activeElement()))
|
||||
await waitFor(() => (
|
||||
expect(
|
||||
getByRole('option', { name: 'Default', selected: true }).id)
|
||||
.toBe(queryByRole('listbox', { name: 'Skin tones' }).getAttribute('aria-activedescendant'))
|
||||
)
|
||||
)
|
||||
|
||||
await fireEvent.keyDown(activeElement(), { key: 'Escape', code: 'Escape' })
|
||||
|
||||
|
@ -354,7 +368,6 @@ describe('Picker tests', () => {
|
|||
await waitFor(() => expect(emoji && emoji.name === 'donkey'))
|
||||
}, 5000)
|
||||
|
||||
// TODO: re-enable this behavior. See https://github.com/nolanlawson/emoji-picker-element/issues/14
|
||||
test('Closes skintone picker when blurred', async () => {
|
||||
fireEvent.click(getByRole('button', { name: /Choose a skin tone/ }))
|
||||
await waitFor(() => expect(getByRole('listbox', { name: 'Skin tones' })).toBeVisible())
|
||||
|
@ -364,6 +377,16 @@ describe('Picker tests', () => {
|
|||
await waitFor(() => expect(queryAllByRole('listbox', { name: 'Skin tones' })).toHaveLength(0))
|
||||
})
|
||||
|
||||
test('Closes skintone picker when focus moves to skintone trigger button', async () => {
|
||||
const chooseSkintoneButton = getByRole('button', { name: /Choose a skin tone/ })
|
||||
fireEvent.click(chooseSkintoneButton)
|
||||
await waitFor(() => expect(getByRole('listbox', { name: 'Skin tones' })).toBeVisible())
|
||||
// Simulating a focusout event is hard, have to both focus and blur
|
||||
chooseSkintoneButton.focus()
|
||||
fireEvent.focusOut(getByRole('listbox', { name: 'Skin tones' }))
|
||||
await waitFor(() => expect(queryAllByRole('listbox', { name: 'Skin tones' })).toHaveLength(0))
|
||||
})
|
||||
|
||||
test('Custom emoji with categories', async () => {
|
||||
picker.customEmoji = [
|
||||
{
|
||||
|
|
|
@ -16,6 +16,11 @@ export async function openSkintoneListbox (container) {
|
|||
// JSDom doesn't fire transitionend events, so we do it manually here
|
||||
// https://github.com/jsdom/jsdom/issues/1781#issuecomment-467935000
|
||||
fireEvent(getByRole(container, 'listbox', { name: 'Skin tones' }), new Event('transitionend'))
|
||||
|
||||
await waitFor(() => (
|
||||
expect(container.getRootNode().activeElement)
|
||||
.toBe(getByRole(container, 'listbox', { name: 'Skin tones' }))
|
||||
))
|
||||
}
|
||||
|
||||
export function checkEmojiEquals (actual, expected) {
|
||||
|
|
Loading…
Reference in New Issue