diff --git a/src/picker/components/Picker/Picker.js b/src/picker/components/Picker/Picker.js index 30ac902..b6cc7f3 100644 --- a/src/picker/components/Picker/Picker.js +++ b/src/picker/components/Picker/Picker.js @@ -392,7 +392,7 @@ export function createRoot (target, props) { updateCurrentEmojis(newEmojis) updateSearchMode(true) } - } else if (currentGroup) { + } else { const { id: currentGroupId } = currentGroup // avoid race condition where currentGroupId is -1 and customEmoji is undefined/empty if (currentGroupId !== -1 || (customEmoji && customEmoji.length)) { @@ -544,11 +544,12 @@ export function createRoot (target, props) { case 'ArrowUp': return goToNextOrPrevious(true) case 'Enter': - if (state.activeSearchItem !== -1) { + if (state.activeSearchItem === -1) { + // focus the first option in the list since the list must be non-empty at this point (it's verified above) + state.activeSearchItem = 0 + } else { // there is already an active search item halt(event) return clickEmoji(state.currentEmojis[state.activeSearchItem].id) - } else if (state.currentEmojis.length) { - state.activeSearchItem = 0 } } } @@ -652,6 +653,7 @@ export function createRoot (target, props) { function onClickSkinToneButton (event) { state.skinTonePickerExpanded = !state.skinTonePickerExpanded state.activeSkinTone = state.currentSkinTone + // this should always be true, since the button is obscured by the listbox, so this `if` is just to be sure if (state.skinTonePickerExpanded) { halt(event) requestAnimationFrame(() => focus('skintone-list')) @@ -672,10 +674,6 @@ export function createRoot (target, props) { }) function onSkinToneOptionsKeydown (event) { - if (!state.skinTonePickerExpanded) { - return - } - const changeActiveSkinTone = async nextSkinTone => { halt(event) state.activeSkinTone = nextSkinTone @@ -703,9 +701,6 @@ export function createRoot (target, props) { } function onSkinToneOptionsKeyup (event) { - if (!state.skinTonePickerExpanded) { - return - } switch (event.key) { case ' ': // enter on keydown, space on keyup. this is just how browsers work for buttons diff --git a/src/picker/components/Picker/framework.js b/src/picker/components/Picker/framework.js index 851ee88..b60d66a 100644 --- a/src/picker/components/Picker/framework.js +++ b/src/picker/components/Picker/framework.js @@ -33,6 +33,7 @@ function doChildrenNeedRerender (parentNode, newChildren) { oldChild = oldChild.nextSibling oldChildrenCount++ } + /* istanbul ignore if */ if (process.env.NODE_ENV !== 'production' && oldChildrenCount !== parentNode.children.length) { throw new Error('parentNode.children.length is different from oldChildrenCount, it should not be') } diff --git a/test/spec/picker/Picker.test.js b/test/spec/picker/Picker.test.js index dcdce42..e971ca6 100644 --- a/test/spec/picker/Picker.test.js +++ b/test/spec/picker/Picker.test.js @@ -178,6 +178,16 @@ describe('Picker tests', () => { await waitFor(() => expect(queryAllByRole('listbox', { name: 'Skin tones' })).toHaveLength(0)) }) + test('Click skintone button while picker is open', async () => { + // this should not be possible since the picker covers the button when it's open, + // but this is for test coverage, and just to be safe + await openSkintoneListbox(container) + await fireEvent.click(getByRole('button', { name: /Choose a skin tone/ })) + + // listbox closes + await waitFor(() => expect(queryAllByRole('listbox', { name: 'Skin tones' })).toHaveLength(0)) + }) + test('nav keyboard test', async () => { getByRole('tab', { name: 'Smileys and emoticons', selected: true }).focus() @@ -339,6 +349,19 @@ describe('Picker tests', () => { ), { timeout: 5000 }) }, 10000) + test('press enter on an empty search list', async () => { + await tick(120) + type(getByRole('combobox'), 'xxxyyyzzzhahaha') + await waitFor(() => expect(queryAllByRole('option')).toHaveLength(0)) + expect(getByRole('combobox').getAttribute('aria-activedescendant')).toBeFalsy() + await tick(120) + fireEvent.keyDown(getByRole('combobox'), { key: 'Enter', code: 'Enter' }) + await tick(120) + // should do nothing basically since there's nothing to search for + expect(queryAllByRole('option')).toHaveLength(0) + expect(getByRole('combobox').getAttribute('aria-activedescendant')).toBeFalsy() + }, 10000) + test('press enter to make first search item active - custom emoji', async () => { picker.customEmoji = [ {